Skip to content

Commit 6ba1234

Browse files
authored
[3.11] gh-104372: Drop the GIL around the vfork() call. (#104782) (#104958)
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. (cherry picked from commit d086792)
1 parent b4784b0 commit 6ba1234

File tree

3 files changed

+32
-7
lines changed

3 files changed

+32
-7
lines changed

Doc/library/subprocess.rst

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,13 @@ underlying :class:`Popen` interface can be used directly.
5757
and combine both streams into one, use ``stdout=PIPE`` and ``stderr=STDOUT``
5858
instead of *capture_output*.
5959

60-
The *timeout* argument is passed to :meth:`Popen.communicate`. If the timeout
61-
expires, the child process will be killed and waited for. The
62-
:exc:`TimeoutExpired` exception will be re-raised after the child process
63-
has terminated.
60+
A *timeout* may be specified in seconds, it is internally passed on to
61+
:meth:`Popen.communicate`. If the timeout expires, the child process will be
62+
killed and waited for. The :exc:`TimeoutExpired` exception will be
63+
re-raised after the child process has terminated. The initial process
64+
creation itself cannot be interrupted on many platform APIs so you are not
65+
guaranteed to see a timeout exception until at least after however long
66+
process creation takes.
6467

6568
The *input* argument is passed to :meth:`Popen.communicate` and thus to the
6669
subprocess's stdin. If used it must be a byte sequence, or a string if
@@ -736,7 +739,7 @@ arguments.
736739
code.
737740

738741
All of the functions and methods that accept a *timeout* parameter, such as
739-
:func:`call` and :meth:`Popen.communicate` will raise :exc:`TimeoutExpired` if
742+
:func:`run` and :meth:`Popen.communicate` will raise :exc:`TimeoutExpired` if
740743
the timeout expires before the process exits.
741744

742745
Exceptions defined in this module all inherit from :exc:`SubprocessError`.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
On Linux where :mod:`subprocess` can use the ``vfork()`` syscall for faster
2+
spawning, prevent the parent process from blocking other threads by dropping
3+
the GIL while it waits for the vfork'ed child process ``exec()`` outcome.
4+
This prevents spawning a binary from a slow filesystem from blocking the
5+
rest of the application.

Modules/_posixsubprocess.c

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ reset_signal_handlers(const sigset_t *child_sigmask)
537537
* required by POSIX but not supported natively on Linux. Another reason to
538538
* avoid this family of functions is that sharing an address space between
539539
* processes running with different privileges is inherently insecure.
540-
* See bpo-35823 for further discussion and references.
540+
* See https://bugs.python.org/issue35823 for discussion and references.
541541
*
542542
* In some C libraries, setrlimit() has the same thread list/signalling
543543
* behavior since resource limits were per-thread attributes before
@@ -774,15 +774,31 @@ do_fork_exec(char *const exec_array[],
774774
pid_t pid;
775775

776776
#ifdef VFORK_USABLE
777+
PyThreadState *vfork_tstate_save;
777778
if (child_sigmask) {
778779
/* These are checked by our caller; verify them in debug builds. */
779780
assert(!call_setuid);
780781
assert(!call_setgid);
781782
assert(!call_setgroups);
782783
assert(preexec_fn == Py_None);
783784

785+
/* Drop the GIL so that other threads can continue execution while this
786+
* thread in the parent remains blocked per vfork-semantics on the
787+
* child's exec syscall outcome. Exec does filesystem access which
788+
* can take an arbitrarily long time. This addresses GH-104372.
789+
*
790+
* The vfork'ed child still runs in our address space. Per POSIX it
791+
* must be limited to nothing but exec, but the Linux implementation
792+
* is a little more usable. See the child_exec() comment - The child
793+
* MUST NOT re-acquire the GIL.
794+
*/
795+
vfork_tstate_save = PyEval_SaveThread();
784796
pid = vfork();
785-
if (pid == -1) {
797+
if (pid != 0) {
798+
// Not in the child process, reacquire the GIL.
799+
PyEval_RestoreThread(vfork_tstate_save);
800+
}
801+
if (pid == (pid_t)-1) {
786802
/* If vfork() fails, fall back to using fork(). When it isn't
787803
* allowed in a process by the kernel, vfork can return -1
788804
* with errno EINVAL. https://bugs.python.org/issue47151. */
@@ -795,6 +811,7 @@ do_fork_exec(char *const exec_array[],
795811
}
796812

797813
if (pid != 0) {
814+
// Parent process.
798815
return pid;
799816
}
800817

0 commit comments

Comments
 (0)