[issue1191964] add non-blocking read and write methods to subprocess.Popen

[STINNER Victor]

Change by STINNER Victor :


--
resolution:  -> out of date

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1191964] add non-blocking read and write methods to subprocess.Popen

[Josiah Carlson]

Josiah Carlson  added the comment:

Someone else can resurrect this concept and/ore patch if they care about this 
feature. Best of luck to future readers.

--
stage: test needed -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1191964] add non-blocking read and write methods to subprocess.Popen

[Martin Panter]

Martin Panter added the comment:

Yes, I think the special return value of None is only standard to the “io” 
module, which was added in Python 2.6 and 3. And even there it is inconsistent.

For what it’s worth, don’t have strong opinions on the special non-blocking and 
broken pipe cases, as long as they can be differentiated. So the current 
situation is fine by me. Just that potentially surprising differences with 
other APIs [e.g. read() -> b"" means empty buffer, not closed pipe] need 
documenting.

The asyncio.StreamReader.read() etc methods are documented as coroutines, so 
the problem of differentiating an empty buffer from a closed pipe does not 
exist there.

Just a thought: would it make sense for these methods to be decoupled from the 
subprocess class, and generalized to apply to any pipes, as a kind of Windows 
vs POSIX abstraction layer? Then you wouldn’t have to add duplicate methods 
like read_stderr_available() and read_stderr_exactly() all the time. You might 
also have a more portable way of doing non-blocking reads from sys.stdin, etc 
as well.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1191964] add non-blocking read and write methods to subprocess.Popen

[Josiah Carlson]

Josiah Carlson added the comment:

Non-blocking IO returning a None on "no data currently available" is new to me. 
It is new to me simply because I don't recall it ever happening in Python 2.x 
with any socket IO that I ever dealt with, and socket IO is my primary source 
for non-blocking IO experience.

>From my experience, reading from a socket would always return a string, unless 
>the connection was closed/broken, at which point you got a bad file descriptor 
>exception. Writing would return 0 if the buffer was full, or a bad file 
>descriptor exception if the connection was closed/broken. The implementation 
>of asyncore/asynchat at least until 3.0 shows this in practice, and I doubt 
>this has changed in the intervening years since I updated those libraries for 
>3.0.

My choice to implement this based on BrokenPipeError was based on reading 
Victor(haypo)'s comments from 2014-07-23 and 2014-07-24, in which he stated 
that in asyncio, when it comes to process management, .communicate() hides 
exceptions so that communication can finish, but direct reading and writing 
surface those exceptions.

I was attempting to mimic the asyncio interface simply because it made sense to 
me coming from the socket world, and because asyncio is where people are likely 
to go if the non-blocking subprocess support in subprocess is insufficient for 
their needs.

Note that I also initially resisted raising an exception in those methods, but 
Victor(haypo)'s comments changed my mind.

> Do you know a better method that would allow to distinguish between EOF (no 
> future data, ever) and "would block" (future data possible) without calling 
> process.poll()?

Better is subjective when it comes to API, but different? Yes, it's already 
implemented in patch #8. BrokenPipeError exception when no more data is 
sendable/receivable. A 0 or b'' when writing to a full buffer, or reading from 
an empty buffer. This API tries to be the same as the 
asyncio.subprocess.Process() behavior when accessing stdin, stdout, and stderr 
directly.

> Returning None for non blocking I/O is standard in Python.

In some contexts, yes. In others, no. The asyncio.StreamReader() coroutines 
.read(), .readline(), and .readexactly() return strings. Raw async or sync 
sockets don't return None on read. SSL-wrapped async or sync sockets don't 
return None on read. Asyncio low-level socket operations don't yield None on a 
(currently empty) read.


In the context of the subprocess module as it exists in 3.4 (and 3.5 as it is 
unpatched), the only object that returns None on read when no data is 
available, but where data *might* be available in the future, is an underlying 
posix pipe (on posix platforms) - which isn't generally used directly.

The purpose of this patch is to expose stdin, stdout, and stderr in a way that 
allows non-blocking reads and writes from the subprocess that also plays nicely 
with .communicate() as necessary. Directly exposing the pipes doesn't work due 
to API inconsistencies between Windows and posix, so we have to add a layer.

I would argue that this layer of non-blocking (Windows and posix) pipe 
abstraction has much more in common with how asyncio deals with process IO, or 
how sockets deal with IO, than it does with pipes in general (the vast majority 
of which are blocking), so I would contend that it should have a similar API 
(no returning None).

That said, considering that the expected use of non-blocking subprocess 
communication *does not* include using a multiplexer (select, poll, etc.), I 
have the sense that a few other higher-level methods might be warranted to ease 
use substantially, and which I would expect people would end up using much more 
often than the lower-level direct read/write operations (different names might 
make more sense here):
.write_all(data, timeout=None)
.read_available(bufsize=-1) (and .read_stderr_available)
.read_exactly(bufsize, timeout=None) (and .read_stderr_exactly)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1191964] add non-blocking read and write methods to subprocess.Popen

[Martin Panter]

Martin Panter added the comment:

Josiah’s code now seems to handle the four special cases like this:

* Reading a closed pipe raises BrokenPipeError. This is novel to me, and bends 
the current definition of the exception. But it appears to be the way Windows 
works at a lower level, and does make a bit of sense to me. However it is 
inconsistent with how normal reading from files, pipes and sockets works, which 
return the special value b"" in the equivalent case. Also various 
communications protocols use an empty packet to signal EOF.

