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

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 #104372.
This commit is contained in:
Gregory P. Smith 2023-05-25 13:14:09 -07:00 committed by GitHub
parent 08888650aa
commit d08679212d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 31 additions and 6 deletions

View file

@ -57,10 +57,13 @@ underlying :class:`Popen` interface can be used directly.
and combine both streams into one, use ``stdout=PIPE`` and ``stderr=STDOUT`` and combine both streams into one, use ``stdout=PIPE`` and ``stderr=STDOUT``
instead of *capture_output*. instead of *capture_output*.
The *timeout* argument is passed to :meth:`Popen.communicate`. If the timeout A *timeout* may be specified in seconds, it is internally passed on to
expires, the child process will be killed and waited for. The :meth:`Popen.communicate`. If the timeout expires, the child process will be
:exc:`TimeoutExpired` exception will be re-raised after the child process killed and waited for. The :exc:`TimeoutExpired` exception will be
has terminated. re-raised after the child process has terminated. The initial process
creation itself cannot be interrupted on many platform APIs so you are not
guaranteed to see a timeout exception until at least after however long
process creation takes.
The *input* argument is passed to :meth:`Popen.communicate` and thus to the The *input* argument is passed to :meth:`Popen.communicate` and thus to the
subprocess's stdin. If used it must be a byte sequence, or a string if subprocess's stdin. If used it must be a byte sequence, or a string if
@ -734,7 +737,7 @@ arguments.
code. code.
All of the functions and methods that accept a *timeout* parameter, such as All of the functions and methods that accept a *timeout* parameter, such as
:func:`call` and :meth:`Popen.communicate` will raise :exc:`TimeoutExpired` if :func:`run` and :meth:`Popen.communicate` will raise :exc:`TimeoutExpired` if
the timeout expires before the process exits. the timeout expires before the process exits.
Exceptions defined in this module all inherit from :exc:`SubprocessError`. Exceptions defined in this module all inherit from :exc:`SubprocessError`.

View file

@ -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.

View file

@ -559,7 +559,7 @@ reset_signal_handlers(const sigset_t *child_sigmask)
* required by POSIX but not supported natively on Linux. Another reason to * required by POSIX but not supported natively on Linux. Another reason to
* avoid this family of functions is that sharing an address space between * avoid this family of functions is that sharing an address space between
* processes running with different privileges is inherently insecure. * 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 * In some C libraries, setrlimit() has the same thread list/signalling
* behavior since resource limits were per-thread attributes before * behavior since resource limits were per-thread attributes before
@ -798,6 +798,7 @@ do_fork_exec(char *const exec_array[],
pid_t pid; pid_t pid;
#ifdef VFORK_USABLE #ifdef VFORK_USABLE
PyThreadState *vfork_tstate_save;
if (child_sigmask) { if (child_sigmask) {
/* These are checked by our caller; verify them in debug builds. */ /* These are checked by our caller; verify them in debug builds. */
assert(uid == (uid_t)-1); assert(uid == (uid_t)-1);
@ -805,7 +806,22 @@ do_fork_exec(char *const exec_array[],
assert(extra_group_size < 0); assert(extra_group_size < 0);
assert(preexec_fn == Py_None); 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 does 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 - The child
* MUST NOT re-acquire the GIL.
*/
vfork_tstate_save = PyEval_SaveThread();
pid = vfork(); pid = vfork();
if (pid != 0) {
// Not in the child process, reacquire the GIL.
PyEval_RestoreThread(vfork_tstate_save);
}
if (pid == (pid_t)-1) { if (pid == (pid_t)-1) {
/* If vfork() fails, fall back to using fork(). When it isn't /* If vfork() fails, fall back to using fork(). When it isn't
* allowed in a process by the kernel, vfork can return -1 * allowed in a process by the kernel, vfork can return -1
@ -819,6 +835,7 @@ do_fork_exec(char *const exec_array[],
} }
if (pid != 0) { if (pid != 0) {
// Parent process.
return pid; return pid;
} }