Skip to content

gh-121381 Remove subprocess._USE_VFORK escape hatch #121383

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

Merged
merged 7 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
25 changes: 5 additions & 20 deletions Doc/library/subprocess.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1561,41 +1561,26 @@ runtime):
Module which provides function to parse and escape command lines.


.. _disable_vfork:
.. _disable_posix_spawn:

Disabling use of ``vfork()`` or ``posix_spawn()``
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Disable use of ``posix_spawn()``
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

On Linux, :mod:`subprocess` defaults to using the ``vfork()`` system call
internally when it is safe to do so rather than ``fork()``. This greatly
improves performance.

If you ever encounter a presumed highly unusual situation where you need to
prevent ``vfork()`` from being used by Python, you can set the
:const:`subprocess._USE_VFORK` attribute to a false value.

::

subprocess._USE_VFORK = False # See CPython issue gh-NNNNNN.

Setting this has no impact on use of ``posix_spawn()`` which could use
``vfork()`` internally within its libc implementation. There is a similar
:const:`subprocess._USE_POSIX_SPAWN` attribute if you need to prevent use of
that.

::

subprocess._USE_POSIX_SPAWN = False # See CPython issue gh-NNNNNN.

It is safe to set these to false on any Python version. They will have no
effect on older versions when unsupported. Do not assume the attributes are
available to read. Despite their names, a true value does not indicate that the
It is safe to set this to false on any Python version. It will have no
effect on older or newer versions where unsupported. Do not assume the attribute
is available to read. Despite the name, a true value does not indicate the
corresponding function will be used, only that it may be.

Please file issues any time you have to use these private knobs with a way to
reproduce the issue you were seeing. Link to that issue from a comment in your
code.

.. versionadded:: 3.8 ``_USE_POSIX_SPAWN``
.. versionadded:: 3.11 ``_USE_VFORK``
3 changes: 1 addition & 2 deletions Lib/multiprocessing/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,8 +445,7 @@ def spawnv_passfds(path, args, passfds):
return _posixsubprocess.fork_exec(
args, [path], True, passfds, None, None,
-1, -1, -1, -1, -1, -1, errpipe_read, errpipe_write,
False, False, -1, None, None, None, -1, None,
subprocess._USE_VFORK)
False, False, -1, None, None, None, -1, None)
finally:
os.close(errpipe_read)
os.close(errpipe_write)
Expand Down
3 changes: 1 addition & 2 deletions Lib/subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,6 @@ def _use_posix_spawn():
# 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
_HAVE_POSIX_SPAWN_CLOSEFROM = hasattr(os, 'POSIX_SPAWN_CLOSEFROM')


Expand Down Expand Up @@ -1902,7 +1901,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
errpipe_read, errpipe_write,
restore_signals, start_new_session,
process_group, gid, gids, uid, umask,
preexec_fn, _USE_VFORK)
preexec_fn)
self._child_created = True
finally:
# be sure the FD is closed no matter what
Expand Down
6 changes: 3 additions & 3 deletions Lib/test/test_capi/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,15 @@ def __len__(self):
return 1
with self.assertRaisesRegex(TypeError, 'indexing'):
_posixsubprocess.fork_exec(
1,Z(),True,(1, 2),5,6,7,8,9,10,11,12,13,14,True,True,17,False,19,20,21,22,False)
1,Z(),True,(1, 2),5,6,7,8,9,10,11,12,13,14,True,True,17,False,19,20,21,22)
# Issue #15736: overflow in _PySequence_BytesToCharpArray()
class Z(object):
def __len__(self):
return sys.maxsize
def __getitem__(self, i):
return b'x'
self.assertRaises(MemoryError, _posixsubprocess.fork_exec,
1,Z(),True,(1, 2),5,6,7,8,9,10,11,12,13,14,True,True,17,False,19,20,21,22,False)
1,Z(),True,(1, 2),5,6,7,8,9,10,11,12,13,14,True,True,17,False,19,20,21,22)

@unittest.skipUnless(_posixsubprocess, '_posixsubprocess required for this test.')
def test_subprocess_fork_exec(self):
Expand All @@ -138,7 +138,7 @@ def __len__(self):

