diff options
| -rw-r--r-- | Lib/subprocess.py | 20 | ||||
| -rw-r--r-- | Lib/test/test_subprocess.py | 33 | ||||
| -rw-r--r-- | Misc/NEWS | 3 |
3 files changed, 53 insertions, 3 deletions
diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 35a98c8..7cd5a57 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -744,13 +744,27 @@ class Popen(object): errread, errwrite, restore_signals, start_new_session) except: - # Cleanup if the child failed starting - for f in filter(None, [self.stdin, self.stdout, self.stderr]): + # Cleanup if the child failed starting. + for f in filter(None, (self.stdin, self.stdout, self.stderr)): try: f.close() except EnvironmentError: - # Ignore EBADF or other errors + pass # Ignore EBADF or other errors. + + # Make sure the child pipes are closed as well. + to_close = [] + if stdin == PIPE: + to_close.append(p2cread) + if stdout == PIPE: + to_close.append(c2pwrite) + if stderr == PIPE: + to_close.append(errwrite) + for fd in to_close: + try: + os.close(fd) + except EnvironmentError: pass + raise diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 15fb498..5c20975 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -65,6 +65,18 @@ class BaseTestCase(unittest.TestCase): 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): @@ -841,6 +853,27 @@ class ProcessTestCase(BaseTestCase): process.communicate() + # 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.""" @@ -162,6 +162,9 @@ Core and Builtins 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(). |