Merged revisions 87695 via svnmerge from

svn+ssh://pythondev@svn.python.org/python/branches/py3k

........
  r87695 | antoine.pitrou | 2011-01-03 19:23:55 +0100 (lun., 03 janv. 2011) | 5 lines

  Issue #10806, issue #9905: Fix subprocess pipes when some of the standard
  file descriptors (0, 1, 2) are closed in the parent process.  Initial
  patch by Ross Lagerwall.
........

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 4c235d9a6147424ebe34ebe1e831a66ea3d6d708..2ecde0bb39d7aaa088788d6f7b21d8b9c30f3f71 100644 (file)
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1051,14 +1051,17 @@ class Popen(object):
                     errread, errwrite)
 
 
-        def _set_cloexec_flag(self, fd):
+        def _set_cloexec_flag(self, fd, cloexec=True):
             try:
                 cloexec_flag = fcntl.FD_CLOEXEC
             except AttributeError:
                 cloexec_flag = 1
 
             old = fcntl.fcntl(fd, fcntl.F_GETFD)
-            fcntl.fcntl(fd, fcntl.F_SETFD, old | cloexec_flag)
+            if cloexec:
+                fcntl.fcntl(fd, fcntl.F_SETFD, old | cloexec_flag)
+            else:
+                fcntl.fcntl(fd, fcntl.F_SETFD, old & ~cloexec_flag)
 
 
         def _close_fds(self, but):
@@ -1128,21 +1131,25 @@ class Popen(object):
                             os.close(errpipe_read)
 
                             # Dup fds for child
-                            if p2cread is not None:
-                                os.dup2(p2cread, 0)
-                            if c2pwrite is not None:
-                                os.dup2(c2pwrite, 1)
-                            if errwrite is not None:
-                                os.dup2(errwrite, 2)
-
-                            # Close pipe fds.  Make sure we don't close the same
-                            # fd more than once, or standard fds.
-                            if p2cread is not None and p2cread not in (0,):
-                                os.close(p2cread)
-                            if c2pwrite is not None and c2pwrite not in (p2cread, 1):
-                                os.close(c2pwrite)
-                            if errwrite is not None and errwrite not in (p2cread, c2pwrite, 2):
-                                os.close(errwrite)
+                            def _dup2(a, b):
+                                # dup2() removes the CLOEXEC flag but
+                                # we must do it ourselves if dup2()
+                                # would be a no-op (issue #10806).
+                                if a == b:
+                                    self._set_cloexec_flag(a, False)
+                                elif a is not None:
+                                    os.dup2(a, b)
+                            _dup2(p2cread, 0)
+                            _dup2(c2pwrite, 1)
+                            _dup2(errwrite, 2)
+
+                            # Close pipe fds.  Make sure we don't close the
+                            # same fd more than once, or standard fds.
+                            closed = { None }
+                            for fd in [p2cread, c2pwrite, errwrite]:
+                                if fd not in closed and fd > 2:
+                                    os.close(fd)
+                                    closed.add(fd)
 
                             # Close all other fds, if asked for
                             if close_fds:

diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 26adf225d56c15751105516a278ad70917a0819f..37dc245ebead98b539baf5f516a5dfa748fb2912 100644 (file)
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -778,6 +778,58 @@ class POSIXProcessTestCase(BaseTestCase):
         self.assertStderrEqual(stderr, '')
         self.assertEqual(p.wait(), -signal.SIGTERM)
 
+    def check_close_std_fds(self, fds):
+        # Issue #9905: test that subprocess pipes still work properly with
+        # some standard fds closed
+        stdin = 0
+        newfds = []
+        for a in fds:
+            b = os.dup(a)
+            newfds.append(b)
+            if a == 0:
+                stdin = b
+        try:
+            for fd in fds:
+                os.close(fd)
+            out, err = subprocess.Popen([sys.executable, "-c",
+                              'import sys;'
+                              'sys.stdout.write("apple");'
+                              'sys.stdout.flush();'
+                              'sys.stderr.write("orange")'],
+                       stdin=stdin,
+                       stdout=subprocess.PIPE,
+                       stderr=subprocess.PIPE).communicate()
+            err = test_support.strip_python_stderr(err)
+            self.assertEqual((out, err), (b'apple', b'orange'))
+        finally:
+            for b, a in zip(newfds, fds):
+                os.dup2(b, a)
+            for b in newfds:
+                os.close(b)
+
+    def test_close_fd_0(self):
+        self.check_close_std_fds([0])
+
+    def test_close_fd_1(self):
+        self.check_close_std_fds([1])
+
+    def test_close_fd_2(self):
+        self.check_close_std_fds([2])
+
+    def test_close_fds_0_1(self):
+        self.check_close_std_fds([0, 1])
+
+    def test_close_fds_0_2(self):
+        self.check_close_std_fds([0, 2])
+
+    def test_close_fds_1_2(self):
+        self.check_close_std_fds([1, 2])
+
+    def test_close_fds_0_1_2(self):
+        # Issue #10806: test that subprocess pipes still work properly with
+        # all standard fds closed.
+        self.check_close_std_fds([0, 1, 2])
+
     def test_wait_when_sigchild_ignored(self):
         # NOTE: sigchild_ignore.py may not be an effective test on all OSes.
         sigchild_ignore = test_support.findfile("sigchild_ignore.py",

diff --git a/Lib/test/test_support.py b/Lib/test/test_support.py
index 6c2b2b2397487bbde39a712a24c67ea6dae75188..cf912e10efcd3d60dba86274a7dac0fd85a8664f 100644 (file)
--- a/Lib/test/test_support.py
+++ b/Lib/test/test_support.py
@@ -1219,3 +1219,13 @@ def args_from_interpreter_flags():
         if v > 0:
             args.append('-' + opt * v)
     return args
+
+def strip_python_stderr(stderr):
+    """Strip the stderr of a Python process from potential debug output
+    emitted by the interpreter.
+
+    This will typically be run on the result of the communicate() method
+    of a subprocess.Popen object.
+    """
+    stderr = re.sub(br"\[\d+ refs\]\r?\n?$", b"", stderr).strip()
+    return stderr

diff --git a/Misc/NEWS b/Misc/NEWS
index 9c900ffc570bdbb4ec1a7dc2bb56bc84875c7bc1..c6faa12528aff51c2a222c33712ef9a198bfbedc 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -25,6 +25,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #10806, issue #9905: Fix subprocess pipes when some of the standard
+  file descriptors (0, 1, 2) are closed in the parent process.  Initial
+  patch by Ross Lagerwall.
+
 - Issue #4662: os.tempnam(), os.tmpfile() and os.tmpnam() now raise a py3k
   DeprecationWarning.