Skip to content

Commit

Permalink
[3.10] gh-91401: Conservative backport of subprocess._USE_VFORK (#9…
Browse files Browse the repository at this point in the history
…1932)

This does not alter the `_posixsubprocess.fork_exec()` private API to
avoid issues for anyone relying on that (bad idea) or for anyone who's
`subprocess.py` and `_posixsubprocess.so` upgrades may not become
visible to existing Python 3.10 processes at the same time.

Backports the concept of cd5726f.

Provides a fail-safe way to disable vfork for #91401.

I didn't backport the documentation as I don't actually expect this to be used and `.. versionadded: 3.10.5` always looks weird in docs. It's being done more to have a fail-safe in place for people just in case.
  • Loading branch information
gpshead committed May 1, 2022
1 parent e7de543 commit ea1eba0
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 2 deletions.
3 changes: 3 additions & 0 deletions Lib/subprocess.py
Expand Up @@ -691,7 +691,10 @@ def _use_posix_spawn():
return False


# These are primarily fail-safe knobs for negatives. A True value does not
# guarantee the given libc/syscall API will be used.
_USE_POSIX_SPAWN = _use_posix_spawn()
_USE_VFORK = True


class Popen:
Expand Down
22 changes: 22 additions & 0 deletions Lib/test/test_subprocess.py
Expand Up @@ -1702,6 +1702,28 @@ def test_run_with_shell_timeout_and_capture_output(self):
msg="TimeoutExpired was delayed! Bad traceback:\n```\n"
f"{stacks}```")

@unittest.skipIf(not sysconfig.get_config_var("HAVE_VFORK"),
"vfork() not enabled by configure.")
def test__use_vfork(self):
# Attempts code coverage within _posixsubprocess.c on the code that
# probes the subprocess module for the existence and value of this
# attribute in 3.10.5.
self.assertTrue(subprocess._USE_VFORK) # The default value regardless.
with mock.patch.object(subprocess, "_USE_VFORK", False):
self.assertEqual(self.run_python("pass").returncode, 0,
msg="False _USE_VFORK failed")

class RaisingBool:
def __bool__(self):
raise RuntimeError("force PyObject_IsTrue to return -1")

with mock.patch.object(subprocess, "_USE_VFORK", RaisingBool()):
self.assertEqual(self.run_python("pass").returncode, 0,
msg="odd bool()-error _USE_VFORK failed")
del subprocess._USE_VFORK
self.assertEqual(self.run_python("pass").returncode, 0,
msg="lack of a _USE_VFORK attribute failed")


def _get_test_grp_name():
for name_group in ('staff', 'nogroup', 'grp', 'nobody', 'nfsnobody'):
Expand Down
@@ -0,0 +1,5 @@
Provide a fail-safe way to disable :mod:`subprocess` use of ``vfork()`` via
a private ``subprocess._USE_VFORK`` attribute. While there is currently no
known need for this, if you find a need please only set it to ``False``.
File a CPython issue as to why you needed it and link to that from a
comment in your code. This attribute is documented as a footnote in 3.11.
27 changes: 25 additions & 2 deletions Modules/_posixsubprocess.c
Expand Up @@ -936,8 +936,31 @@ subprocess_fork_exec(PyObject *module, PyObject *args)
#ifdef VFORK_USABLE
/* Use vfork() only if it's safe. See the comment above child_exec(). */
sigset_t old_sigs;
if (preexec_fn == Py_None &&
!call_setuid && !call_setgid && !call_setgroups) {
int allow_vfork;
if (preexec_fn == Py_None) {
allow_vfork = 1; /* 3.10.0 behavior */
PyObject *subprocess_module = PyImport_ImportModule("subprocess");
if (subprocess_module != NULL) {
PyObject *allow_vfork_obj = PyObject_GetAttrString(
subprocess_module, "_USE_VFORK");
Py_DECREF(subprocess_module);
if (allow_vfork_obj != NULL) {
allow_vfork = PyObject_IsTrue(allow_vfork_obj);
Py_DECREF(allow_vfork_obj);
if (allow_vfork < 0) {
PyErr_Clear(); /* Bad _USE_VFORK attribute. */
allow_vfork = 1; /* 3.10.0 behavior */
}
} else {
PyErr_Clear(); /* No _USE_VFORK attribute. */
}
} else {
PyErr_Clear(); /* no subprocess module? suspicious; don't care. */
}
} else {
allow_vfork = 0;
}
if (allow_vfork && !call_setuid && !call_setgid && !call_setgroups) {
/* Block all signals to ensure that no signal handlers are run in the
* child process while it shares memory with us. Note that signals
* used internally by C libraries won't be blocked by
Expand Down

0 comments on commit ea1eba0

Please sign in to comment.