Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
May 25, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions Doc/library/subprocess.rst
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,13 @@ underlying :class:`Popen` interface can be used directly.
and combine both streams into one, use ``stdout=PIPE`` and ``stderr=STDOUT``
instead of *capture_output*.

The *timeout* argument is passed to :meth:`Popen.communicate`. If the timeout
expires, the child process will be killed and waited for. The
:exc:`TimeoutExpired` exception will be re-raised after the child process
has terminated.
A *timeout* may be specified in seconds, it is internally passed on to
:meth:`Popen.communicate`. If the timeout expires, the child process will be
killed and waited for. The :exc:`TimeoutExpired` exception will be
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
subprocess's stdin. If used it must be a byte sequence, or a string if
Expand Down Expand Up @@ -734,7 +737,7 @@ arguments.
code.

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.

Exceptions defined in this module all inherit from :exc:`SubprocessError`.
Expand Down
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.
18 changes: 17 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,14 +798,29 @@ do_fork_exec(char *const exec_array[],
pid_t pid;

#ifdef VFORK_USABLE
PyThreadState *vfork_tstate_save;
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 does filesystem access which
* can take an arbitrarily long time. This addresses GH-104372.
*
gpshead marked this conversation as resolved.
Show resolved Hide resolved
* 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 != 0) {
// Not in the child process, reacquire the GIL.
PyEval_RestoreThread(vfork_tstate_save);
}
if (pid == (pid_t)-1) {
/* If vfork() fails, fall back to using fork(). When it isn't
* allowed in a process by the kernel, vfork can return -1
Expand All @@ -819,6 +834,7 @@ do_fork_exec(char *const exec_array[],
}

if (pid != 0) {
// Parent process.
return pid;
}

Expand Down