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 5684d523952c8905956cac148f6b8b20c5aef7e4..b8e3c0e4c856aa34f87c8cbc9c40e42c341d1888 100644 (file)
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1012,14 +1012,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):
@@ -1080,23 +1083,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)
+                            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.
-                            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)
+                            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/support.py b/Lib/test/support.py
index 842cda6c349580caae8fdaec10b365b228d16b0c..3353053071fa103612599258da91d20e19fc30d0 100644 (file)
--- a/Lib/test/support.py
+++ b/Lib/test/support.py
@@ -10,6 +10,7 @@ import gc
 import socket
 import sys
 import os
+import re
 import platform
 import shutil
 import warnings
@@ -1056,3 +1057,13 @@ def reap_children():
                     break
             except:
                 break
+
+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/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index a633af5058071157bfbef56227d94454dcf64dd8..ce3e0c232b386ba13ebe0d5e59d4ef508abfa215 100644 (file)
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -804,6 +804,62 @@ class ProcessTestCase(BaseTestCase):
                              " non-zero with this error:\n%s" %
                              stderr.decode('utf8'))
 
+        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 = 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_surrogates_error_message(self):
+            def prepare():
+                raise ValueError("surrogate:\uDCff")
+
 
     #
     # Windows tests

diff --git a/Misc/NEWS b/Misc/NEWS
index 512ffac6bdacf0313608d9210b27302f9d7ed104..f9dd5a10270c228c830a779f300390c4244a173f 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -27,6 +27,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 10753 - Characters ';','=' and ',' in the PATH_INFO environment
   variable won't be quoted when the URI is constructed by the wsgiref.util 's
   request_uri method. According to RFC 3986, these characters can be a part of