Use PeekNamedPipe for Windows pipe read readiness#452
Merged
Conversation
The Windows path of run_xev_poll_read submitted a 0-byte ReadFile as a "readiness probe" via libxev. On IOCP a 0-byte ReadFile completes synchronously regardless of pipe state, so the parked reader G was woken immediately — before any data arrived. test_poller_pipe_read_delayed caught this: the writer thread sleeps 50ms, but the reader had already checked delayed_writer_done == 0 and stored read_done = -1. Replace that probe with a PeekNamedPipe-based poll driven by a recurring 5ms libxev timer. The slot bookkeeping (read_g, read_armed) is unchanged; the timer walks active slots, fires the readiness callback when avail > 0, and re-arms while any reader is parked. Preserves the readiness-based public API (callers still issue their own read to consume data) and is gated to builtin.os.tag == .windows so POSIX paths are untouched. WASI keeps the empty-buffer-read shape since there is no test coverage and PeekNamedPipe is unavailable there. Write polling is left untouched — test_poller_write_park is already #ifndef _WIN32; a follow-up can apply the same pattern.
The Windows PeekNamedPipe poll timer fires every 5ms regardless of
whether any registered pipe has data. With the previous single
loop.run(.once) call inside run_xev_tick_blocking, each spurious timer
fire caused the scheduler's run_poller_poll_blocking to return 0, which
the single-threaded scheduler treats as deadlock — leading to the
"all green threads are deadlocked" abort in
test_poller_pipe_read_delayed.
Loop run_xev_tick_blocking on loop.run(.once) until one of three
conditions holds:
1. The user-supplied timeout (if any) elapses.
2. A callback fired that woke a G (callbacks_fired advanced).
3. The cross-thread async wakeup was signaled (asyncNoop set
async_fired).
This preserves the existing semantics on Linux/macOS — the first
loop.run(.once) returns on actual I/O readiness or async wake, and
the loop predicate exits immediately — while letting the Windows path
quietly absorb the recurring poll-timer fires until real progress is
made.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Windows path of run_xev_poll_read submitted a 0-byte ReadFile as a
"readiness probe" via libxev. On IOCP a 0-byte ReadFile completes
synchronously regardless of pipe state, so the parked reader G was woken
immediately — before any data arrived. test_poller_pipe_read_delayed
caught this: the writer thread sleeps 50ms, but the reader had already
checked delayed_writer_done == 0 and stored read_done = -1.
Replace that probe with a PeekNamedPipe-based poll driven by a recurring
5ms libxev timer. The slot bookkeeping (read_g, read_armed) is unchanged;
the timer walks active slots, fires the readiness callback when avail > 0,
and re-arms while any reader is parked. Preserves the readiness-based
public API (callers still issue their own read to consume data) and is
gated to builtin.os.tag == .windows so POSIX paths are untouched. WASI
keeps the empty-buffer-read shape since there is no test coverage and
PeekNamedPipe is unavailable there. Write polling is left untouched —
test_poller_write_park is already #ifndef _WIN32; a follow-up can apply
the same pattern.