Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interpreter exits on Windows due to ValueError raised in linecache.py #122170

Closed
devdanzin opened this issue Jul 23, 2024 · 18 comments
Closed

Interpreter exits on Windows due to ValueError raised in linecache.py #122170

devdanzin opened this issue Jul 23, 2024 · 18 comments
Labels
OS-windows type-bug An unexpected behavior, bug, or error

Comments

@devdanzin
Copy link
Contributor

devdanzin commented Jul 23, 2024

Bug report

Bug description:

It's possible to make the interpreter exit on Windows in 3.13.0b4 and main due to ValueError: stat: path too long for Windows being raised by os.stat() in updatecache in linecache.py when trying to print a traceback:

>>> exec(compile("print(2**100000)", "s"*99999, "exec"))
Exception ignored in the internal traceback machinery:
Traceback (most recent call last):
  File "~\PycharmProjects\cpython\Lib\traceback.py", line 139, in _print_exception_bltin
    return print_exception(exc, limit=BUILTIN_EXCEPTION_LIMIT, file=file, colorize=colorize)
  File "~\PycharmProjects\cpython\Lib\traceback.py", line 129, in print_exception
    te = TracebackException(type(value), value, tb, limit=limit, compact=True)
  File "~\PycharmProjects\cpython\Lib\traceback.py", line 1133, in __init__
    context = TracebackException(
  File "~\PycharmProjects\cpython\Lib\traceback.py", line 1043, in __init__
    self.stack = StackSummary._extract_from_extended_frame_gen(
  File "~\PycharmProjects\cpython\Lib\traceback.py", line 492, in _extract_from_extended_frame_gen
    f.line
  File "~\PycharmProjects\cpython\Lib\traceback.py", line 369, in line
    self._set_lines()
  File "~\PycharmProjects\cpython\Lib\traceback.py", line 350, in _set_lines
    lines.append(linecache.getline(self.filename, lineno).rstrip())
  File "~\PycharmProjects\cpython\Lib\linecache.py", line 25, in getline
    lines = getlines(filename, module_globals)
  File "~\PycharmProjects\cpython\Lib\linecache.py", line 41, in getlines
    return updatecache(filename, module_globals)
  File "~\PycharmProjects\cpython\Lib\linecache.py", line 100, in updatecache
    stat = os.stat(fullname)
ValueError: stat: path too long for Windows
Traceback (most recent call last):
  File "~\PycharmProjects\cpython\Lib\code.py", line 91, in runcode
    exec(code, self.locals)
  File "<python-input-2>", line 1, in <module>
  File "sss[...]sssssss", line 1, in <module>
ValueError: Exceeds the limit (4300 digits) for integer string conversion; use sys.set_int_max_str_digits() to increase the limit

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "~\PycharmProjects\cpython\Lib\runpy.py", line 198, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "~\PycharmProjects\cpython\Lib\runpy.py", line 88, in _run_code
    exec(code, run_globals)
  File "~\PycharmProjects\cpython\Lib\_pyrepl\__main__.py", line 6, in <module>
    __pyrepl_interactive_console()
  File "~\PycharmProjects\cpython\Lib\_pyrepl\main.py", line 59, in interactive_console
    run_multiline_interactive_console(console)
  File "~\PycharmProjects\cpython\Lib\_pyrepl\simple_interact.py", line 156, in run_multiline_interactive_console
    more = console.push(_strip_final_indent(statement), filename=input_name, _symbol="single")  # type: ignore[call-arg]
  File "~\PycharmProjects\cpython\Lib\code.py", line 303, in push
    more = self.runsource(source, filename, symbol=_symbol)
  File "~\PycharmProjects\cpython\Lib\_pyrepl\console.py", line 200, in runsource
    self.runcode(code)
  File "~\PycharmProjects\cpython\Lib\code.py", line 95, in runcode
    self.showtraceback()
  File "~\PycharmProjects\cpython\Lib\_pyrepl\console.py", line 168, in showtraceback
    super().showtraceback(colorize=self.can_colorize)
  File "~\PycharmProjects\cpython\Lib\code.py", line 147, in showtraceback
    lines = traceback.format_exception(ei[0], ei[1], last_tb.tb_next, colorize=colorize)
  File "~\PycharmProjects\cpython\Lib\traceback.py", line 154, in format_exception
    te = TracebackException(type(value), value, tb, limit=limit, compact=True)
  File "~\PycharmProjects\cpython\Lib\traceback.py", line 1043, in __init__
    self.stack = StackSummary._extract_from_extended_frame_gen(
  File "~\PycharmProjects\cpython\Lib\traceback.py", line 492, in _extract_from_extended_frame_gen
    f.line
  File "~\PycharmProjects\cpython\Lib\traceback.py", line 369, in line
    self._set_lines()
  File "~\PycharmProjects\cpython\Lib\traceback.py", line 350, in _set_lines
    lines.append(linecache.getline(self.filename, lineno).rstrip())
  File "~\PycharmProjects\cpython\Lib\linecache.py", line 25, in getline
    lines = getlines(filename, module_globals)
  File "~\PycharmProjects\cpython\Lib\linecache.py", line 41, in getlines
    return updatecache(filename, module_globals)
  File "~\PycharmProjects\cpython\Lib\linecache.py", line 100, in updatecache
    stat = os.stat(fullname)
ValueError: stat: path too long for Windows
[Thread 30100.0x6108 exited with code 1]
[Thread 30100.0x5da4 exited with code 1]
[Thread 30100.0x90a0 exited with code 1]
[Inferior 1 (process 30100) exited with code 01]

CPython versions tested on:

3.13, CPython main branch

Operating systems tested on:

Windows

Linked PRs

@devdanzin devdanzin added the type-bug An unexpected behavior, bug, or error label Jul 23, 2024
@picnixz
Copy link
Contributor

picnixz commented Jul 23, 2024

I'm surprised that os.stat raises a ValueError in this case actually and not an OSError. I'll make a PR for handling this case in linecache.

@devdanzin
Copy link
Contributor Author

I'll make a PR for handling this case in linecache.

I have one ready.

@picnixz
Copy link
Contributor

picnixz commented Jul 23, 2024

Oh I was just going to push it now xD Go for yours then, I'll review it.

@devdanzin
Copy link
Contributor Author

Yours is much more complete and polished, is it OK if I close mine?

@picnixz
Copy link
Contributor

picnixz commented Jul 23, 2024

Err... it's up to you.

@devdanzin
Copy link
Contributor Author

I'll close it, I'd rather have code of higher quality in the standard library than code I wrote :)

@eryksun
Copy link
Contributor

eryksun commented Jul 23, 2024

I'm surprised that os.stat raises a ValueError in this case actually and not an OSError. I'll make a PR for handling this case in linecache.

For some reason, code was added to the path_t converter to raise ValueError if the path length exceeds 32767 characters. The length limit is actually 32766 characters, because the terminating null must be included in the buffer size. Properly, the expression UNICODE_STRING_MAX_CHARS - 1 should be used instead of 32766. But really, a path_t using function should just let the system API fail and raise an OSError, either for the system error code ERROR_FILENAME_EXCED_RANGE or ERROR_PATH_NOT_FOUND, depending on the system API function that's called.

cpython/Modules/posixmodule.c

Lines 1331 to 1336 in 2c1b1e7

#ifdef MS_WINDOWS
if (!path->nonstrict && length > 32767) {
FORMAT_EXCEPTION(PyExc_ValueError, "%s too long for Windows");
goto error_exit;
}
#endif

cpython/Modules/posixmodule.c

Lines 1398 to 1403 in 2c1b1e7

#ifdef MS_WINDOWS
if (!path->nonstrict && length > 32767) {
FORMAT_EXCEPTION(PyExc_ValueError, "%s too long for Windows");
goto error_exit;
}
#endif

@ZeroIntensity
Copy link
Member

I get a different error when using the reproducer on Linux:

ValueError: Exceeds the limit (4300 digits) for integer string conversion; use sys.set_int_max_str_digits() to increase the limit

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/runpy.py", line 198, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/runpy.py", line 88, in _run_code
    exec(code, run_globals)
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/_pyrepl/__main__.py", line 6, in <module>
    __pyrepl_interactive_console()
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/_pyrepl/main.py", line 57, in interactive_console
    run_multiline_interactive_console(console)
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/_pyrepl/simple_interact.py", line 156, in run_multiline_interactive_console
    more = console.push(_strip_final_indent(statement), filename=input_name, _symbol="single")  # type: ignore[call-arg]
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/code.py", line 303, in push
    more = self.runsource(source, filename, symbol=_symbol)
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/_pyrepl/console.py", line 200, in runsource
    self.runcode(code)
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/code.py", line 95, in runcode
    self.showtraceback()
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/_pyrepl/console.py", line 168, in showtraceback
    super().showtraceback(colorize=self.can_colorize)
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/code.py", line 147, in showtraceback
    lines = traceback.format_exception(ei[0], ei[1], last_tb.tb_next, colorize=colorize)
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/traceback.py", line 155, in format_exception
    return list(te.format(chain=chain, colorize=colorize))
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/traceback.py", line 1384, in format
    yield from _ctx.emit(exc.stack.format(colorize=colorize))
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/traceback.py", line 747, in format
    formatted_frame = self.format_frame_summary(frame_summary, colorize=colorize)
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/traceback.py", line 583, in format_frame_summary
    show_carets = self._should_show_carets(start_offset, end_offset, all_lines, anchors)
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/traceback.py", line 701, in _should_show_carets
    statement = tree.body[0]
IndexError: list index out of range

CC @picnixz, this might be a separate issue. This still occurs on both PRs.

@devdanzin
Copy link
Contributor Author

devdanzin commented Jul 23, 2024

I get a different error when using the reproducer on Linux:

[...]
  File "/home/zero/Desktop/Projects/Python/cpython/Lib/traceback.py", line 701, in _should_show_carets
    statement = tree.body[0]
IndexError: list index out of range

That's #122145.

@picnixz
Copy link
Contributor

picnixz commented Jul 23, 2024

Which is fixed by #122161

@picnixz
Copy link
Contributor

picnixz commented Jul 23, 2024

@eryksun I see the ValueError was implemented in #118355, which is recent enough. However, I think that os.stat raising an OSError would be more correct because on Linux, this is what happens: OSError: [Errno 36] File name too long: ' but on Windows, IIRC, it should be a "FileNotFound" (but I have no way to check this now). Do you think I should open another issue for os.stat ? (and maybe more functions?)

@eryksun
Copy link
Contributor

eryksun commented Jul 23, 2024

@eryksun I see the ValueError was implemented in #118355, which is recent enough. However, I think that os.stat raising an OSError would be more correct because on Linux, this is what happens: OSError: [Errno 36] File name too long: ' but on Windows, IIRC, it should be a "FileNotFound" (but I have no way to check this now). Do you think I should open another issue for os.stat ? (and maybe more functions?)

The issue is with the path_t converter, not specifically with os.stat(). I'd be in favor of letting the system API call fail and raising whichever OSError exception corresponds to the error code. C ENAMETOOLONG isn't used on Windows. It's going to be either ENOENT or EINVAL, and only the former gets specialized as FileNotFoundError.

If long paths are enabled at the system level (via the "LongPathsEnabled" registry value in Windows 10+), then the legacy DOS MAX_PATH (260) limit is removed, and only native NT limits are enforced. In this case, WinAPI CreateFileW() fails with ERROR_FILENAME_EXCED_RANGE (206) if the pathname length exceeds the system's pathname length limit UNICODE_STRING_MAX_CHARS (32767, including the terminating null), or fails with ERROR_INVALID_NAME (123) if the filename length exceeds the filesystem's filename length limit, which is typically the system's filename length limit MAXIMUM_FILENAME_LENGTH (256, including the terminating null). The C runtime library maps ERROR_FILENAME_EXCED_RANGE to C ENOENT (2), and it maps ERROR_INVALID_NAME to C EINVAL (22). For the sake of consistency, Python follows suit in "PC/errmap.h".

If long paths are disabled or not supported (i.e. prior to Windows 10), then pathnames that aren't prefixed by "\\?\" are limited to MAX_PATH (260), and filenames are typically limited to MAXIMUM_FILENAME_LENGTH. In this case, WinAPI CreateFileW() fails with ERROR_PATH_NOT_FOUND (3) if the pathname length exceeds MAX_PATH or with ERROR_FILENAME_EXCED_RANGE if the pathname length exceeds UNICODE_STRING_MAX_CHARS. Usually in this case the filename length limit doesn't affect the error, assuming the filesystem's limit is MAXIMUM_FILENAME_LENGTH. In the rare case where the pathname of the working directory is only 4 characters (e.g. "C:\" plus the terminating null), then a filename length of exactly 256 characters causes the error to be ERROR_INVALID_NAME instead of ERROR_PATH_NOT_FOUND. The C runtime library maps ERROR_PATH_NOT_FOUND to C ENOENT, and Python follows suit.

@picnixz
Copy link
Contributor

picnixz commented Jul 24, 2024

I'll file an issue to summarize what you told me (and to make the discussion distinct from this issue). If we were to patch the path_t converter, then we would not need the patch for linecache and it could be better IMO (I don't really like to intercept this ValueError that is raised by os.stat because of a conversion function and not because of the underlying C call).

@devdanzin
Copy link
Contributor Author

If we were to patch the path_t converter, then we would not need the patch for linecache and it could be better IMO (I don't really like to intercept this ValueError that is raised by os.stat because of a conversion function and not because of the underlying C call).

I believe that would also fix two minor related issues I've found (posting here so I don't forget about them):

> $Env:PYTHONPATH="a" * 33000
> py -3.13
Exception ignored in running getpath:
Traceback (most recent call last):
  File "<frozen getpath>", line 668, in <module>
OSError: failed to make path absolute
Fatal Python error: error evaluating path
Python runtime state: core initialized

Current thread 0x00005a8c (most recent call first):
  <no Python frame>
>>> import sys
>>> sys.path.insert(0, "a" * 33000)
>>> import email
Traceback (most recent call last):
  File "<frozen importlib._bootstrap_external>", line 1512, in _path_importer_cache
KeyError: 'aaaaaaaaaaaaaaaaaaaaaaaa[...]aaaaaaaaaa'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<python-input-6>", line 1, in <module>
    import email
  File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1322, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 1262, in _find_spec
  File "<frozen importlib._bootstrap_external>", line 1555, in find_spec
  File "<frozen importlib._bootstrap_external>", line 1527, in _get_spec
  File "<frozen importlib._bootstrap_external>", line 1514, in _path_importer_cache
  File "<frozen importlib._bootstrap_external>", line 1490, in _path_hooks
  File "<frozen importlib._bootstrap_external>", line 1714, in path_hook_for_FileFinder
  File "<frozen importlib._bootstrap_external>", line 173, in _path_isdir
  File "<frozen importlib._bootstrap_external>", line 158, in _path_is_mode_type
  File "<frozen importlib._bootstrap_external>", line 152, in _path_stat
ValueError: stat: path too long for Windows

@picnixz
Copy link
Contributor

picnixz commented Jul 24, 2024

Actually, I probably found another bug in linecache where requesting a filename with null bytes makes it fail:

$ ./python -c 'import linecache;linecache.getlines("a\x00bc")'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
    import linecache;linecache.getlines("a\x00bc")
                     ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
  File "/lib/python/cpython/Lib/linecache.py", line 41, in getlines
    return updatecache(filename, module_globals)
  File "/lib/python/cpython/Lib/linecache.py", line 100, in updatecache
    stat = os.stat(fullname)
ValueError: stat: embedded null character in path

but linecache is not supposed to fail =/.

@picnixz
Copy link
Contributor

picnixz commented Jul 24, 2024

(So the PR is still correct, would not be fixed even if we change the path_t converter since raising ValueError for NUL bytes seems to be something that was done for ages)

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 27, 2024
…ythonGH-122176)

