[Python-checkins] cpython (merge 3.3 -> default): Issue #18763: subprocess: The file descriptors are now closed after calling the

[charles-francois.natali]

http://hg.python.org/cpython/rev/748a5d39e7ce
changeset:   85392:748a5d39e7ce
parent:      85373:ffb01a6c0960
parent:      85391:4c52d4bac03c
user:        Charles-François Natali <cf.natali at gmail.com>
date:        Sun Aug 25 18:25:38 2013 +0200
summary:
  Issue #18763: subprocess: The file descriptors are now closed after calling the
preexec_fn callback, which may open file descriptors.

files:
  Lib/test/test_subprocess.py |  17 +++++++++++++++++
  Modules/_posixsubprocess.c  |  23 ++++++++++++-----------
  2 files changed, 29 insertions(+), 11 deletions(-)


diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -1972,6 +1972,23 @@
         self.assertRaises(OSError, os.waitpid, pid, 0)
         self.assertNotIn(ident, [id(o) for o in subprocess._active])
 
+    def test_close_fds_after_preexec(self):
+        fd_status = support.findfile("fd_status.py", subdir="subprocessdata")
+
+        # this FD is used as dup2() target by preexec_fn, and should be closed
+        # in the child process
+        fd = os.dup(1)
+        self.addCleanup(os.close, fd)
+
+        p = subprocess.Popen([sys.executable, fd_status],
+                             stdout=subprocess.PIPE, close_fds=True,
+                             preexec_fn=lambda: os.dup2(1, fd))
+        output, ignored = p.communicate()
+
+        remaining_fds = set(map(int, output.split(b',')))
+
+        self.assertNotIn(fd, remaining_fds)
+
 
 @unittest.skipUnless(mswindows, "Windows specific tests")
 class Win32ProcessTestCase(BaseTestCase):
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -412,17 +412,6 @@
         POSIX_CALL(close(errwrite));
     }
 
-    if (close_fds) {
-        int local_max_fd = max_fd;
-#if defined(__NetBSD__)
-        local_max_fd = fcntl(0, F_MAXFD);
-        if (local_max_fd < 0)
-            local_max_fd = max_fd;
-#endif
-        /* TODO HP-UX could use pstat_getproc() if anyone cares about it. */
-        _close_open_fd_range(3, local_max_fd, py_fds_to_keep);
-    }
-
     if (cwd)
         POSIX_CALL(chdir(cwd));
 
@@ -451,6 +440,18 @@
         /* Py_DECREF(result); - We're about to exec so why bother? */
     }
 
+    /* close FDs after executing preexec_fn, which might open FDs */
+    if (close_fds) {
+        int local_max_fd = max_fd;
+#if defined(__NetBSD__)
+        local_max_fd = fcntl(0, F_MAXFD);
+        if (local_max_fd < 0)
+            local_max_fd = max_fd;
+#endif
+        /* TODO HP-UX could use pstat_getproc() if anyone cares about it. */
+        _close_open_fd_range(3, local_max_fd, py_fds_to_keep);
+    }
+
     /* This loop matches the Lib/os.py _execvpe()'s PATH search when */
     /* given the executable_list generated by Lib/subprocess.py.     */
     saved_errno = 0;

-- 
Repository URL: http://hg.python.org/cpython