diff -r ccce01988603 Lib/subprocess.py --- a/Lib/subprocess.py Thu Jul 28 09:55:13 2011 -0700 +++ b/Lib/subprocess.py Thu Jul 28 22:51:39 2011 +0200 @@ -424,12 +424,16 @@ except: MAXFD = 256 +# This lists holds Popen instances for which the underlying process had not +# exited at the time its __del__ method got called: those processes are wait()ed +# for synchronously from _cleanup() when a new Popen object is created, to avoid +# zombie processes. _active = [] def _cleanup(): for inst in _active[:]: res = inst._internal_poll(_deadstate=sys.maxsize) - if res is not None and res >= 0: + if res is not None: try: _active.remove(inst) except ValueError: @@ -1272,6 +1276,7 @@ errread, errwrite, errpipe_read, errpipe_write, restore_signals, start_new_session, preexec_fn) + self._child_created = True finally: # be sure the FD is closed no matter what os.close(errpipe_write) diff -r ccce01988603 Lib/test/test_subprocess.py --- a/Lib/test/test_subprocess.py Thu Jul 28 09:55:13 2011 -0700 +++ b/Lib/test/test_subprocess.py Thu Jul 28 22:51:39 2011 +0200 @@ -1622,6 +1622,63 @@ if dir is not None: os.rmdir(dir) + def test_zombie_fast_process_del(self): + # Issue #12650: on Unix, if Popen.__del__() was called before the + # process exited, it wouldn't be added to subprocess._active, and would + # remain a zombie. + # spawn a Popen, and delete its reference before it exits + p = subprocess.Popen([sys.executable, "-c", + 'import sys, time;' + 'time.sleep(0.2)'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + ident = id(p) + pid = p.pid + del p + # check that p is in the active processes list + self.assertIn(ident, [id(o) for o in subprocess._active]) + + # sleep a little to let the process exit, and create a new Popen: this + # should trigger the wait() of p + time.sleep(1) + with self.assertRaises(EnvironmentError) as c: + with subprocess.Popen(['nonexisting_i_hope'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) as proc: + pass + # p should have been wait()ed on, and removed from the _active list + self.assertRaises(OSError, os.waitpid, pid, 0) + self.assertNotIn(ident, [id(o) for o in subprocess._active]) + + def test_leak_fast_process_del_killed(self): + # Issue #12650: on Unix, if Popen.__del__() was called before the + # process exited, and the process got killed by a signal, it would never + # be removed from subprocess._active, which triggered a FD and memory + # leak. + # spawn a Popen, delete its reference and kill it + p = subprocess.Popen([sys.executable, "-c", + 'import time;' + 'time.sleep(3)'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + ident = id(p) + pid = p.pid + del p + os.kill(pid, signal.SIGKILL) + # check that p is in the active processes list + self.assertIn(ident, [id(o) for o in subprocess._active]) + + # let some time for the process to exit, and create a new Popen: this + # should trigger the wait() of p + time.sleep(0.2) + with self.assertRaises(EnvironmentError) as c: + with subprocess.Popen(['nonexisting_i_hope'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) as proc: + pass + # p should have been wait()ed on, and removed from the _active list + self.assertRaises(OSError, os.waitpid, pid, 0) + self.assertNotIn(ident, [id(o) for o in subprocess._active]) @unittest.skipUnless(getattr(subprocess, '_has_poll', False), "poll system call not supported")