New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
subprocess.Popen.__exit__ doesn't wait for process end #56253
Comments
I find it a bit strange that Popen.__exit__ closes all standard file descriptors leading to the child process, but doesn't wait for process end afterwards. |
Seems like it would be enough to add a wait() at the end? diff -r 9e473917cbfb Lib/subprocess.py
--- a/Lib/subprocess.py Mon May 09 21:17:02 2011 +0200
+++ b/Lib/subprocess.py Mon May 09 15:30:02 2011 -0500
@@ -796,6 +796,7 @@
self.stderr.close()
if self.stdin:
self.stdin.close()
+ self.wait() def __del__(self, _maxsize=sys.maxsize, _active=_active):
if not self._child_created: |
Probably, perhaps with a try/finally? I'm not entirely sure this is a good idea, by the way. May there be some |
Actually, I don't think the wait() is a good idea. If you want to block and infinitely wait on the process to close, you should do so explicitly. It's probably better that we try to use terminate() or kill() and raise if that fails. |
Ok.
Uh, I think it's worse. Using a context manager shouldn't do something |
Hm, yeah, not sure what I was thinking there. I'm thinking there's not a lot we can do here, but also not a lot that we should do here. We don't want to wait, and we don't want to close, so maybe we should just document that the usage should be limited only to expecting the open handles to be closed? |
Sounds good indeed :) |
Looks like we already mention that. """ Antoine, do you think this should be more strongly worded? |
Well, no, looks ok for me then :) |
There's just one thing I'm concerned with. """ The problem is that until a child process is wait()ed on or its parent |
I didn't initially like the idea of __exit__ blocking on another It is a backwards incompatible change if anyone has started using the I say change the behavior to wait() in 3.3, 3.2.1 and 2.7.2. Keep the |
I'm re-opening this issue, since Gregory agrees to change the current behaviour. |
New changeset 7a3f3ad83676 by Gregory P. Smith in branch 'default':
|
New changeset b00a64a5cb93 by Gregory P. Smith in branch '3.2': |
did my commits in the reverse order (default before 3.2), oops. this is fixed. this wasn't ever in 2.7 so no need for the documentation note. i'm not worried about adding a note about 3.2.0 vs 3.2.1 beyond the mention in Misc/NEWS as this was new in 3.2.0 and fixing this behavior is a pretty clear bug fix. |
New changeset 361f87c8f36a by Łukasz Langa in branch 'default': |
See also issue bpo-12494: "subprocess: check_output() doesn't close pipes on error". |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: