[Python-checkins] [3.10] gh-91401: Conservative backport of `subprocess._USE_VFORK` (#91932)
gpshead
webhook-mailer at python.org
Sun May 1 19:09:55 EDT 2022
https://github.com/python/cpython/commit/ea1eba03e7e8401d5acf8b30b56b41faa209e8c6
commit: ea1eba03e7e8401d5acf8b30b56b41faa209e8c6
branch: 3.10
author: Gregory P. Smith <greg at krypto.org>
committer: gpshead <greg at krypto.org>
date: 2022-05-01T16:09:50-07:00
summary:
[3.10] gh-91401: Conservative backport of `subprocess._USE_VFORK` (#91932)
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 cd5726fe674eaff442510eeb6c75628858be9e9f.
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.
files:
A Misc/NEWS.d/next/Library/2022-04-26-00-10-06.gh-issue-91401.mddRC8.rst
M Lib/subprocess.py
M Lib/test/test_subprocess.py
M Modules/_posixsubprocess.c
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index ccb46a6337b3d..a414321b9d197 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -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:
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 7bb049296912b..b91791a02a2e5 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -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'):
diff --git a/Misc/NEWS.d/next/Library/2022-04-26-00-10-06.gh-issue-91401.mddRC8.rst b/Misc/NEWS.d/next/Library/2022-04-26-00-10-06.gh-issue-91401.mddRC8.rst
new file mode 100644
index 0000000000000..964a54f8935c2
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2022-04-26-00-10-06.gh-issue-91401.mddRC8.rst
@@ -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.
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
index 18a81c6ed8b35..b852ad71d7225 100644
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -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
More information about the Python-checkins
mailing list