Issue #12650: Fix a race condition where a subprocess.Popen could leak
resources (FD/zombie) when killed at the wrong time.
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index e92961a..abd76ee 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -460,7 +460,7 @@
def _cleanup():
for inst in _active[:]:
res = inst._internal_poll(_deadstate=sys.maxint)
- if res is not None and res >= 0:
+ if res is not None:
try:
_active.remove(inst)
except ValueError:
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 477accf..074f86d 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -1080,6 +1080,63 @@
subprocess._eintr_retry_call(fake_os_func, 666))
self.assertEqual([(256, 999), (666,), (666,)], record_calls)
+ 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(mswindows, "mswindows only")
class CommandsWithSpaces (BaseTestCase):
diff --git a/Misc/NEWS b/Misc/NEWS
index 139eb6f..679b4b0 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -40,6 +40,9 @@
Library
-------
+- Issue #12650: Fix a race condition where a subprocess.Popen could leak
+ resources (FD/zombie) when killed at the wrong time.
+
- Issue #12752: Fix regression which prevented locale.normalize() from
accepting unicode strings.