Description

gpshead

opened on May 10, 2023

Issue body actions

Background

When using the subprocess module on Linux, vfork() rather than fork() has been preferred when possible for a few Python releases as is generally offers MUCH higher performance, the larger the parent process, the more cpu time it saves on page table copying.

Problem

vfork() - by design - does not return control to parent process until after the child process has successfully performed an exec() system call or has died.

Python subprocess uses _posixsubprocess.fork_exec() extension module implementation to do handle async-signal-safe fork/vfork+exec code path (as that cannot safely be implemented in Python). This function does not release the GIL. Thus the GIL is held across the vfork() call. Meaning the parent process does not resume execution until the exec has succeeded, or failed and returned an error which our child writes to it's errpipe_write fd before exiting.

This can result in all Python threads hanging (ie: the GIL is held by a blocked thread) when the child exec system call takes a long time. Such as the binary existing on a potentially slow or high latency network filesystem. (We witnessed this on a FUSE filesystem backed by remote (high latency) cloud storage with a large executable... but any slow or high latency filesystem containing an executable or anything interfering in the exec system call can cause the same problem).

Workarounds

Building without vfork support in _posixsubprocess or otherwise passing an arg to subprocess API that happens to disable vfork because it is incompatible with the vfork-concept are workarounds. (I won't recommend any of those because none of them are a good idea to pass when not needed for their primary purpose).

Another workaround viable in our specific case is to pre-read the executable before asking subprocess to launch it as that moves the high IO latency from the exec system call into a read without the GIL held beforehand as it pulls the executable into a fast-enough local storage cache. (That being useful at all depends on your particular filesystem latency implementation situation)

Potential Solutions

If we could simply release the GIL around the vfork() call that would seem to be ideal... It's not clear to me that we can actually do that, I'm working out what a PR would look like or what blocks us from doing so safely.

Offering yet another keyword argument to subprocess APIs to disable the use of APIs that could block the entire process upon exec() such as vfork is viable - but doesn't feel great given our existing long list of subprocess args.

Linked PRs

• gh-104372: Unlock the GIL before exec when using vfork(). #104515
• gh-104372: Cleanup _posixsubprocess make_inheritable for async signal safety #104518
• gh-104372: Use non-Raw malloc for c_fds_to_keep in _posixsubprocess #104697
• gh-104372: Drop the GIL around the vfork() call. #104782
• [3.11] gh-104372: Cleanup _posixsubprocess make_inheritable for async signal safety gh-104518 #104785
• gh-104372: use == -1 before PyErr_Occurred #104831
• [3.12] gh-104372: use == -1 before PyErr_Occurred (GH-104831) #104833
• [3.12] gh-104372: Drop the GIL around the vfork() call. (GH-104782) #104942
• [3.11] gh-104372: Drop the GIL around the vfork() call. (#104782) #104958

Activity

1. gpshead

   added

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   stdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directory

   extension-modulesC modules in the Modules dirC modules in the Modules dir

   on May 10, 2023
2. gpshead

   self-assigned this

   on May 10, 2023
3. gpshead commented on May 15, 2023

   

   gpshead

   on May 15, 2023

   MemberAuthor

   More actions

   A practical workaround usable in some cases:

   +import shlex
    ...
   -    subprocess.run(argv, ...)
   +    subprocess.run(shlex.join(["exec"] + argv, shell=True, ...)

   Do not use this workaround unless you fully control argv and know that it cannot contain anything that might wind up as shell injection.

   That defers the blocking exec into an intermediary post-vfork-exec child process using your already installed shell that is already capable of doing that.
4. gpshead

   added 2 commits that reference this issue on May 16, 2023

   pythongh-104372: Cleanup make_inheritable for async signal safety.

   adae5af

   gh-104372: Cleanup _posixsubprocess `make_inheritable` for async sign…

   c649df6
5. JelleZijlstra

   added a commit that references this issue on May 18, 2023

   pythongh-104372: Cleanup _posixsubprocess `make_inheritable` for asyn…

   4b2bf16
6. gpshead

   added 7 commits that reference this issue on May 20, 2023

   gh-104372: Use non-Raw malloc for c_fds_to_keep in _posixsubprocess (#…

   d1732fe

   pythongh-104372: Drop the GIL around the vfork() call.

   2d54d8e

   [3.11] pythongh-104372: Cleanup _posixsubprocess `make_inheritable` f…

   2c04e99

   [3.11] pythongh-104372: Cleanup _posixsubprocess `make_inheritable` f…

   80ee5fc

   [3.11] pythongh-104372: Cleanup _posixsubprocess `make_inheritable` f…

   ff570e5

   pythongh-104372: Cleanup _posixsubprocess `make_inheritable` for asyn…

   de47455

   [3.11] pythongh-104372: Cleanup _posixsubprocess `make_inheritable` f…

   0991dae
7. 3 remaining items

8. gpshead commented on May 23, 2023

   

   gpshead

   on May 23, 2023

   MemberAuthor

   More actions

   A heavy handed workaround, disable the use of vfork entirely:

   subprocess._USE_VFORK = False

   if your process is large this will slow down process launching time (regular fork is used - which must copy the entire process memory map) but for most things that is perfectly fine.
9. gpshead

   added 2 commits that reference this issue on May 24, 2023

   [3.11] gh-104372: Cleanup _posixsubprocess make_inheritable for async…

   6d00ae3

   pythongh-104372: use == -1 before PyErr_Occurred

   eefb96d
10. miss-islington

    added a commit that references this issue on May 24, 2023

    pythongh-104372: use == -1 before PyErr_Occurred (pythonGH-104831)

    be64b8c
11. gpshead

    added 2 commits that reference this issue on May 24, 2023

    gh-104372: use == -1 before PyErr_Occurred (#104831)

    7f963bf

    [3.12] gh-104372: use == -1 before PyErr_Occurred (GH-104831) (#104833)

    b719dd8
12. gpshead

    closed this as completedin #104782on May 25, 2023
13. gpshead

    added a commit that references this issue on May 25, 2023

    gh-104372: Drop the GIL around the vfork() call. (#104782)

    d086792
14. miss-islington

    added a commit that references this issue on May 25, 2023

    pythongh-104372: Drop the GIL around the vfork() call. (pythonGH-104782)

    302217d
15. gpshead commented on May 25, 2023

    

    gpshead

    on May 25, 2023

    MemberAuthor

    More actions

    I'll take care of a 3.11.x bug fix backport, but I want to let this bake in some 3.12beta releases for a while first. It isn't urgent.
16. gpshead

    added a commit that references this issue on May 25, 2023

    [3.12] gh-104372: Drop the GIL around the vfork() call. (GH-104782) (#…

    5c2971b
17. gpshead

    reopened this on May 26, 2023
18. gpshead

    added a commit that references this issue on May 26, 2023

    pythongh-104372: Drop the GIL around the vfork() call. (python#104782)

    75f7969
19. gpshead

    closed this as completedon May 26, 2023
20. gpshead

    added a commit that references this issue on Sep 1, 2023

    [3.11] gh-104372: Drop the GIL around the vfork() call. (#104782) (#1…

    6ba1234

Issue actions