io: split read/write ticks in ScheduledIo to fix poll_read livelock - #8221
io: split read/write ticks in ScheduledIo to fix poll_read livelock#8221ameyypawar wants to merge 1 commit into
ScheduledIo to fix poll_read livelock#8221Conversation
ScheduledIo packed a single generation counter (tick) shared by read and write readiness, advanced by the I/O driver on every event. A reader could observe read readiness at tick T, read WouldBlock, and call clear_readiness with that snapshot; if the driver processed a write-only event in between (advancing the shared tick to T+1), the clear saw a mismatch and refused to clear read readiness, so poll_read kept observing readiness while reads returned WouldBlock -- a livelock. This is observed on Windows, where reregistering read interest re-arms POLL_SEND and produces spurious writable events, but the shared-counter fragility is platform-independent. Split the counter into independent read_tick and write_tick. The driver advances each based on the direction(s) an event touches, and clear_readiness validates only the tick(s) for the direction(s) it clears, so a write-only event can no longer invalidate a read-side clear (and vice versa). The two 7-bit counters fit alongside readiness and shutdown in a usize on 32-bit and wasm targets. Per tokio-rs#6135 the single 8-bit tick predating that PR was already sufficient; split, each direction counts only its own events, leaving equal-or-greater headroom. Revives the approach from tokio-rs#8066. Closes tokio-rs#8054.
| // | shutdown | write tick | read tick | readiness | | ||
| // |----------+------------+-----------+-----------| | ||
| // | 1 bit | 7 bits | 7 bits | 16 bits | | ||
| // | ||
| // The driver maintains separate generation counters ("ticks") for the read and | ||
| // write directions. A clear of read readiness is validated only against the | ||
| // read tick (and write against the write tick), so a write-only notification | ||
| // cannot invalidate a pending read-side `clear_readiness`, which previously | ||
| // caused a livelock (see <https://github.com/tokio-rs/tokio/issues/8054>). | ||
| // | ||
| // Each tick is 7 bits so that both fit alongside the 16-bit readiness and the | ||
| // shutdown bit within a `usize` on 32-bit targets (including `wasm32`). The | ||
| // pre-#6135 design used a single 8-bit tick shared across both directions and | ||
| // was sufficient; with the counters split, each direction counts only its own | ||
| // (fewer) events, so 7 bits per direction leaves equal-or-greater headroom. |
There was a problem hiding this comment.
I'm concerned about making this change. Wraparound in the tick may result in I/O resource hangs similar to #6133, so I don't want to reduce the resolution as it significantly increases the risk of such hangs.
We should instead fix this by updating the reregister logic so that we do not request write readiness when the resource is already known to be ready for writing.
There was a problem hiding this comment.
Squeezing two counters into the tick lowers the wraparound margin, and #6133 is exactly the failure mode that makes that risky, so I'll drop the tick-split approach.
I traced the spurious write readiness: it's mio's Windows AFD re-arm. After a read WouldBlock, IoSourceState::do_io reregisters the socket's full stored interest (read+write), and since the socket is writable the AFD poll completes immediately with POLL_SEND — that's the write event that keeps invalidating the read-side clear_readiness. So the fix you described lands in mio's SockState re-arm rather than in Tokio.
There was a problem hiding this comment.
If the issue is in mio, do you mind submitting a PR for that?
There was a problem hiding this comment.
Yes, happy to. I'll open a PR against mio and link it here.
There was a problem hiding this comment.
Yes — tokio-rs/mio#2001, and mio#1963 before it. Sorry for the silence.
|
Closing — the fix belongs in mio (tokio-rs/mio#2001), not here. |
2 participants
Problem
ScheduledIopacks a single generation counter (tick) shared by read and write readiness; the I/O driver advances it on every mio event. A reader can observe read readiness at tickT, readWouldBlock, and callclear_readinesswith that snapshot — but if the driver processes a write-only event in between (advancing the shared tick toT+1), the clear sees a mismatch and refuses to clear read readiness.poll_readthen keeps observing readiness while reads returnWouldBlock: a livelock.This reproduces on Windows, where reregistering read interest re-arms
POLL_SENDand produces spurious writable events (see #8054 for @sandersaares's detailed trace), but the shared-counter fragility is platform-independent.Fix
Split the counter into independent
read_tickandwrite_tick. The driver advances each based on the direction(s) an event touches;clear_readinessvalidates only the tick(s) for the direction(s) it clears. A write-only event advanceswrite_tickonly and can no longer invalidate a read-side clear (and vice versa).The two 7-bit counters fit alongside the 16-bit readiness and shutdown bit in a
usizeon 32-bit/wasm. Per #6135, the single 8-bit tick predating that PR was already sufficient; split, each direction counts only its own (fewer) events, so headroom is equal-or-greater.Tests
scheduled_io.rs: a write-only event doesn't block a read clear, the symmetric case, and a same-direction event still invalidates a stale clear (the generation-counter protection is preserved). I confirmed the first scenario strands read readiness onmasterand is fixed here.#[cfg(loom)]test modeling the concurrent race (driver write event vs reader snapshot+clear), passing under all interleavings. It isn't wired into aloom.ymlscope yet (the readiness path has none — cf. io::driver should have loom tests #3018); happy to add one if preferred.Notes
Revives the approach from #8066 by @CrossEyedCat (self-closed before @sandersaares confirmed it fixes the livelock and improves latency).
I'm aware of the discussion in #8054 about addressing this in mio (only reregistering read readiness). This is complementary defense-in-depth: separating the counters is correct regardless of how often the driver reregisters, and resolves the observed livelock today.
Closes #8054.