Fixed bug #1531862: Do not close standard file descriptors in subprocess.

Let's try that once more. Buildbots were broken last time, but probably
because tests were sending data to stderr for testing it (sending to a
file doesn't touch the problem).

The fix is still the same, but tests were reduced (removing tests to
be able to fix something is weird, but oh well).

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 5438f158cb84c97d6337116dc947c03021b18c48..7c229dc785eb9bba379b725f4f701e5af1db1e77 100644 (file)
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1000,14 +1000,10 @@ class Popen(object):
                     if errwrite:
                         os.dup2(errwrite, 2)
 
-                    # Close pipe fds.  Make sure we doesn't close the same
-                    # fd more than once.
-                    if p2cread:
-                        os.close(p2cread)
-                    if c2pwrite and c2pwrite not in (p2cread,):
-                        os.close(c2pwrite)
-                    if errwrite and errwrite not in (p2cread, c2pwrite):
-                        os.close(errwrite)
+                    # Close pipe fds.  Make sure we don't close the same
+                    # fd more than once, or standard fds.
+                    for fd in set((p2cread, c2pwrite, errwrite))-set((0,1,2)):
+                        if fd: os.close(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 8c8ac405f8c0f71067b702f5bb9b79f3e33cbae2..19a31ea8f0edff3d7495c4aad2dc4975b233bcc9 100644 (file)
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -234,6 +234,18 @@ class ProcessTestCase(unittest.TestCase):
         stripped = remove_stderr_debug_decorations(output)
         self.assertEqual(stripped, "appleorange")
 
+    def test_stdout_filedes_of_stdout(self):
+        # stdout is set to 1 (#1531862).
+        cmd = r"import sys, os; sys.exit(os.write(sys.stdout.fileno(), '.\n'))"
+        rc = subprocess.call([sys.executable, "-c", cmd], stdout=1)
+        self.assertEquals(rc, 2)
+
+    def test_stdout_fileobj_of_stdout(self):
+        # stdout is set to sys.stdout (#1531862).
+        cmd = r"import sys, os; sys.exit(os.write(sys.stdout.fileno(), '.\n'))"
+        rc = subprocess.call([sys.executable, "-c", cmd], stdout=sys.stdout)
+        self.assertEquals(rc, 2)
+
     def test_cwd(self):
         tmpdir = os.getenv("TEMP", "/tmp")
         # We cannot use os.path.realpath to canonicalize the path,

diff --git a/Misc/NEWS b/Misc/NEWS
index 2cad993d9dafb5fe6a2b4265d073818c7f7e3f75..53708c1b9a436e83f706ffbbd31e440088b9629b 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -56,6 +56,8 @@ Library
 
 - Bug #1543303, patch #1543897: remove NUL padding from tarfiles.
 
+- Bug #1531862: Do not close standard file descriptors in subprocess.
+
 
 Extension Modules
 -----------------