Don't select for read on character devices in _UnixWritePipeTransport #374
Conversation
_UnixWritePipeTransport selects for read on write pipes to detect when the remote end of the pipe is closed. This works for unidirectional FIFOs and dedicated socket pairs where nothing is written to the read side of the pair, but it can cause problems with devices or sockets where bidirectional I/O is being done. This commit changes _UnixWritePipeTransport to only select for read on sockets and FIFOs (on OSes which support that), and not on character devices. When connect_write_pipe() is used on those devices, end-of-file will need to be detected using a read pipe which accepts inputs from the device. No change is made to how sockets are handled, so passing in a socket used for bidirectional I/O to connect_write_pipe() is not supported. Async I/O on sockets should be performed using functions like BaseEventLoop.create_connection(). Socket pairs which are only used for undirectional I/O can be used here, though.
|
I think we have a couple of tests for |
|
The main thing that this change addresses is when the file object passed into connect_write_pipe() is a TTY object with both read & write happening on it. It looks like there are read & write pty tests already in test_events.py, but they don't specifically address the bidirectional I/O case. It should be possible to add another test for this, though. I'll see what I can put together. If I get something working, should I just add it to this same pull request? |
|
I've gone ahead and checked in a new unit test which exercises this change. Without the fix here, the unit test would fail, reporting that the write pipe had closed prematurely. With the fix, bidirectional I/O can be performed on PTY/TTY pair without an issue. |
|
I don't think the failed test here has anything to do with this commit. It's testing socket code, and from looking at the test my guess is that it's a race condition related to random port assignment on the test system. It assumes that the random port that was selected won't be in use when it connects after closing the listener, but something else could have come along and re-opened that port in the interim. |
While the Windows release of Python seems to include the 'tty' module, attempting to import it fails with an error about not finding the 'termios' module. Since the tty tests here aren't run on Windows, though, there's no need to import tty there. This commit makes the import conditional on the platform.
|
Is there anything more I can do on this? I signed the Python Contributor Agreement tonight. |
|
LGTM. |
|
Thanks, @ronf |
|
My pleasure. Thanks for merging the fix! |
…python#374) * Don't select for read on character devices in _UnixWritePipeTransport _UnixWritePipeTransport selects for read on write pipes to detect when the remote end of the pipe is closed. This works for unidirectional FIFOs and dedicated socket pairs where nothing is written to the read side of the pair, but it can cause problems with devices or sockets where bidirectional I/O is being done. This commit changes _UnixWritePipeTransport to only select for read on sockets and FIFOs (on OSes which support that), and not on character devices. When connect_write_pipe() is used on those devices, end-of-file will need to be detected using a read pipe which accepts inputs from the device. No change is made to how sockets are handled, so passing in a socket used for bidirectional I/O to connect_write_pipe() is not supported. Async I/O on sockets should be performed using functions like BaseEventLoop.create_connection(). Socket pairs which are only used for undirectional I/O can be used here, though. * Add new unit test for bidirectional I/O on TTYs * Don't import tty module on Windows While the Windows release of Python seems to include the 'tty' module, attempting to import it fails with an error about not finding the 'termios' module. Since the tty tests here aren't run on Windows, though, there's no need to import tty there. This commit makes the import conditional on the platform.
Upstream python/asyncio#374 by Ron Frederick. --HG-- branch : 3.5
Upstream python/asyncio#374 by Ron Frederick.
Here's a pull request designed to fix the issue reported in #369.
_UnixWritePipeTransport selects for read on write pipes to detect when
the remote end of the pipe is closed. This works for unidirectional FIFOs
and dedicated socket pairs where nothing is written to the read side of
the pair, but it can cause problems with devices or sockets where
bidirectional I/O is being done.
This commit changes _UnixWritePipeTransport to only select for read on
sockets and FIFOs (on OSes which support that), and not on character
devices. When connect_write_pipe() is used on those devices,
end-of-file will need to be detected using a read pipe which accepts
inputs from the device.
No change is made to how sockets are handled, so passing in a socket
used for bidirectional I/O to connect_write_pipe() is not supported.
Async I/O on sockets should be performed using functions like
BaseEventLoop.create_connection(). Socket pairs which are only
used for undirectional I/O can be used here, though.