Fixes issue #16327: The subprocess module no longer leaks file descriptors
used for stdin/stdout/stderr pipes to the child when fork() fails.
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 8356782..d1377d3 100644
--- 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
index 5e74f4a..3146265 100644
--- 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
index c27f829..96c8614 100644
--- 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().