Skip to content

Commit

Permalink
pythongh-104372: Drop the GIL around the vfork() call.
Browse files Browse the repository at this point in the history
On Linux where the `subprocess` module can use the `vfork` syscall for
faster spawning, prevent the parent process from blocking other threads
by dropping the GIL while it waits for the vfork'ed child process `exec`
outcome.  This prevents spawning a binary from a slow filesystem from
blocking the rest of the application.

Fixes python#104372.
  • Loading branch information
gpshead committed May 23, 2023
1 parent 2e5d8a9 commit 2d54d8e
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
On Linux where :mod:`subprocess` can use the ``vfork()`` syscall for faster
spawning, prevent the parent process from blocking other threads by dropping
the GIL while it waits for the vfork'ed child process ``exec()`` outcome.
This prevents spawning a binary from a slow filesystem from blocking the
rest of the application.
21 changes: 20 additions & 1 deletion Modules/_posixsubprocess.c
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ reset_signal_handlers(const sigset_t *child_sigmask)
* required by POSIX but not supported natively on Linux. Another reason to
* avoid this family of functions is that sharing an address space between
* processes running with different privileges is inherently insecure.
* See bpo-35823 for further discussion and references.
* See https://bugs.python.org/issue35823 for discussion and references.
*
* In some C libraries, setrlimit() has the same thread list/signalling
* behavior since resource limits were per-thread attributes before
Expand Down Expand Up @@ -798,15 +798,28 @@ do_fork_exec(char *const exec_array[],
pid_t pid;

#ifdef VFORK_USABLE
PyThreadState *vfork_tstate_save = NULL;
if (child_sigmask) {
/* These are checked by our caller; verify them in debug builds. */
assert(uid == (uid_t)-1);
assert(gid == (gid_t)-1);
assert(extra_group_size < 0);
assert(preexec_fn == Py_None);

/* Drop the GIL so that other threads can continue execution while this
* thread in the parent remains blocked per vfork-semantics on the
* child's exec syscall outcome. Exec requires filesystem access which
* can take an arbitrarily long time. This addresses GH-104372.
*
* The vfork'ed child still runs in our address space. Per POSIX it
* must be limited to nothing but exec, but the Linux implementation
* is a little more usable. See the child_exec() comment.
*/
vfork_tstate_save = PyEval_SaveThread();
pid = vfork();
if (pid == (pid_t)-1) {
PyEval_RestoreThread(vfork_tstate_save);
vfork_tstate_save = NULL;
/* If vfork() fails, fall back to using fork(). When it isn't
* allowed in a process by the kernel, vfork can return -1
* with errno EINVAL. https://bugs.python.org/issue47151. */
Expand All @@ -819,6 +832,12 @@ do_fork_exec(char *const exec_array[],
}

if (pid != 0) {
// Parent process.
#ifdef VFORK_USABLE
if (vfork_tstate_save != NULL) {
PyEval_RestoreThread(vfork_tstate_save);
}
#endif
return pid;
}

Expand Down

0 comments on commit 2d54d8e

Please sign in to comment.