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>

9 files changed, 91 insertions, 5 deletions

diff --git a/Doc/library/os.rst b/Doc/library/os.rst
index a079f1f..1138cc1 100644
--- a/Doc/library/os.rst
+++ b/Doc/library/os.rst
@@ -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, \
diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst
index 4af0235..2c869cb 100644
--- a/Doc/whatsnew/3.13.rst
+++ b/Doc/whatsnew/3.13.rst
@@ -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
 ==========
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 1919ea4..d5bd9a9 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -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)
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 5eeea54..6d3228b 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -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"
diff --git a/Misc/NEWS.d/next/Library/2023-12-16-10-58-34.gh-issue-113117.0zF7bH.rst b/Misc/NEWS.d/next/Library/2023-12-16-10-58-34.gh-issue-113117.0zF7bH.rst
new file mode 100644
index 0000000..718226a
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2023-12-16-10-58-34.gh-issue-113117.0zF7bH.rst
@@ -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.
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 2dc5d7d..8ffe0f5 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -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)
diff --git a/configure b/configure
index 668a0ef..7e50abc 100755
--- a/configure
+++ b/configure
@@ -17780,6 +17780,12 @@ 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
 then :
diff --git a/configure.ac b/configure.ac
index 020553a..e064848 100644
--- a/configure.ac
+++ b/configure.ac
@@ -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 \
diff --git a/pyconfig.h.in b/pyconfig.h.in
index 9c429c0..d8a9f68 100644
--- a/pyconfig.h.in
+++ b/pyconfig.h.in
@@ -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