# Issue #15738: crash in subprocess_fork_exec()
self.assertRaises(TypeError, _posixsubprocess.fork_exec,
Z(),[b'1'],True,(1, 2),5,6,7,8,9,10,11,12,13,14,True,True,17,False,19,20,21,22,False)
Z(),[b'1'],True,(1, 2),5,6,7,8,9,10,11,12,13,14,True,True,17,False,19,20,21,22)

@unittest.skipIf(MISSING_C_DOCSTRINGS,
"Signature information for builtins requires docstrings")
Expand Down
22 changes: 1 addition & 21 deletions Lib/test/test_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -3278,7 +3278,7 @@ def __int__(self):
1, 2, 3, 4,
True, True, 0,
None, None, None, -1,
None, True)
None)
self.assertIn('fds_to_keep', str(c.exception))
finally:
if not gc_enabled:
Expand Down Expand Up @@ -3413,25 +3413,6 @@ def __del__(self):
self.assertEqual(out.strip(), b"OK")
self.assertIn(b"preexec_fn not supported at interpreter shutdown", err)

@unittest.skipIf(not sysconfig.get_config_var("HAVE_VFORK"),
"vfork() not enabled by configure.")
@mock.patch("subprocess._fork_exec")
@mock.patch("subprocess._USE_POSIX_SPAWN", new=False)
def test__use_vfork(self, mock_fork_exec):
self.assertTrue(subprocess._USE_VFORK) # The default value regardless.
mock_fork_exec.side_effect = RuntimeError("just testing args")
with self.assertRaises(RuntimeError):
subprocess.run([sys.executable, "-c", "pass"])
mock_fork_exec.assert_called_once()
# NOTE: These assertions are *ugly* as they require the last arg
# to remain the have_vfork boolean. We really need to refactor away
# from the giant "wall of args" internal C extension API.
self.assertTrue(mock_fork_exec.call_args.args[-1])
with mock.patch.object(subprocess, '_USE_VFORK', False):
with self.assertRaises(RuntimeError):
subprocess.run([sys.executable, "-c", "pass"])
self.assertFalse(mock_fork_exec.call_args_list[-1].args[-1])

@unittest.skipIf(not sysconfig.get_config_var("HAVE_VFORK"),
"vfork() not enabled by configure.")
@unittest.skipIf(sys.platform != "linux", "Linux only, requires strace.")
Expand Down Expand Up @@ -3478,7 +3459,6 @@ def test_vfork_used_when_expected(self):
# Test that each individual thing that would disable the use of vfork
# actually disables it.
for sub_name, preamble, sp_kwarg, expect_permission_error in (
("!use_vfork", "subprocess._USE_VFORK = False", "", False),
("preexec", "", "preexec_fn=lambda: None", False),
("setgid", "", f"group={os.getgid()}", True),
("setuid", "", f"user={os.getuid()}", True),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Remove ``subprocess._USE_VFORK`` escape hatch code and documentation.
It was added just in case, and doesn't have any known cases that require it.
7 changes: 3 additions & 4 deletions Modules/_posixsubprocess.c
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,6 @@ _posixsubprocess.fork_exec as subprocess_fork_exec
uid as uid_object: object
child_umask: int
preexec_fn: object
allow_vfork: bool
/

Spawn a fresh new child process.
Expand Down Expand Up @@ -1014,8 +1013,8 @@ subprocess_fork_exec_impl(PyObject *module, PyObject *process_args,
pid_t pgid_to_set, PyObject *gid_object,
PyObject *extra_groups_packed,
PyObject *uid_object, int child_umask,
PyObject *preexec_fn, int allow_vfork)
/*[clinic end generated code: output=7ee4f6ee5cf22b5b input=51757287ef266ffa]*/
PyObject *preexec_fn)
/*[clinic end generated code: output=288464dc56e373c7 input=f311c3bcb5dd55c8]*/
{
PyObject *converted_args = NULL, *fast_args = NULL;
PyObject *preexec_fn_args_tuple = NULL;
Expand Down Expand Up @@ -1218,7 +1217,7 @@ subprocess_fork_exec_impl(PyObject *module, PyObject *process_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 && allow_vfork &&
if (preexec_fn == Py_None &&
uid == (uid_t)-1 && gid == (gid_t)-1 && extra_group_size < 0) {
/* Block all signals to ensure that no signal handlers are run in the
* child process while it shares memory with us. Note that signals
Expand Down
15 changes: 5 additions & 10 deletions Modules/clinic/_posixsubprocess.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading