classification
Title: subprocess.Popen does not handle file-like objects without file descriptors
Type: behavior Stage: resolved
Components: Versions: Python 3.6
process
Status: closed Resolution: not a bug
Dependencies: Superseder: subprocess documentation not explicit about fileno()
View: 19992
Assigned To: Nosy List: josh.r, martin.panter, rtpg, terry.reedy, xiang.zhang
Priority: normal Keywords:

Created on 2017-04-05 02:38 by rtpg, last changed 2017-04-10 07:20 by terry.reedy. This issue is now closed.

Messages (8)
msg291151 - (view) Author: Raphael Gaschignard (rtpg) Date: 2017-04-05 02:38
From the documentation of the io module:

fileno()
Return the underlying file descriptor (an integer) of the stream if it exists. An OSError is raised if the IO object does not use a file descriptor.

However, when passing a file-like object without a file descriptor (that raises OSError when calling f.fileno()) to POpen (for stdout, for example), the raised exception is not handled properly.

(However, on inspection of subprocess code, returning -1 will cause the code to handle this properly)

I'm not sure whether this is an issue in the io module documentation or in the subprocess code.


the core issue seems to be in POpen.get_handles, that seems to expect that -1 is used to signal "no file descriptor available".
msg291217 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-04-06 08:39
Hmm, what do you mean by 'the raised exception is not handled properly'? Currently the exception is just propagated to the user to signal the argument seems to be wrong, right?
msg291220 - (view) Author: Raphael Gaschignard (rtpg) Date: 2017-04-06 09:30
the subprocess module has code to handle file objects that don't use file descriptors. It's not that file descriptors are necessary for the parameter, but that the way POpen is trying to detect "no file descriptor support" is through "fileno returns -1" rather than "fileno raises OSError"
msg291296 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-04-07 21:10
'crash' means OS message rather than Python exiting with exception traceback and message.

Can you post a minimal reproducer?  What OS?  subprocess.Popen._get_handles is different on POSIX and windows, though both seem to call f.fileno() without try-except. 

All filenoes are initialized to -1, so it seems to me that either
a. all accesses should be wrapped with try-except: pass, or
b. subprocess doc should say that file-like objects must include a fileno method returning -1.

I am puzzled though.  The 2.7 doc for (builtin)file.fileno() says 
"
Note
File-like objects which do not have a real file descriptor should not provide this method! "
"
Rather than return -1

In must be that the subprocess test does not test with a 'file-like object without a file descriptor'
msg291308 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2017-04-08 00:09
Raphael: Can you point to the implementation code that handles file objects without a file descriptor (or give a demonstration of it)? I suspect there is no such support and you are mistaken.

Perhaps we can instead clarify in the “subprocess” documentation that “fileno” is required. Issue 19992 already proposes this.

Issue 1260171 and Issue 10482 are also related, about streaming data between Python file objects in the parent and pipes connected to the child.
msg291403 - (view) Author: Raphael Gaschignard (rtpg) Date: 2017-04-10 01:31
I'm sorry for the confusion here, it turns out I was misinterpreting the results of my program and my workaround was not working. Like others have said, the subprocess code seems to rely on the existence of file descriptors

I would be pretty partial to the idea of having the subprocess's internal code catch OSError and raise a clearer error message like "subprocess can only operate on files with file descriptor support".

Having "complete" file object support would seem like a major win, but looking at the internals/OS support it seems like a hard thing to set up.
msg291407 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2017-04-10 03:14
The current exception seems to give a reasonable hint:

>>> subprocess.Popen((executable,), stdout=BytesIO())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.5/subprocess.py", line 914, in __init__
    errread, errwrite) = self._get_handles(stdin, stdout, stderr)
  File "/usr/lib/python3.5/subprocess.py", line 1400, in _get_handles
    c2pwrite = stdout.fileno()
io.UnsupportedOperation: fileno

Perhaps we can close this in favour of fixing the documentation? (Issue 19992)
msg291409 - (view) Author: Raphael Gaschignard (rtpg) Date: 2017-04-10 04:33
that error is what is raised by StringIO (subclassing OSError), other file-like objects could just be raising OSError (according to the file object spec described in the documentation for the io module). 

Fixing documentation would already be helpful though. I will close this issue
History
Date User Action Args
2017-04-10 07:20:19terry.reedysetresolution: not a bug
2017-04-10 04:33:38rtpgsetstatus: open -> closed

messages: + msg291409
stage: test needed -> resolved
2017-04-10 03:14:39martin.pantersetmessages: + msg291407
2017-04-10 01:31:30rtpgsetmessages: + msg291403
2017-04-08 00:09:57martin.pantersetnosy: + martin.panter
messages: + msg291308

superseder: subprocess documentation not explicit about fileno()
stage: test needed
2017-04-07 21:10:52terry.reedysettype: crash -> behavior

messages: + msg291296
nosy: + terry.reedy
2017-04-06 17:53:05josh.rsetnosy: + josh.r
2017-04-06 09:30:44rtpgsetmessages: + msg291220
2017-04-06 08:39:02xiang.zhangsetnosy: + xiang.zhang
messages: + msg291217
2017-04-05 02:38:53rtpgcreate