--- Lib/subprocess.py | 33 +++++++++++++++++++++++++++------ Lib/test/test_subprocess.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 6 deletions(-) --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -671,12 +671,33 @@ class Popen(object): 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 OSError: + pass + + raise exc_type, exc_value, exc_trace if mswindows: if p2cwrite is not None: --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -58,6 +58,23 @@ 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, executable, preexec_fn, close_fds, + cwd, env, universal_newlines, + startupinfo, creationflags, shell, + p2cread, p2cwrite, + c2pread, c2pwrite, + errread, errwrite): + + raise PopenTestException("Forced Exception for Test") + + class ProcessTestCase(BaseTestCase): def test_call_seq(self): @@ -631,6 +648,23 @@ class ProcessTestCase(BaseTestCase): time.sleep(2) p.communicate("x" * 2**20) + # This test is Linux 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") + # Test for the fd leak reported in http://bugs.python.org/issue16327 + def test_failed_child_execute_fd_leak(self): + fd_directory = '/proc/%d/fd' % os.getpid() + num_fds_before_popen = len(os.listdir(fd_directory)) + with self.assertRaises(PopenTestException): + PopenExecuteChildRaises( + [sys.executable, "-c", "print()"], stdin=subprocess.PIPE, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + num_fds_after_exception = len(os.listdir(fd_directory)) + self.assertEqual(num_fds_before_popen, num_fds_after_exception) + + # context manager class _SuppressCoreFiles(object): """Try to prevent core files from being created."""