classification
Title: Python crash on macOS when CWD is invalid
Type: Stage: resolved
Components: macOS Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: lkollar, ncoghlan, ned.deily, pablogsal, ronaldoussoren, vstinner
Priority: normal Keywords: patch

Created on 2019-03-08 12:32 by lkollar, last changed 2019-03-30 12:43 by ncoghlan. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 12237 closed pablogsal, 2019-03-08 12:38
PR 12424 merged vstinner, 2019-03-19 02:19
PR 12441 merged vstinner, 2019-03-19 15:24
PR 12450 merged vstinner, 2019-03-19 23:03
Messages (19)
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) 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) 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) 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) 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) 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) 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) 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) 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) 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) 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) 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) Date: 2019-03-19 17:24
Pablo: is Python 3.7 affected by the issue?
msg338394 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) 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) 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) 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) 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) 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) Date: 2019-03-30 12:43
Thanks for sorting this out, Victor!
History
Date User Action Args
2019-03-30 12:43:15ncoghlansetresolution: fixed
messages: + msg339195
2019-03-24 21:01:23vstinnersetmessages: + msg338754
2019-03-24 19:42:03ned.deilysetstatus: open -> closed

messages: + msg338746
stage: patch review -> resolved
2019-03-19 23:31:21vstinnersetmessages: + msg338418
2019-03-19 23:30:50vstinnersetmessages: + msg338417
2019-03-19 23:03:38vstinnersetpull_requests: + pull_request12404
2019-03-19 17:29:48pablogsalsetmessages: + msg338394
2019-03-19 17:24:40vstinnersetmessages: + msg338393
2019-03-19 17:22:58vstinnersetmessages: + msg338392
2019-03-19 15:32:41pablogsalsetmessages: + msg338365
2019-03-19 15:24:35vstinnersetpull_requests: + pull_request12396
2019-03-19 15:12:18vstinnersetmessages: + msg338363
2019-03-19 15:09:36vstinnersetmessages: + msg338362
2019-03-19 02:19:33vstinnersetpull_requests: + pull_request12379
2019-03-19 00:32:14pablogsalsetmessages: + msg338301
2019-03-19 00:02:42vstinnersetmessages: + msg338300
2019-03-18 23:10:07pablogsalsetmessages: + msg338297
2019-03-18 12:06:48vstinnersetmessages: + msg338209
2019-03-17 04:30:02ncoghlansetmessages: + msg338116
2019-03-12 15:57:44vstinnersetversions: + Python 3.8
2019-03-12 15:57:03vstinnersetnosy: + ncoghlan
messages: + msg337767
2019-03-09 00:42:59vstinnersetnosy: + vstinner
2019-03-08 12:38:34pablogsalsetkeywords: + patch
stage: patch review
pull_requests: + pull_request12224
2019-03-08 12:38:17pablogsalsetnosy: + pablogsal
messages: + msg337470
2019-03-08 12:32:47lkollarcreate