| msg337469 - (view) | Author: László Kiss Kollár (lkollar) | Date: 2019-03-08 12:32 |
CPython crashes with "pymain_compute_path0: memory allocation failed" when attempting to execute it with a library module from a previously deleted directory. To reproduce: cd ~/tmp/python_crash rm -rf ~/tmp/python_crash python3.7 -m pdb Fatal Python error: pymain_compute_path0: memory allocation failed ValueError: character U+ef3a8fe0 is not in range [U+0000; U+10ffff] Current thread 0x000000011060e5c0 (most recent call first): |
| msg337470 - (view) | Author: Pablo Galindo Salgado (pablogsal) *  | Date: 2019-03-08 12:38 |
This is happening because _Py_wgetcwd returns NULL (although the underliying getcwd system call populates the `fullpath` variable correctly) but its caller, _PyPathConfig_ComputeArgv0, does not check the return value: _Py_wgetcwd(fullpath, Py_ARRAY_LENGTH(fullpath)); argv0 = fullpath; and its caller, pymain_run_python, interprets a failure in _PyPathConfig_ComputeArgv0 as a memory problem: PyObject *path0 = _PyPathConfig_ComputeArgv0(config->argc, config->argv); if (path0 == NULL) { err = _Py_INIT_NO_MEMORY(); goto done; } |
| msg337767 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2019-03-12 15:57 |
_Py_wgetcwd() call has been introduced by the following commit: commit ee3784594b33c72c3fdca6a71892d22f14045ab6 Author: Nick Coghlan <ncoghlan@gmail.com> Date: Sun Mar 25 23:43:50 2018 +1000 bpo-33053: -m now adds *starting* directory to sys.path (GH-6231) (#6236) Historically, -m added the empty string as sys.path zero, meaning it resolved imports against the current working directory, the same way -c and the interactive prompt do. This changes the sys.path initialisation to add the *starting* working directory as sys.path[0] instead, such that changes to the working directory while the program is running will have no effect on imports when using the -m switch. (cherry picked from commit d5d9e02dd3c6df06a8dd9ce75ee9b52976420a8b) I don't think that it's correct to fail with a fatal error if the current directory no longer exist. Would it be possible to not add it to sys.path if it doesn't exist? Silently ignore the error. @Nick: What do you think? |
| msg338116 - (view) | Author: Alyssa Coghlan (ncoghlan) *  | Date: 2019-03-17 04:30 |
Omitting it from sys.path in that case makes sense to me, but I'm not sure what sys.argv[0] should be set to. |
| msg338209 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2019-03-18 12:06 |
> Omitting it from sys.path in that case makes sense to me, but I'm not sure what sys.argv[0] should be set to. I propose to use ".". It would be consistent with platforms which doesn't implement realpath: if (have_module_arg) { #if defined(HAVE_REALPATH) || defined(MS_WINDOWS) _Py_wgetcwd(fullpath, Py_ARRAY_LENGTH(fullpath)); argv0 = fullpath; n = wcslen(argv0); #else argv0 = L"."; n = 1; #endif } And it defers the error handler to later. Example of Python 3 running in a removed directory: $ python3 Python 3.7.2 (default, Jan 16 2019, 19:49:22) >>> import os >>> os.getcwd() FileNotFoundError: [Errno 2] No such file or directory >>> os.path.abspath('.') Traceback (most recent call last): File "/usr/lib64/python3.7/posixpath.py", line 383, in abspath cwd = os.getcwd() FileNotFoundError: [Errno 2] No such file or directory I would prefer "." than "-m". |
| msg338297 - (view) | Author: Pablo Galindo Salgado (pablogsal) *  | Date: 2019-03-18 23:10 |
If we set argv0 to ".": if (_Py_wgetcwd(fullpath, Py_ARRAY_LENGTH(fullpath))) { argv0 = fullpath; n = wcslen(argv0); } else { argv0 = L"."; n = 1; } then the caller does not have any way of knowing if the returned argv0 is due to the fact that _Py_wgetcwd failed so it will blindly add "." to sys.path unless we set the result using PyObject** and then returning some error code to signal what happened (or something similar). On the other hand, I do not like this API, because returning some error code != 0 from _PyPathConfig_ComputeArgv0 would be weird because the call actually succeeded (it has returned "." as the value for argv0). What do you think is the best way to signal pymain_run_python that the current directory does not exist (because _Py_wgetcwd has failed)? |
| msg338300 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2019-03-19 00:02 |
Oh. It seems like I misunderstood the issue. I read "argv0" as sys.argv[0], but _PyPathConfig_ComputeArgv0 is used to insert a value at the start of sys.path. That's different... If the current directory has been removed, sys.path should just be left unchanged, no? |
| msg338301 - (view) | Author: Pablo Galindo Salgado (pablogsal) *  | Date: 2019-03-19 00:32 |
Yup. But what is the best way to signal the caller that _PyPathConfig_ComputeArgv0 has failed because _Py_wgetcwd has failed without changing the API massively? Right now if _PyPathConfig_ComputeArgv0 returns null is assumed that is due to a memory error when calling PyUnicode_FromWideChar. So either we stop returning _Py_INIT_NO_MEMORY() and then skip appending to sys_path or we change the API to signal different problems to the caller. Also, notice that the same function is used in sysmodule.c in PySys_SetArgvEx: If argv[0] is not '-c' nor '-m', prepend argv[0] to sys.path. |
| msg338362 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2019-03-19 15:09 |
New changeset dcf617152e1d4c4a5e7965733928858a9c0936ca by Victor Stinner in branch 'master': bpo-36236: Handle removed cwd at Python init (GH-12424) https://github.com/python/cpython/commit/dcf617152e1d4c4a5e7965733928858a9c0936ca |
| msg338363 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2019-03-19 15:12 |
Can someone please on macOS to confirm that the bug is fixed? |
| msg338365 - (view) | Author: Pablo Galindo Salgado (pablogsal) *  | Date: 2019-03-19 15:32 |
It seems to work: ❯ uname -a Darwin C02VL073HTDG 18.2.0 Darwin Kernel Version 18.2.0: Thu Dec 20 20:46:53 PST 2018; root:xnu-4903.241.1~1/RELEASE_X86_64 x86_64 ❯ pwd /tmp/check /tmp/check ❯ rm -rf ../check /tmp/check ❯ ~/github/cpython/python.exe -m pdb usage: pdb.py [-c command] ... [-m module | pyfile] [arg] ... Debug the Python program given by pyfile. Alternatively, an executable module or package to debug can be specified using the -m switch. Initial commands are read from .pdbrc files in your home directory and in the current directory, if they exist. Commands supplied with -c are executed after commands from .pdbrc files. To let the script run until an exception occurs, use "-c continue". To let the script run up to a given line X in the debugged file, use "-c 'until X'". |
| msg338392 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2019-03-19 17:22 |
New changeset fc96e5474a7bda1c5dec66420e4467fc9f7ca968 by Victor Stinner in branch 'master': bpo-36236: Fix _PyPathConfig_ComputeSysPath0() for empty argv (GH-12441) https://github.com/python/cpython/commit/fc96e5474a7bda1c5dec66420e4467fc9f7ca968 |
| msg338393 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2019-03-19 17:24 |
Pablo: is Python 3.7 affected by the issue? |
| msg338394 - (view) | Author: Pablo Galindo Salgado (pablogsal) *  | Date: 2019-03-19 17:29 |
Yup: ~ ❯ cd /tmp /tmp ❯ mkdir check /tmp ❯ cd check /tmp/check ❯ rm -rf ../check /tmp/check ❯ python3.7 -m pdb Fatal Python error: pymain_compute_path0: memory allocation failed ValueError: character U+e0895f00 is not in range [U+0000; U+10ffff] Current thread 0x000000011d9405c0 (most recent call first): |
| msg338417 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2019-03-19 23:30 |
New changeset f7959a9fe7e7e316899c60251e29390c4ed0307a by Victor Stinner in branch '3.7': bpo-36236: Handle removed cwd at Python init (GH-12450) https://github.com/python/cpython/commit/f7959a9fe7e7e316899c60251e29390c4ed0307a |
| msg338418 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2019-03-19 23:31 |
Pablo: Oh ok, thanks for testing. I fixed 3.7 as well. Would you mind to validate my fix? |
| msg338746 - (view) | Author: Ned Deily (ned.deily) *  | Date: 2019-03-24 19:42 |
The fix for 3.7 works too: no more crashing. Thanks, everyone! |
| msg338754 - (view) | Author: STINNER Victor (vstinner) *  | Date: 2019-03-24 21:01 |
> The fix for 3.7 works too: no more crashing. Thanks, everyone! I planned to test, but I had no access to macOS last years. Thanks for testing Ned. Good to hear that the bug is now fixed in 3.7 and master. |
| msg339195 - (view) | Author: Alyssa Coghlan (ncoghlan) *  | Date: 2019-03-30 12:43 |
Thanks for sorting this out, Victor! |
|
| Date | User | Action | Args |
| 2022-04-11 14:59:12 | admin | set | github: 80417 |
| 2019-03-30 12:43:15 | ncoghlan | set | resolution: fixed messages: + msg339195 |
| 2019-03-24 21:01:23 | vstinner | set | messages: + msg338754 |
| 2019-03-24 19:42:03 | ned.deily | set | status: open -> closed
messages: + msg338746 stage: patch review -> resolved |
| 2019-03-19 23:31:21 | vstinner | set | messages: + msg338418 |
| 2019-03-19 23:30:50 | vstinner | set | messages: + msg338417 |
| 2019-03-19 23:03:38 | vstinner | set | pull_requests: + pull_request12404 |
| 2019-03-19 17:29:48 | pablogsal | set | messages: + msg338394 |
| 2019-03-19 17:24:40 | vstinner | set | messages: + msg338393 |
| 2019-03-19 17:22:58 | vstinner | set | messages: + msg338392 |
| 2019-03-19 15:32:41 | pablogsal | set | messages: + msg338365 |
| 2019-03-19 15:24:35 | vstinner | set | pull_requests: + pull_request12396 |
| 2019-03-19 15:12:18 | vstinner | set | messages: + msg338363 |
| 2019-03-19 15:09:36 | vstinner | set | messages: + msg338362 |
| 2019-03-19 02:19:33 | vstinner | set | pull_requests: + pull_request12379 |
| 2019-03-19 00:32:14 | pablogsal | set | messages: + msg338301 |
| 2019-03-19 00:02:42 | vstinner | set | messages: + msg338300 |
| 2019-03-18 23:10:07 | pablogsal | set | messages: + msg338297 |
| 2019-03-18 12:06:48 | vstinner | set | messages: + msg338209 |
| 2019-03-17 04:30:02 | ncoghlan | set | messages: + msg338116 |
| 2019-03-12 15:57:44 | vstinner | set | versions: + Python 3.8 |
| 2019-03-12 15:57:03 | vstinner | set | nosy: + ncoghlan messages: + msg337767
|
| 2019-03-09 00:42:59 | vstinner | set | nosy: + vstinner
|
| 2019-03-08 12:38:34 | pablogsal | set | keywords: + patch stage: patch review pull_requests: + pull_request12224 |
| 2019-03-08 12:38:17 | pablogsal | set | nosy: + pablogsal messages: + msg337470
|
| 2019-03-08 12:32:47 | lkollar | create | |