diff options
author | Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> | 2019-09-06 09:14:24 (GMT) |
---|---|---|
committer | Victor Stinner <vstinner@redhat.com> | 2019-09-06 09:14:24 (GMT) |
commit | 1e2707d7e82aedf73c59772bc7aa228286323c3c (patch) | |
tree | af464c4cd5fd147ef3c4fff1f851a4357a92f4af /Lib/test/test_subprocess.py | |
parent | 256f03befb1f144340e11bc6eee92d3afd84ee23 (diff) | |
download | cpython-1e2707d7e82aedf73c59772bc7aa228286323c3c.zip cpython-1e2707d7e82aedf73c59772bc7aa228286323c3c.tar.gz cpython-1e2707d7e82aedf73c59772bc7aa228286323c3c.tar.bz2 |
bpo-37380: subprocess: don't use _active on win (GH-14360) (GH-15706)
As noted by @eryksun in [1] and [2], using _cleanup and _active(in
__del__) is not necessary on Windows, since:
> Unlike Unix, a process in Windows doesn't have to be waited on by
> its parent to avoid a zombie. Keeping the handle open will actually
> create a zombie until the next _cleanup() call, which may be never
> if Popen() isn't called again.
This patch simply defines `subprocess._active` as `None`, for which we already
have the proper logic in place in `subprocess.Popen.__del__`, that prevents it
from trying to append the process to the `_active`. This patch also defines
`subprocess._cleanup` as a noop for Windows.
[1] https://bugs.python.org/issue37380GH-msg346333
[2] https://bugs.python.org/issue36067GH-msg336262
Signed-off-by: Ruslan Kuprieiev <ruslan@iterative.ai>
(cherry picked from commit 042821ae3cf537e01963c9ec85d1a454d921e826)
Co-authored-by: Ruslan Kuprieiev <kupruser@gmail.com>
Diffstat (limited to 'Lib/test/test_subprocess.py')
-rw-r--r-- | Lib/test/test_subprocess.py | 34 |
1 files changed, 25 insertions, 9 deletions
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 8419061..41e6a09 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -59,10 +59,14 @@ class BaseTestCase(unittest.TestCase): support.reap_children() def tearDown(self): - for inst in subprocess._active: - inst.wait() - subprocess._cleanup() - self.assertFalse(subprocess._active, "subprocess._active not empty") + if not mswindows: + # subprocess._active is not used on Windows and is set to None. + for inst in subprocess._active: + inst.wait() + subprocess._cleanup() + self.assertFalse( + subprocess._active, "subprocess._active not empty" + ) self.doCleanups() support.reap_children() @@ -2622,8 +2626,12 @@ class POSIXProcessTestCase(BaseTestCase): with support.check_warnings(('', ResourceWarning)): p = None - # check that p is in the active processes list - self.assertIn(ident, [id(o) for o in subprocess._active]) + if mswindows: + # subprocess._active is not used on Windows and is set to None. + self.assertIsNone(subprocess._active) + else: + # check that p is in the active processes list + self.assertIn(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 @@ -2644,8 +2652,12 @@ class POSIXProcessTestCase(BaseTestCase): p = None os.kill(pid, signal.SIGKILL) - # check that p is in the active processes list - self.assertIn(ident, [id(o) for o in subprocess._active]) + if mswindows: + # subprocess._active is not used on Windows and is set to None. + self.assertIsNone(subprocess._active) + else: + # 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 @@ -2657,7 +2669,11 @@ class POSIXProcessTestCase(BaseTestCase): 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]) + if mswindows: + # subprocess._active is not used on Windows and is set to None. + self.assertIsNone(subprocess._active) + else: + self.assertNotIn(ident, [id(o) for o in subprocess._active]) def test_close_fds_after_preexec(self): fd_status = support.findfile("fd_status.py", subdir="subprocessdata") |