Skip to content

Conversation

@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Sep 5, 2019

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 [email protected]
(cherry picked from commit 042821a)

Co-authored-by: Ruslan Kuprieiev [email protected]

https://bugs.python.org/issue37380

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 <[email protected]>
(cherry picked from commit 042821a)

Co-authored-by: Ruslan Kuprieiev <[email protected]>
@miss-islington
Copy link
Contributor Author

@efiop and @vstinner: Status check is done, and it's a success ✅ .

1 similar comment
@miss-islington
Copy link
Contributor Author

@efiop and @vstinner: Status check is done, and it's a success ✅ .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants