Open sorcio opened 1 year ago
I can make a PR if this is accepted as a bug.
I think that it makes sense to catch exceptions on _add_write(). I suppose that it would look like something like that:
except Exception as exc:
self._conn_lost += 1
self._fatal_error(exc, 'Fatal write error on pipe transport')
return
It looks like calling self._loop._remove_reader() in _close() is fine, since it should do nothing if _add_writer() failed.
Related discussion from ten years ago in bpo-19293 (search for "EPIPE") and bpo-19294. The decision at the time was to use a socket pair instead of a pipe for stdin because some platforms (AIX and Darwin primarily) did not behave as expected, and it was deemed as a platform bug that Python could not solve. This was partially reverted recently (bpo-46364) by switching back to using pipes instead of socket pairs for non-AIX platforms.
Darwin changed behavior (since macOS 10.15) and now they say that registering a kqueue event on a half-closed pipe should not be an error.
As shown in the reproducer, the error does not only happen with a subprocess, because a pipe created elsewhere can be attached to asyncio. Going back to using a socketpair for FreeBSD instead of a pipe (like it used to be until before Python 3.11) could be an easy way out for subprocesses, but does not solve the problem for pipes.
Knowing that Apple fixed this, I'm more convinced that this shouldn't be handled in higher-level code. I will take a moment to consider the impact of changing this in KqueueSelector. Otherwise I will prepare a fix for asyncio.
While looking into this I found out something that looks like a bug in KqueueSelector
. When a fd is registered for both read and write, kqueue requires to register two filters; but KqueueSelector.select() will count the fd as only one (because it's one key) and call kqueue.control() with the smaller number. Therefore some events might not be returned, which could lead to worsened latency and possibly deadlock.
While looking into this I found out something that looks like a bug in KqueueSelector. When a fd is registered for both read and write, kqueue requires to register two filters; but KqueueSelector.select() will count the fd as only one (because it's one key) and call kqueue.control() with the smaller number. Therefore some events might not be returned, which could lead to worsened latency and possibly deadlock.
Oh wow, that sounds bad!
Open a separate issue and opened a PR for this https://github.com/python/cpython/issues/110038
Bug report
Bug description:
Earlier discussion: https://github.com/python/cpython/pull/109710#discussion_r1334470510
This issue is specific to some systems with kqueue. I can reproduce on FreeBSD, and according to the internet it should be reproduced on older macOS versions (10.11) and on OpenBSD/NetBSD (with a different error code). A current version of macOS (13) is not affected.
Minimal snippet to explain the root issue:
Running this code on Linux or macOS will not raise any exception. The last call to
sel.register()
will return a list with an event, signaling that a write on the FD is possible. A subsequent write will, of course, raise aBrokenPipeError
. On FreeBSD,sel.register()
will raise directly.asyncio does not account for this platform difference. This reproducer is a bit contrived because it needs to trigger a race condition[^1]:
[^1]: the other end of the pipe needs to be closed after
_UnixWritePipeTransport.write()
callsos.write()
but before it tries to register a writer.On most platform this will run indefinitely. On FreeBSD, after a few run I get a traceback:
The issue here is that
transport.write()
is not supposed to raise, as far as I understand.The current implementation of
_UnixWritePipeTransport.write()
catches all exceptions onos.write()
but not onself._loop._add_writer()
. ~The same happens switching read and write ends.~ Probably the issue was not detected before because it's a rare condition and it doesn't happen on Linux. gh-109709 showed this occurring on a subprocess test case. The fix applied in that case works by wrapping the call to write() in an exception handler, but I think in general user code can't be expected to always catch that error.I found prior discussion of the equivalent issue in Tokio. In their case, they decided to solve this at an abstraction level that is closer to
KqueueSelector.register()
, by ignoring the EPIPE and instead reporting the fd as ~readable~/writable (which is what users expect in other selectors, and what happens in modern macOS). libevent also does something similar. I wonder if this could be a valid solution for Python, because the actual work is done in selectmodule which is lower level. By contrast, to the best of my understanding, libuv does not have special handling for this.It can also be caught in asyncio (at some layer, either in selector event loops, or in code that calls ~add_reader~/add_writer). Given that this is only seems to happen with pipes, it could make sense to handle this in pipe-specific code.
I can make a PR if this is accepted as a bug.
cc @vstinner
Edit note: my mistake, the error is not raised when registering the read end of a pipe, only the write end.
CPython versions tested on:
3.12, CPython main branch
Operating systems tested on:
Other