It's unsafe to call remove_writer in a coroutine finally block, since the finally block can execute after the file descriptor has been closed and reopened for another purpose.
We've got this problem in portage.util.futures._asyncio.streams._writer coroutine.
In https://idea.popcount.org/2017-03-20-epoll-is-fundamentally-broken-22/ I've found this potentially relevant note about unregistration of closed file descriptors:
> epoll_ctl(EPOLL_CTL_ADD) doesn't actually register a file
> descriptor. Instead it registers a tuple1 of a file descriptor
> and a pointer to underlying kernel object. Most confusingly the
> lifetime of an epoll subscription is not tied to the lifetime of
> a file descriptor. It's tied to the life of the kernel object.
> Due to this implementation quirk calling close() on a file
> descriptor might or might not trigger epoll unsubscription. If
> the close call removes the last pointer to kernel object
> and causes the object to be freed, then it will cause epoll
> subscription cleanup. But if there are more pointers to kernel
> object, more file descriptors, in any process on the system,
> then close will not cause the epoll subscription cleanup. It
> is totally possible to receive events on previously closed
> file descriptors.
Patch posted for review:
The bug has been referenced in the following commit(s):
Author: Zac Medico <email@example.com>
AuthorDate: 2020-06-18 05:33:26 +0000
Commit: Zac Medico <firstname.lastname@example.org>
CommitDate: 2020-06-18 17:54:37 +0000
_writer: fix unsafe finally clause (bug 728580)
In the coroutine finally clause, do not call remove_writer in cases
where fd has been closed and then re-allocated to a concurrent
coroutine as in bug 716636.
Also, assume that the caller will put the file in non-blocking mode
and close the file when done, so that this function is suitable for
use within a loop.
Reviewed-by: Brian Dolbec <email@example.com>
Signed-off-by: Zac Medico <firstname.lastname@example.org>
lib/portage/util/futures/_asyncio/process.py | 11 ++++--
lib/portage/util/futures/_asyncio/streams.py | 50 +++++++++++++---------------
2 files changed, 33 insertions(+), 28 deletions(-)