classification
Title: Popen._internal_poll() references errno.ECHILD outside of the local scope
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.2, Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: asvetlov, gregory.p.smith, jcea, pitrou, python-dev, serhiy.storchaka, terry.reedy
Priority: normal Keywords: patch

Created on 2012-12-09 07:03 by serhiy.storchaka, last changed 2013-09-15 04:22 by terry.reedy. This issue is now closed.

Files
File name Uploaded Description Edit
subprocess_reference_nonlocal.patch serhiy.storchaka, 2012-12-09 07:03 review
Messages (8)
msg177201 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-09 07:03
As noted in Popen._internal_poll() docstring this method cannot reference anything outside of the local scope. However it references errno.ECHILD. The proposed patch fixes this.

Is it good that Popen._handle_exitstatus() references building SubprocessError?
msg177226 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-12-09 16:50
The patch LGTM.
About _handle_exitstatus: I guess nothing wrong to fix it also.
msg177227 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-12-09 16:58
I'm just asking if this is a bug. If using building exceptions is safe, then we can get rid of _os_error and _ECHILD in 3.3+, using OSError and ChildProcessError instead.
msg177229 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2012-12-09 18:00
it's a potential bug.  your patch looks good.

as for _handle_exitstatus referring to SubprocessError, that is fine.  In that situation it is trying to raise the exception and the only time that would ever be a problem is when called by the gc during a __del__ where such an exception for this "impossible" situation cannot be caught anyways.  It would effectively become an uncaught NameError instead of an uncaught SubprocessError; not a big deal.
msg177686 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-12-18 12:15
As I can see in Py_Finalize finalized for standard exception classes is called after any python code has been finished, so we don't need to protect those exceptions.
msg178085 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-12-24 18:09
New changeset 0cc4fe5634cf by Andrew Svetlov in branch '3.2':
Keep ref to ECHILD in local scope (#16650)
http://hg.python.org/cpython/rev/0cc4fe5634cf

New changeset 0b1a49f99169 by Andrew Svetlov in branch '3.3':
Keep ref to ECHILD in local scope (#16650)
http://hg.python.org/cpython/rev/0b1a49f99169

New changeset 8f30461395b1 by Andrew Svetlov in branch 'default':
Keep ref to ECHILD in local scope (#16650)
http://hg.python.org/cpython/rev/8f30461395b1

New changeset a963dd401a63 by Andrew Svetlov in branch '2.7':
Keep ref to ECHILD in local scope (#16650)
http://hg.python.org/cpython/rev/a963dd401a63
msg178086 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-12-24 18:11
Close the issue.
For future improvements (like ChildProcessError using) please open new one.
msg197746 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-09-15 04:22
sorry for noise
History
Date User Action Args
2013-09-15 04:22:47terry.reedysetnosy: + terry.reedy

messages: + msg197746
versions: + Python 3.2
2013-09-15 04:22:03terry.reedysetversions: - Python 3.2
2012-12-24 18:11:52asvetlovsetstatus: open -> closed
resolution: fixed
messages: + msg178086

stage: patch review -> resolved
2012-12-24 18:09:39python-devsetnosy: + python-dev
messages: + msg178085
2012-12-18 12:15:50asvetlovsetmessages: + msg177686
2012-12-15 22:01:23serhiy.storchakalinkissue16648 dependencies
2012-12-10 12:03:52jceasetnosy: + jcea
2012-12-09 18:00:08gregory.p.smithsetmessages: + msg177229
2012-12-09 16:58:32serhiy.storchakasetmessages: + msg177227
2012-12-09 16:50:20asvetlovsetmessages: + msg177226
2012-12-09 13:04:19pitrousetnosy: + gregory.p.smith
2012-12-09 07:03:20serhiy.storchakacreate