(cherry picked from commit 7a6d4cc)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 27, 2024
…ythonGH-122176)

(cherry picked from commit 7a6d4cc)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
serhiy-storchaka pushed a commit that referenced this issue Jul 27, 2024
…H-122176) (GH-122349)

(cherry picked from commit 7a6d4cc)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
serhiy-storchaka pushed a commit that referenced this issue Jul 27, 2024
…H-122176) (GH-122348)

(cherry picked from commit 7a6d4cc)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@picnixz
Copy link
Contributor

picnixz commented Jul 27, 2024

Now that the PRs have been merged, I think this specific issue can be closed. It remains to fix the other issue at the level of imports and sys.path.insert, but I'm not sure it's something related to linecache. For now, I'm closing this issue (though, it remains to merge #122161 so if anyone can do it, I'd be happy) and we should open a new one for the imports related issue (could you do it @devdanzin please?)

@picnixz picnixz closed this as completed Jul 27, 2024
@zooba
Copy link
Member

zooba commented Aug 6, 2024

For the record, while I think it's okay for linecache to silently ignore this issue, I don't think that applies generally. Any code that's attempting to use the bad value ought to raise ValueError, and most likely it ought to propagate out to whoever provided the bad value.

So I'm not going to insist on reverting anything here, but I do intend to hold up further changes until they've been properly considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS-windows type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

5 participants