* Reading an empty pipe returns b"". This is consistent with partial reads, 
which I like. However it could be surprising for someone used to b"" signalling 
EOF. Alternatives are returning None like RawIOBase.read(), and raising 
BlockingIOError like os.write() and socket.send().

* Writing to a closed pipe raises BrokenPipeError. This is consistent with how 
other APIs work, but since it is an asynchronous condition, a naive program may 
work sometimes without handling it, and then see suprious tracebacks or worse 
in other situations. Maybe dropping the data and setting a flag could be an 
alternative.

* Writing to a full pipe returns zero. This is consistent with partial writes, 
which I like. Alternatives are returning None like RawIOBase.write(), and 
raising BlockingIOError like os.write(), socket.send(), BufferedIOBase.write().

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1191964] add non-blocking read and write methods to subprocess.Popen

[STINNER Victor]

STINNER Victor added the comment:

Returning None for non blocking I/O is standard in Python.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1191964] add non-blocking read and write methods to subprocess.Popen

[Akira Li]

Akira Li added the comment:

> I'm going to be honest; seeing None being returned from a pipe read feels 
> *really* broken to me. When I get None returned from an IO read operation, my 
> first instinct is "there can't be anything else coming, why else would it 
> return None?"

It is how it is done in similar cases (returning `None` to indicate 
"would block"). Do you know a better method that would allow to 
distinguish between EOF (no future data, ever) and "would block"
(future data possible) without calling process.poll()?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1191964] add non-blocking read and write methods to subprocess.Popen

[Josiah Carlson]

Josiah Carlson added the comment:

I'm going to be honest; seeing None being returned from a pipe read feels 
*really* broken to me. When I get None returned from an IO read operation, my 
first instinct is "there can't be anything else coming, why else would it 
return None?"

After changing writing to raise BrokenPipeError on a closed pipe, I've also 
changed reading to do the same and added a test to verify the behavior.

--
Added file: http://bugs.python.org/file38691/subprocess_8.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1191964] add non-blocking read and write methods to subprocess.Popen

[Martin Panter]

Martin Panter added the comment:

Regarding special cases on the reading side, I think there should be an easy 
way to distinguish them. The existing RawIOBase way is probably as good as any, 
unless you want to take the chance to invent a better API:

* EOF (pipe closed, no more data ever) should be indicated by returning b""
* Empty pipe (still open, future data possible) should be indicated by 
returning None

Beware that BlockingIOError is normally not raised by Python’s standard 
library, despite the BufferedIOBase documentation, and I think it should say 
that way (the implementation, that is). See Issue 13322.

Regarding handling a broken pipe condition, it would be nice to have a way of 
signalling that to the caller, whether via an exception or other means. For 
example if the subprocess is a video player which gets closed early by the end 
user, it would be good to know to stop wasting time and bandwidth generating 
and piping video the the player.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1191964] add non-blocking read and write methods to subprocess.Popen

[Josiah Carlson]

Josiah Carlson added the comment:

Okay, I'm sorry for falling asleep at the wheel on this. At this point, I don't 
expect this to go in for 3.5 - but if it has a chance, I'll do what I need to 
do to get it there. Let me know.

I agree with the .communicate() not raising on broken pipe, and 
read/write_nonblocking raising on broken pipe. I'll get a patch in with the 
comments on the existing code review, along with those changes on Monday or 
Tuesday.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1191964] add non-blocking read and write methods to subprocess.Popen

[STINNER Victor]

STINNER Victor added the comment:

> Do Popen.write_nonblocking() and Popen.read_nonblocking() methods
> belong to the second category? Should they raise BrokenPipeError?

IMO when you write directly to stdin and read from stdout/stderr of a
child process, your code should be written to handle BrokenPipeError.
You must decide how to handle them. Otherwise, you have to poll
manually the child process using proc.poll() which is less efficient.

If you forget to poll the process, your program may enter an unlimited
loop which only occur in some cases (bug in the child process which
exits before reading the whole stdin input).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1191964] add non-blocking read and write methods to subprocess.Popen

[akira]

akira added the comment:

STINNER Victor  writes:
>
>> I have implemented and would continue to lean towards continuing to
> hide BrokenPipeError on the additional API endpoints.
>
> FYI asyncio.Process.communicate() ignores BrokenPipeError and
> ConnectionResetError, whereas asyncio.Process.stdin.drain() (coroutine
> to wait until all bytes are written) raises a BrokenPipeError or
> ConnectionResetError if the child process exited. I think subprocess
> has the same design.

Do Popen.write_nonblocking() and Popen.read_nonblocking() methods
belong to the second category? Should they raise BrokenPipeError?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1191964] add non-blocking read and write methods to subprocess.Popen

[STINNER Victor]

STINNER Victor added the comment:

> I have implemented and would continue to lean towards continuing to hide 
> BrokenPipeError on the additional API endpoints.

FYI asyncio.Process.communicate() ignores BrokenPipeError and 
ConnectionResetError, whereas asyncio.Process.stdin.drain() (coroutine to wait 
until all bytes are written) raises a BrokenPipeError or ConnectionResetError 
if the child process exited. I think subprocess has the same design.

(I modified recently asyncio to ignore BrokenPipeError in communicate(), it was 
a bug.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1191964] add non-blocking read and write methods to subprocess.Popen

[STINNER Victor]

Changes by STINNER Victor :


--
title: asynchronous Subprocess -> add non-blocking read and write methods to 
subprocess.Popen

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com