Skip to content

windows: only re-arm the direction that blocked - #2001

Open
ameyypawar wants to merge 12 commits into
tokio-rs:masterfrom
ameyypawar:fix/1963-narrow-rearm
Open

windows: only re-arm the direction that blocked#2001
ameyypawar wants to merge 12 commits into
tokio-rs:masterfrom
ameyypawar:fix/1963-narrow-rearm

Conversation

@ameyypawar

@ameyypawar ameyypawar commented Aug 9, 2026

Copy link
Copy Markdown

A socket registered READABLE | WRITABLE gets a writable event on every blocked read: do_io reregisters the full stored interest, set_event re-adds POLL_SEND, and the AFD poll completes with it immediately. Downstream this livelocks a tokio reader (tokio-rs/tokio#8054).

do_io_with re-arms only the direction the caller operated on, through a new rearm that adds to the events already being waited for rather than replacing them — narrowing one direction must not drop readiness still wanted in the other. Applied to TcpStream, TcpListener and UdpSocket, the types that reach this path on Windows. try_io keeps using do_io, since a caller-supplied closure has no known direction. No public API change.

Verified on Windows CI: the added test fails on master and passes with this change.

The tests also fail on poll(2), event_ports(2) and WASI, which re-arm the same way; they are skipped there and left for a follow-up.

Fixes #1963

A socket registered `READABLE | WRITABLE` emits a spurious writable event
on every read. After a read returns `WouldBlock`, `IoSourceState::do_io`
reregisters the socket's full stored interest; `set_event` rewrites
`user_evts`, re-adding `POLL_SEND`, and since the socket is writable the
next AFD poll completes immediately with `POLL_SEND`. So each blocked read
produces a writable event nothing asked for.

Add `do_io_with`, which takes the interest the caller actually operated on
and re-arms only that direction, never widening past what the source is
registered for. Re-arming goes through a new `rearm`, which *adds* to the
events the socket is already waiting for instead of replacing them, so
narrowing one direction cannot drop readiness the caller still wants in
the other.

Only the Windows selector re-arms this way, so `IoSource::do_io_with` is
exactly `do_io` elsewhere.

Refs tokio-rs#1963
`set_event` rewrites `user_evts` wholesale, which is right when the user
changes their registered interest but wrong when re-arming after a blocked
operation: re-arming a single direction that way would drop the readiness
the caller is still waiting for in the other one.

`add_event` ORs the requested events into the ones already being waited
for. That composes correctly with `feed_event`, which clears delivered
events from `user_evts`, so re-arming restores exactly what was consumed
and leaves the other direction untouched.
`Read`, `Write` and `peek` know which direction they operate on, so pass
it through `do_io_with`. `try_io` keeps using `do_io`: it runs a
caller-supplied closure, so the direction is not knowable there.
Building with `net` but without `os-poll` selects the shell backend, which
also needs the method. Like `do_io` there, it holds no state and just calls
the function.
`IoSource::do_io_with` only forwards to the backend on Windows; everywhere
else it is `do_io`. Gate the shell method the same way so it is not dead
code on other targets.
Regression test for tokio-rs#1963. Fails on the Windows selector before the
re-arm is narrowed, because re-arming the full registered interest after a
blocked read re-requests write readiness, which AFD completes immediately
for a writable socket.
The `poll(2)` selector re-arms the same way and so has the same defect:
`SelectorState::reregister` overwrites the `pollfd` event mask, and
`registered_io_source` re-arms the full registered interest after any
blocked operation. Verified locally with
`--cfg mio_unsupported_force_poll_poll`, where this test reports an
unexpected writable event.

That path is shared with `event_ports(2)` and is left for a follow-up
rather than widening a Windows fix. Skip the test there instead of
silently dropping the invariant.
Windows was not the only selector affected. Every selector that re-arms a
source after a blocked operation re-arms the full registered interest, and
so reports write readiness after a blocked read:

  - `poll(2)`      reproduced locally with `--cfg mio_unsupported_force_poll_poll`
  - `event_ports(2)` reproduced on the Solaris CI job
  - WASI            reproduced on the wasm32-wasip2 CI job

All three report `Event { readable: false, writable: true }` from
`expect_no_events`, the same failure this test catches on Windows.

`epoll(7)` and `kqueue(2)` are edge triggered and never re-arm, so they
already satisfy the invariant.

Narrowing the other selectors means giving each one a re-arm that adds to
the requested events instead of replacing them, which is a larger change
than this Windows fix. Skip the test there for now rather than dropping
the invariant, and record where it still does not hold.
The comment claimed no other selector re-arms in a way that can raise
readiness for the direction the caller did not block on. That is wrong:
`poll(2)`, `event_ports(2)` and WASI all do, which is why the test is
skipped there. Say what is actually true -- they are affected but not
fixed here.

Also add a second case. The first only shows the narrowed re-arm does not
raise write readiness; on its own that would also pass if the re-arm did
nothing at all. This one shows the direction we narrowed *to* still works,
by having the peer send after the blocked read and expecting the readable
event.
Left over from an earlier version of the skip attribute; it fails clippy's
`-D warnings`.
`UdpSocket` goes through the same `IoSource` path as `TcpStream`, so on
Windows a blocked `recv`/`recv_from`/`peek` re-requested write readiness
just the same. Pass the direction through for the operations that know it.

`try_io` keeps using `do_io`, since the direction of a caller-supplied
closure is not knowable.
`accept` is a read-side operation, so a blocked one should not re-request
write readiness for the listener.
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.

Windows: re-arm on a read WouldBlock produces a spurious writable event

1 participant