mirror of
https://github.com/python/cpython.git
synced 2025-08-15 06:10:47 +00:00
bpo-37380: subprocess: don't use _active on win (GH-14360) (GH-15707)
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 042821ae3c
)
Co-authored-by: Ruslan Kuprieiev <kupruser@gmail.com>
This commit is contained in:
parent
b8c66779c7
commit
4d1abedce9
3 changed files with 58 additions and 24 deletions
|
@ -218,22 +218,38 @@ else:
|
|||
_PopenSelector = selectors.SelectSelector
|
||||
|
||||
|
||||
# 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 = []
|
||||
if _mswindows:
|
||||
# On Windows we just need to close `Popen._handle` when we no longer need
|
||||
# it, so that the kernel can free it. `Popen._handle` gets closed
|
||||
# implicitly when the `Popen` instance is finalized (see `Handle.__del__`,
|
||||
# which is calling `CloseHandle` as requested in [1]), so there is nothing
|
||||
# for `_cleanup` to do.
|
||||
#
|
||||
# [1] https://docs.microsoft.com/en-us/windows/desktop/ProcThread/
|
||||
# creating-processes
|
||||
_active = None
|
||||
|
||||
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():
|
||||
pass
|
||||
else:
|
||||
# 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():
|
||||
if _active is None:
|
||||
return
|
||||
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
|
||||
|
||||
PIPE = -1
|
||||
STDOUT = -2
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue