Skip to content

fix(tokio,async_io): keep a waker armed after draining to WouldBlock - #35

Open
glamberson wants to merge 1 commit into
ids1024:masterfrom
lamco-admin:fix/event-stream-waker
Open

glamberson wants to merge 1 commit into
ids1024:masterfrom
lamco-admin:fix/event-stream-waker

Conversation

@glamberson

Copy link
Copy Markdown
Contributor

EiEventStream::poll_next drains the socket to WouldBlock, clears readiness,
and returns Pending directly, in both the tokio and async-io backends.
Neither poll_read_ready_mut nor Async::poll_readable stores a waker on the
call that already found the descriptor ready: per their own documentation, a
Ready return only means an edge was delivered since the last Pending, and the
waker is armed only on the call that actually finds nothing ready. So a poll
that goes ready, drains, clears, and returns Pending directly leaves no waker
registered for whatever arrives next.

I hit this in a production RDP server using the tokio backend to drive an EIS
handshake against Mutter's RemoteDesktop portal: the connection went to sleep
with the compositor's reply already sitting unread on the socket, roughly one
activation in fifty, and since the handshake runs on the accept task, every
later client sat in the listen backlog until the process was restarted.

The fix loops back to poll_read_ready_mut (poll_readable for async-io) after
clear_ready() instead of returning Pending directly, so the call that ends up
returning Pending is always the one that arms a waker. This is also the shape
tokio's own internal Registration::poll_io already uses for the same
read-drain-clear pattern (runtime/io/registration.rs), just not one
EiEventStream reimplements by hand. Applied to both backends since they share
the same shape.

fmt, clippy --all-features --all-targets -Dwarnings (and the non-stable
--all-features variant), cargo check, cargo build --all-features --examples,
and cargo doc --deny warnings (with and without --all-features) all pass.
There's no unit-test harness in the crate for this path; I could not get a
small standalone repro to reliably discriminate old vs fixed code (the race
needs the scheduling of a busier multi-task runtime, consistent with it only
firing about 2% of the time in practice), so I'm not including one. The
diagnosis rests on the documented poll contracts cited above plus the
production fix already deployed against the real symptom.

EiEventStream::poll_next drains the socket to WouldBlock, clears readiness,
and returns Pending directly, in both the tokio and async-io backends.
Neither poll_read_ready_mut nor Async::poll_readable stores a waker on the
call that already found the descriptor ready: Per their own documentation, a
Ready return only means an edge was delivered since the last Pending, and the
waker is armed only on the call that actually finds nothing ready. So a poll
that goes ready, drains, clears, and returns Pending directly leaves no waker
registered for whatever arrives next.

I hit this in a production RDP server using the tokio backend to drive an EIS
handshake against Mutter's RemoteDesktop portal: The connection went to sleep
with the compositor's reply already sitting unread on the socket, roughly one
activation in fifty, and since the handshake runs on the accept task, every
later client sat in the listen backlog until the process was restarted.

The fix loops back to poll_read_ready_mut (poll_readable for async-io) after
clear_ready() instead of returning Pending directly, so the call that ends up
returning Pending is always the one that arms a waker. This is also the shape
tokio's own internal Registration::poll_io already uses for the same
read-drain-clear pattern (runtime/io/registration.rs), just not one
EiEventStream reimplements by hand. Applied to both backends since they share
the same shape.
@glamberson glamberson changed the title tokio,async_io: keep a waker armed after draining to WouldBlock fix(tokio,async_io): keep a waker armed after draining to WouldBlock Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant