[Python-checkins] cpython (2.7): Fixes issue #16327: The subprocess module no longer leaks file descriptors

gregory.p.smith python-checkins at python.org
Sun Nov 11 07:49:17 CET 2012


http://hg.python.org/cpython/rev/e67620048d2f
changeset:   80371:e67620048d2f
branch:      2.7
parent:      80367:82711f5ab507
user:        Gregory P. Smith <greg at krypto.org>
date:        Sat Nov 10 22:49:03 2012 -0800
summary:
  Fixes issue #16327: The subprocess module no longer leaks file descriptors
used for stdin/stdout/stderr pipes to the child when fork() fails.

files:
  Lib/subprocess.py           |  33 ++++++++++++++++++++----
  Lib/test/test_subprocess.py |  33 +++++++++++++++++++++++++
  Misc/NEWS                   |   3 ++
  3 files changed, 63 insertions(+), 6 deletions(-)


diff --git a/Lib/subprocess.py b/Lib/subprocess.py
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -671,12 +671,33 @@
          c2pread, c2pwrite,
          errread, errwrite) = self._get_handles(stdin, stdout, stderr)
 
-        self._execute_child(args, executable, preexec_fn, close_fds,
-                            cwd, env, universal_newlines,
-                            startupinfo, creationflags, shell,
-                            p2cread, p2cwrite,
-                            c2pread, c2pwrite,
-                            errread, errwrite)
+        try:
+            self._execute_child(args, executable, preexec_fn, close_fds,
+                                cwd, env, universal_newlines,
+                                startupinfo, creationflags, shell,
+                                p2cread, p2cwrite,
+                                c2pread, c2pwrite,
+                                errread, errwrite)
+        except Exception:
+            # Preserve original exception in case os.close raises.
+            exc_type, exc_value, exc_trace = sys.exc_info()
+
+            to_close = []
+            # Only close the pipes we created.
+            if stdin == PIPE:
+                to_close.extend((p2cread, p2cwrite))
+            if stdout == PIPE:
+                to_close.extend((c2pread, c2pwrite))
+            if stderr == PIPE:
+                to_close.extend((errread, errwrite))
+
+            for fd in to_close:
+                try:
+                    os.close(fd)
+                except EnvironmentError:
+                    pass
+
+            raise exc_type, exc_value, exc_trace
 
         if mswindows:
             if p2cwrite is not None:
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
@@ -58,6 +58,18 @@
         self.assertEqual(actual, expected, msg)
 
 
+class PopenTestException(Exception):
+    pass
+
+
+class PopenExecuteChildRaises(subprocess.Popen):
+    """Popen subclass for testing cleanup of subprocess.PIPE filehandles when
+    _execute_child fails.
+    """
+    def _execute_child(self, *args, **kwargs):
+        raise PopenTestException("Forced Exception for Test")
+
+
 class ProcessTestCase(BaseTestCase):
 
     def test_call_seq(self):
@@ -632,6 +644,27 @@
         time.sleep(2)
         p.communicate("x" * 2**20)
 
+    # This test is Linux-ish specific for simplicity to at least have
+    # some coverage.  It is not a platform specific bug.
+    @unittest.skipUnless(os.path.isdir('/proc/%d/fd' % os.getpid()),
+                         "Linux specific")
+    def test_failed_child_execute_fd_leak(self):
+        """Test for the fork() failure fd leak reported in issue16327."""
+        fd_directory = '/proc/%d/fd' % os.getpid()
+        fds_before_popen = os.listdir(fd_directory)
+        with self.assertRaises(PopenTestException):
+            PopenExecuteChildRaises(
+                    [sys.executable, '-c', 'pass'], stdin=subprocess.PIPE,
+                    stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+
+        # NOTE: This test doesn't verify that the real _execute_child
+        # does not close the file descriptors itself on the way out
+        # during an exception.  Code inspection has confirmed that.
+
+        fds_after_exception = os.listdir(fd_directory)
+        self.assertEqual(fds_before_popen, fds_after_exception)
+
+
 # context manager
 class _SuppressCoreFiles(object):
     """Try to prevent core files from being created."""
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -143,6 +143,9 @@
 Library
 -------
 
+- Issue #16327: The subprocess module no longer leaks file descriptors
+  used for stdin/stdout/stderr pipes to the child when fork() fails.
+
 - Issue #14396: Handle the odd rare case of waitpid returning 0 when not
   expected in subprocess.Popen.wait().
 

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


More information about the Python-checkins mailing list