gh-113117: Support posix_spawn in subprocess.Popen with close_fds=True (#113118)

Add support for `os.POSIX_SPAWN_CLOSEFROM` and
`posix_spawn_file_actions_addclosefrom_np` and have the `subprocess` module use
them when available.  This means `posix_spawn` can now be used in the default
`close_fds=True` situation on many platforms.

Co-authored-by: Gregory P. Smith [Google LLC] <greg@krypto.org>
This commit is contained in:
Jakub Kulík 2023-12-17 22:34:57 +01:00 committed by GitHub
parent 32d87a8899
commit 2b93f52242
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 91 additions and 5 deletions

View File

@ -4601,10 +4601,17 @@ written in Python, such as a mail server's external command delivery program.
Performs ``os.dup2(fd, new_fd)``.
.. data:: POSIX_SPAWN_CLOSEFROM
(``os.POSIX_SPAWN_CLOSEFROM``, *fd*)
Performs ``os.closerange(fd, INF)``.
These tuples correspond to the C library
:c:func:`!posix_spawn_file_actions_addopen`,
:c:func:`!posix_spawn_file_actions_addclose`, and
:c:func:`!posix_spawn_file_actions_adddup2` API calls used to prepare
:c:func:`!posix_spawn_file_actions_addclose`,
:c:func:`!posix_spawn_file_actions_adddup2`, and
:c:func:`!posix_spawn_file_actions_addclosefrom_np` API calls used to prepare
for the :c:func:`!posix_spawn` call itself.
The *setpgroup* argument will set the process group of the child to the value
@ -4649,6 +4656,10 @@ written in Python, such as a mail server's external command delivery program.
.. versionchanged:: 3.13
*env* parameter accepts ``None``.
.. versionchanged:: 3.13
``os.POSIX_SPAWN_CLOSEFROM`` is available on platforms where
:c:func:`!posix_spawn_file_actions_addclosefrom_np` exists.
.. availability:: Unix, not Emscripten, not WASI.
.. function:: posix_spawnp(path, argv, env, *, file_actions=None, \

View File

@ -293,6 +293,11 @@ os
process use the current process environment.
(Contributed by Jakub Kulik in :gh:`113119`.)
* :func:`os.posix_spawn` gains an :attr:`os.POSIX_SPAWN_CLOSEFROM` attribute for
use in ``file_actions=`` on platforms that support
:c:func:`!posix_spawn_file_actions_addclosefrom_np`.
(Contributed by Jakub Kulik in :gh:`113117`.)
pathlib
-------
@ -342,6 +347,21 @@ sqlite3
object is not :meth:`closed <sqlite3.Connection.close>` explicitly.
(Contributed by Erlend E. Aasland in :gh:`105539`.)
subprocess
----------
* The :mod:`subprocess` module now uses the :func:`os.posix_spawn` function in
more situations. Notably in the default case of ``close_fds=True`` on more
recent versions of platforms including Linux, FreeBSD, and Solaris where the
C library provides :c:func:`!posix_spawn_file_actions_addclosefrom_np`.
On Linux this should perform similar to our existing Linux :c:func:`!vfork`
based code. A private control knob :attr:`!subprocess._USE_POSIX_SPAWN` can
be set to ``False`` if you need to force :mod:`subprocess` not to ever use
:func:`os.posix_spawn`. Please report your reason and platform details in
the CPython issue tracker if you set this so that we can improve our API
selection logic for everyone.
(Contributed by Jakub Kulik in :gh:`113117`.)
sys
---
@ -415,6 +435,12 @@ Optimizations
* :func:`textwrap.indent` is now ~30% faster than before for large input.
(Contributed by Inada Naoki in :gh:`107369`.)
* The :mod:`subprocess` module uses :func:`os.posix_spawn` in more situations
including the default where ``close_fds=True`` on many modern platforms. This
should provide a noteworthy performance increase launching processes on
FreeBSD and Solaris. See the ``subprocess`` section above for details.
(Contributed by Jakub Kulik in :gh:`113117`.)
Deprecated
==========

View File

@ -748,6 +748,7 @@ def _use_posix_spawn():
# 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')
class Popen:
@ -1751,7 +1752,7 @@ class Popen:
errread, errwrite)
def _posix_spawn(self, args, executable, env, restore_signals,
def _posix_spawn(self, args, executable, env, restore_signals, close_fds,
p2cread, p2cwrite,
c2pread, c2pwrite,
errread, errwrite):
@ -1777,6 +1778,10 @@ class Popen:
):
if fd != -1:
file_actions.append((os.POSIX_SPAWN_DUP2, fd, fd2))
if close_fds:
file_actions.append((os.POSIX_SPAWN_CLOSEFROM, 3))
if file_actions:
kwargs['file_actions'] = file_actions
@ -1824,7 +1829,7 @@ class Popen:
if (_USE_POSIX_SPAWN
and os.path.dirname(executable)
and preexec_fn is None
and not close_fds
and (not close_fds or _HAVE_POSIX_SPAWN_CLOSEFROM)
and not pass_fds
and cwd is None
and (p2cread == -1 or p2cread > 2)
@ -1836,7 +1841,7 @@ class Popen:
and gids is None
and uid is None
and umask < 0):
self._posix_spawn(args, executable, env, restore_signals,
self._posix_spawn(args, executable, env, restore_signals, close_fds,
p2cread, p2cwrite,
c2pread, c2pwrite,
errread, errwrite)

View File

@ -3348,6 +3348,7 @@ class POSIXProcessTestCase(BaseTestCase):
@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")
@ -3366,9 +3367,13 @@ class POSIXProcessTestCase(BaseTestCase):
@unittest.skipIf(not sysconfig.get_config_var("HAVE_VFORK"),
"vfork() not enabled by configure.")
@unittest.skipIf(sys.platform != "linux", "Linux only, requires strace.")
@mock.patch("subprocess._USE_POSIX_SPAWN", new=False)
def test_vfork_used_when_expected(self):
# This is a performance regression test to ensure we default to using
# vfork() when possible.
# Technically this test could pass when posix_spawn is used as well
# because libc tends to implement that internally using vfork. But
# that'd just be testing a libc+kernel implementation detail.
strace_binary = "/usr/bin/strace"
# The only system calls we are interested in.
strace_filter = "--trace=clone,clone2,clone3,fork,vfork,exit,exit_group"

View File

@ -0,0 +1,4 @@
The :mod:`subprocess` module can now use the :func:`os.posix_spawn` function
with ``close_fds=True`` on platforms where
``posix_spawn_file_actions_addclosefrom_np`` is available.
Patch by Jakub Kulik.

View File

@ -6834,6 +6834,9 @@ enum posix_spawn_file_actions_identifier {
POSIX_SPAWN_OPEN,
POSIX_SPAWN_CLOSE,
POSIX_SPAWN_DUP2
#ifdef HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCLOSEFROM_NP
,POSIX_SPAWN_CLOSEFROM
#endif
};
#if defined(HAVE_SCHED_SETPARAM) || defined(HAVE_SCHED_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDPARAM)
@ -7074,6 +7077,24 @@ parse_file_actions(PyObject *file_actions,
}
break;
}
#ifdef HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCLOSEFROM_NP
case POSIX_SPAWN_CLOSEFROM: {
int fd;
if (!PyArg_ParseTuple(file_action, "Oi"
";A closefrom file_action tuple must have 2 elements",
&tag_obj, &fd))
{
goto fail;
}
errno = posix_spawn_file_actions_addclosefrom_np(file_actionsp,
fd);
if (errno) {
posix_error();
goto fail;
}
break;
}
#endif
default: {
PyErr_SetString(PyExc_TypeError,
"Unknown file_actions identifier");
@ -16774,6 +16795,9 @@ all_ins(PyObject *m)
if (PyModule_AddIntConstant(m, "POSIX_SPAWN_OPEN", POSIX_SPAWN_OPEN)) return -1;
if (PyModule_AddIntConstant(m, "POSIX_SPAWN_CLOSE", POSIX_SPAWN_CLOSE)) return -1;
if (PyModule_AddIntConstant(m, "POSIX_SPAWN_DUP2", POSIX_SPAWN_DUP2)) return -1;
#ifdef HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCLOSEFROM_NP
if (PyModule_AddIntMacro(m, POSIX_SPAWN_CLOSEFROM)) return -1;
#endif
#endif
#if defined(HAVE_SPAWNV) || defined (HAVE_RTPSPAWN)

6
configure generated vendored
View File

@ -17779,6 +17779,12 @@ if test "x$ac_cv_func_posix_spawnp" = xyes
then :
printf "%s\n" "#define HAVE_POSIX_SPAWNP 1" >>confdefs.h
fi
ac_fn_c_check_func "$LINENO" "posix_spawn_file_actions_addclosefrom_np" "ac_cv_func_posix_spawn_file_actions_addclosefrom_np"
if test "x$ac_cv_func_posix_spawn_file_actions_addclosefrom_np" = xyes
then :
printf "%s\n" "#define HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCLOSEFROM_NP 1" >>confdefs.h
fi
ac_fn_c_check_func "$LINENO" "pread" "ac_cv_func_pread"
if test "x$ac_cv_func_pread" = xyes

View File

@ -4757,6 +4757,7 @@ AC_CHECK_FUNCS([ \
lockf lstat lutimes madvise mbrtowc memrchr mkdirat mkfifo mkfifoat \
mknod mknodat mktime mmap mremap nice openat opendir pathconf pause pipe \
pipe2 plock poll posix_fadvise posix_fallocate posix_spawn posix_spawnp \
posix_spawn_file_actions_addclosefrom_np \
pread preadv preadv2 pthread_condattr_setclock pthread_init pthread_kill \
pwrite pwritev pwritev2 readlink readlinkat readv realpath renameat \
rtpSpawn sched_get_priority_max sched_rr_get_interval sched_setaffinity \

View File

@ -905,6 +905,10 @@
/* Define to 1 if you have the `posix_spawnp' function. */
#undef HAVE_POSIX_SPAWNP
/* Define to 1 if you have the `posix_spawn_file_actions_addclosefrom_np'
function. */
#undef HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCLOSEFROM_NP
/* Define to 1 if you have the `pread' function. */
#undef HAVE_PREAD