diff -r 6343bdbb7085 Lib/subprocess.py --- a/Lib/subprocess.py Mon Feb 10 03:43:57 2014 -0800 +++ b/Lib/subprocess.py Mon Feb 10 15:44:35 2014 +0200 @@ -461,22 +461,20 @@ 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 = [] +# This lists holds PIDs of processes which had not exited at the time +# corresponding Popen instance's __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 = set() -def _cleanup(): - for inst in _active[:]: - res = inst._internal_poll(_deadstate=sys.maxsize) - if res is not None: - try: - _active.remove(inst) - except ValueError: - # This can happen if two threads create a new Popen instance. - # It's harmless that it was already removed, so ignore. - pass +def _cleanup(_active=_active, _waitpid=os.waitpid, _WNOHANG=os.WNOHANG): + for pid in _active.copy(): + try: + if _waitpid(pid, _WNOHANG)[0] != pid: + continue + except OSError: + pass + _active.discard(pid) PIPE = -1 STDOUT = -2 @@ -890,18 +888,20 @@ # Wait for the process to terminate, to avoid zombies. self.wait() - def __del__(self, _maxsize=sys.maxsize, _active=_active): - # If __init__ hasn't had a chance to execute (e.g. if it - # was passed an undeclared keyword argument), we don't - # have a _child_created attribute at all. - if not getattr(self, '_child_created', False): - # We didn't get to successfully create a child process. - return - # In case the child hasn't been waited on, check if it's done. - self._internal_poll(_deadstate=_maxsize) - if self.returncode is None and _active is not None: - # Child is still running, keep us alive until we can wait on it. - _active.append(self) + if not mswindows: + def __del__(self, _maxsize=sys.maxsize): + # If __init__ hasn't had a chance to execute (e.g. if it + # was passed an undeclared keyword argument), we don't + # have a _child_created attribute at all. + if not getattr(self, '_child_created', False): + # We didn't get to successfully create a child process. + return + # In case the child hasn't been waited on, check if it's done. + self._internal_poll(_deadstate=_maxsize) + active = _active + if self.returncode is None and active is not None: + # Child is still running, keep us alive until we can wait on it. + active.add(self.pid) def _get_devnull(self): if not hasattr(self, '_devnull'): diff -r 6343bdbb7085 Lib/test/test_subprocess.py --- a/Lib/test/test_subprocess.py Mon Feb 10 03:43:57 2014 -0800 +++ b/Lib/test/test_subprocess.py Mon Feb 10 15:44:35 2014 +0200 @@ -54,8 +54,9 @@ support.reap_children() def tearDown(self): - for inst in subprocess._active: - inst.wait() + if not mswindows: + for pid in subprocess._active: + os.waitpid(pid, 0) subprocess._cleanup() self.assertFalse(subprocess._active, "subprocess._active not empty") @@ -1982,6 +1983,7 @@ finally: p.wait() + @unittest.skipIf(mswindows, "Posix specific test") 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 @@ -1994,12 +1996,12 @@ stderr=subprocess.PIPE) self.addCleanup(p.stdout.close) self.addCleanup(p.stderr.close) - 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]) + # check that pid is in the active processes list + self.assertIn(pid, subprocess._active) + @unittest.skipIf(mswindows, "Posix specific test") 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 @@ -2013,24 +2015,23 @@ stderr=subprocess.PIPE) self.addCleanup(p.stdout.close) self.addCleanup(p.stderr.close) - 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]) + self.assertIn(pid, subprocess._active) # let some time for the process to exit, and create a new Popen: this - # should trigger the wait() of p + # should trigger the waitpid() of pid time.sleep(0.2) with self.assertRaises(OSError) 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 + # pid should have been waitpid()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]) + self.assertNotIn(pid, subprocess._active) def test_close_fds_after_preexec(self): fd_status = support.findfile("fd_status.py", subdir="subprocessdata")