issue6559: Adds a pass_fds parameter to subprocess.Popen that allows the caller

to list exactly which file descriptors should be kept open.

2 files changed, 65 insertions, 15 deletions

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index ca67012..949c30a 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -608,7 +608,8 @@ class Popen(object):
                  preexec_fn=None, close_fds=None, shell=False,
                  cwd=None, env=None, universal_newlines=False,
                  startupinfo=None, creationflags=0,
-                 restore_signals=True, start_new_session=False):
+                 restore_signals=True, start_new_session=False,
+                 pass_fds=()):
         """Create new Popen instance."""
         _cleanup()
 
@@ -644,6 +645,9 @@ class Popen(object):
                 raise ValueError("creationflags is only supported on Windows "
                                  "platforms")
 
+        if pass_fds and not close_fds:
+            raise ValueError("pass_fds requires close_fds=True.")
+
         self.stdin = None
         self.stdout = None
         self.stderr = None
@@ -671,7 +675,7 @@ class Popen(object):
          errread, errwrite) = self._get_handles(stdin, stdout, stderr)
 
         self._execute_child(args, executable, preexec_fn, close_fds,
-                            cwd, env, universal_newlines,
+                            pass_fds, cwd, env, universal_newlines,
                             startupinfo, creationflags, shell,
                             p2cread, p2cwrite,
                             c2pread, c2pwrite,
@@ -848,7 +852,7 @@ class Popen(object):
 
 
         def _execute_child(self, args, executable, preexec_fn, close_fds,
-                           cwd, env, universal_newlines,
+                           pass_fds, cwd, env, universal_newlines,
                            startupinfo, creationflags, shell,
                            p2cread, p2cwrite,
                            c2pread, c2pwrite,
@@ -856,6 +860,8 @@ class Popen(object):
                            unused_restore_signals, unused_start_new_session):
             """Execute program (MS Windows version)"""
 
+            assert not pass_fds, "pass_fds not yet supported on Windows"
+
             if not isinstance(args, str):
                 args = list2cmdline(args)
 
@@ -1075,8 +1081,19 @@ class Popen(object):
             os.closerange(but + 1, MAXFD)
 
 
+        def _close_all_but_a_sorted_few_fds(self, fds_to_keep):
+            # precondition: fds_to_keep must be sorted and unique
+            start_fd = 3
+            for fd in fds_to_keep:
+                if fd > start_fd:
+                    os.closerange(start_fd, fd)
+                    start_fd = fd + 1
+            if start_fd <= MAXFD:
+                os.closerange(start_fd, MAXFD)
+
+
         def _execute_child(self, args, executable, preexec_fn, close_fds,
-                           cwd, env, universal_newlines,
+                           pass_fds, cwd, env, universal_newlines,
                            startupinfo, creationflags, shell,
                            p2cread, p2cwrite,
                            c2pread, c2pwrite,
@@ -1124,9 +1141,11 @@ class Popen(object):
                             executable_list = tuple(
                                 os.path.join(os.fsencode(dir), executable)
                                 for dir in os.get_exec_path(env))
+                        fds_to_keep = set(pass_fds)
+                        fds_to_keep.add(errpipe_write)
                         self.pid = _posixsubprocess.fork_exec(
                                 args, executable_list,
-                                close_fds, cwd, env_list,
+                                close_fds, sorted(fds_to_keep), cwd, env_list,
                                 p2cread, p2cwrite, c2pread, c2pwrite,
                                 errread, errwrite,
                                 errpipe_read, errpipe_write,
@@ -1180,7 +1199,14 @@ class Popen(object):
 
                                 # Close all other fds, if asked for
                                 if close_fds:
-                                    self._close_fds(but=errpipe_write)
+                                    if pass_fds:
+                                        fds_to_keep = set(pass_fds)
+                                        fds_to_keep.add(errpipe_write)
+                                        self._close_all_but_a_sorted_few_fds(
+                                                sorted(fds_to_keep))
+                                    else:
+                                        self._close_fds(but=errpipe_write)
+
 
                                 if cwd is not None:
                                     os.chdir(cwd)
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
index 15c3dc4..36da2e5 100644
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -42,7 +42,8 @@ static void child_exec(char *const exec_array[],
                        int errread, int errwrite,
                        int errpipe_read, int errpipe_write,
                        int close_fds, int restore_signals,
-                       int call_setsid,
+                       int call_setsid, Py_ssize_t num_fds_to_keep,
+                       PyObject *py_fds_to_keep,
                        PyObject *preexec_fn,
                        PyObject *preexec_fn_args_tuple)
 {
@@ -91,11 +92,28 @@ static void child_exec(char *const exec_array[],
     /* close() is intentionally not checked for errors here as we are closing */
     /* a large range of fds, some of which may be invalid. */
     if (close_fds) {
-        for (fd_num = 3; fd_num < errpipe_write; ++fd_num) {
-            close(fd_num);
+        Py_ssize_t keep_seq_idx;
+        int start_fd = 3;
+        for (keep_seq_idx = 0; keep_seq_idx < num_fds_to_keep; ++keep_seq_idx) {
+            PyObject* py_keep_fd = PySequence_Fast_GET_ITEM(py_fds_to_keep,
+                                                            keep_seq_idx);
+            int keep_fd = PyLong_AsLong(py_keep_fd);
+            if (keep_fd < 0) {  /* Negative number, overflow or not a Long. */
+                err_msg = "bad value in fds_to_keep.";
+                errno = 0;  /* We don't want to report an OSError. */
+                goto error;
+            }
+            if (keep_fd <= start_fd)
+                continue;
+            for (fd_num = start_fd; fd_num < keep_fd; ++fd_num) {
+                close(fd_num);
+            }
+            start_fd = keep_fd + 1;
         }
-        for (fd_num = errpipe_write+1; fd_num < max_fd; ++fd_num) {
-            close(fd_num);
+        if (start_fd <= max_fd) {
+            for (fd_num = start_fd; fd_num < max_fd; ++fd_num) {
+                close(fd_num);
+            }
         }
     }
 
@@ -170,7 +188,7 @@ static PyObject *
 subprocess_fork_exec(PyObject* self, PyObject *args)
 {
     PyObject *gc_module = NULL;
-    PyObject *executable_list, *py_close_fds;
+    PyObject *executable_list, *py_close_fds, *py_fds_to_keep;
     PyObject *env_list, *preexec_fn;
     PyObject *process_args, *converted_args = NULL, *fast_args = NULL;
     PyObject *preexec_fn_args_tuple = NULL;
@@ -182,11 +200,11 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
     pid_t pid;
     int need_to_reenable_gc = 0;
     char *const *exec_array, *const *argv = NULL, *const *envp = NULL;
-    Py_ssize_t arg_num;
+    Py_ssize_t arg_num, num_fds_to_keep;
 
     if (!PyArg_ParseTuple(
-            args, "OOOOOiiiiiiiiiiO:fork_exec",
-            &process_args, &executable_list, &py_close_fds,
+            args, "OOOOOOiiiiiiiiiiO:fork_exec",
+            &process_args, &executable_list, &py_close_fds, &py_fds_to_keep,
             &cwd_obj, &env_list,
             &p2cread, &p2cwrite, &c2pread, &c2pwrite,
             &errread, &errwrite, &errpipe_read, &errpipe_write,
@@ -198,6 +216,11 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
         PyErr_SetString(PyExc_ValueError, "errpipe_write must be >= 3");
         return NULL;
     }
+    num_fds_to_keep = PySequence_Length(py_fds_to_keep);
+    if (num_fds_to_keep < 0) {
+        PyErr_SetString(PyExc_ValueError, "bad fds_to_keep");
+        return NULL;
+    }
 
     /* We need to call gc.disable() when we'll be calling preexec_fn */
     if (preexec_fn != Py_None) {
@@ -298,6 +321,7 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
                    p2cread, p2cwrite, c2pread, c2pwrite,
                    errread, errwrite, errpipe_read, errpipe_write,
                    close_fds, restore_signals, call_setsid,
+                   num_fds_to_keep, py_fds_to_keep,
                    preexec_fn, preexec_fn_args_tuple);
         _exit(255);
         return NULL;  /* Dead code to avoid a potential compiler warning. */