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) * ![Python committer (Python committer)](@@file/committer.png) |
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) * ![Python committer (Python committer)](@@file/committer.png) |
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: Nick Coghlan (ncoghlan) * ![Python committer (Python committer)](@@file/committer.png) |
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) * ![Python committer (Python committer)](@@file/committer.png) |
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) * ![Python committer (Python committer)](@@file/committer.png) |
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) * ![Python committer (Python committer)](@@file/committer.png) |
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) * ![Python committer (Python committer)](@@file/committer.png) |
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) * ![Python committer (Python committer)](@@file/committer.png) |
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) * ![Python committer (Python committer)](@@file/committer.png) |
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) * ![Python committer (Python committer)](@@file/committer.png) |
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) * ![Python committer (Python committer)](@@file/committer.png) |
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) * ![Python committer (Python committer)](@@file/committer.png) |
Date: 2019-03-19 17:24 |
Pablo: is Python 3.7 affected by the issue?
|
msg338394 - (view) |
Author: Pablo Galindo Salgado (pablogsal) * ![Python committer (Python committer)](@@file/committer.png) |
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) * ![Python committer (Python committer)](@@file/committer.png) |
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) * ![Python committer (Python committer)](@@file/committer.png) |
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) * ![Python committer (Python committer)](@@file/committer.png) |
Date: 2019-03-24 19:42 |
The fix for 3.7 works too: no more crashing. Thanks, everyone!
|
msg338754 - (view) |
Author: STINNER Victor (vstinner) * ![Python committer (Python committer)](@@file/committer.png) |
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: Nick Coghlan (ncoghlan) * ![Python committer (Python committer)](@@file/committer.png) |
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 | |