From: Gregory P. Smith <greg@krypto.org>
Date: Sun, 11 Nov 2012 10:00:49 +0000 (-0800)
Subject: Fix issue #16140 bug that the fix to issue #16327 added - don't double
X-Git-Tag: v2.7.4rc1~387
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=211248b2148472812451b1d294cfb0e2a527dc94;p=thirdparty%2FPython%2Fcpython.git

Fix issue #16140 bug that the fix to issue #16327 added - don't double
close subprocess.PIPE file descriptors when the child encounters an
error prior to exec.
---

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index d1377d3b0471..104d6ec4d5ae 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1274,9 +1274,6 @@ class Popen(object):
                     if e.errno != errno.ECHILD:
                         raise
                 child_exception = pickle.loads(data)
-                for fd in (p2cwrite, c2pread, errread):
-                    if fd is not None:
-                        os.close(fd)
                 raise child_exception
 
 
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 31462652fd73..9d1ddb72cbd4 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -751,6 +751,53 @@ class POSIXProcessTestCase(BaseTestCase):
         self.addCleanup(p.stdout.close)
         self.assertEqual(p.stdout.read(), "apple")
 
+    @unittest.skipIf(not os.path.exists("/dev/zero"), "/dev/zero required.")
+    def test_preexec_errpipe_does_not_double_close_pipes(self):
+        """Issue16140: Don't double close pipes on preexec error."""
+        class SafeConstructorPopen(subprocess.Popen):
+            def __init__(self):
+                pass  # Do nothing so we can modify the instance for testing.
+            def RealPopen(self, *args, **kwargs):
+                subprocess.Popen.__init__(self, *args, **kwargs)
+        def raise_it():
+            raise RuntimeError("force the _execute_child() errpipe_data path.")
+
+        p = SafeConstructorPopen()
+
+        def _test_fds_execute_child_wrapper(
+                args, executable, preexec_fn, close_fds, cwd, env,
+                universal_newlines, startupinfo, creationflags, shell,
+                p2cread, p2cwrite,
+                c2pread, c2pwrite,
+                errread, errwrite):
+            try:
+                subprocess.Popen._execute_child(
+                        p, args, executable, preexec_fn, close_fds,
+                        cwd, env, universal_newlines,
+                        startupinfo, creationflags, shell,
+                        p2cread, p2cwrite,
+                        c2pread, c2pwrite,
+                        errread, errwrite)
+            finally:
+                # Open a bunch of file descriptors and verify that
+                # none of them are the same as the ones the Popen
+                # instance is using for stdin/stdout/stderr.
+                devzero_fds = [os.open("/dev/zero", os.O_RDONLY)
+                               for _ in range(8)]
+                try:
+                    for fd in devzero_fds:
+                        self.assertNotIn(fd, (p2cwrite, c2pread, errread))
+                                         
+                finally:
+                    map(os.close, devzero_fds)
+
+        p._execute_child = _test_fds_execute_child_wrapper
+
+        with self.assertRaises(RuntimeError):
+            p.RealPopen([sys.executable, "-c", "pass"],
+                        stdin=subprocess.PIPE, stdout=subprocess.PIPE,
+                        stderr=subprocess.PIPE, preexec_fn=raise_it)
+
     def test_args_string(self):
         # args is a string
         f, fname = mkstemp()