Repository navigation
Conversation
AsyncIOCP kept its guard, wakeup flag and waiter inside the struct, so a copy of an Async taken before wait() (for example one handed to another thread to notify with) never saw the waiter: notify() on the copy only set the copy's own flag and the loop was never woken. The eventfd and mach port backends do not have this problem because copies share one kernel object. Ghostty hits this on Windows: the io thread notifies a by-value copy of the renderer thread's wakeup, so terminal output is drawn only on the next cursor blink timer tick, 0-600 ms late. Move the state behind a pointer allocated in init() and freed in deinit(), so every copy shares it, as on the other backends. Only one copy may be deinited, as with those handles. A Windows event that copies share, associated with the loop's completion port by NtAssociateWaitCompletionPacket, would avoid the allocation, but needs an ntdll call the SDK does not document and a new completion kind in the IOCP backend. Adds a test that notifies through a copy taken before wait(); it fails without this change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
1 participant
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.
On Windows, xev.Async breaks once you copy it. The IOCP version keeps its state (the lock, the “a notify came in early” flag, and the waiting completion) inside the struct itself. So if you copy an Async before calling wait(), the copy has its own private state, and notify() on it never wakes the loop. The eventfd and mach port versions don’t have this problem, because every copy points at the same kernel object.
Ghostty runs into this: it gives its IO thread a copy of the renderer’s Async. On Windows that means new terminal output only shows up on the next cursor blink, up to ~600 ms late. You mentioned in ghostty-org/ghostty#12050 that the fix should be making Async copy-by-value in libxev on Windows, so here’s a go at that.
The fix moves the state into a small heap allocation made in init() and freed in deinit(), so all copies share it, like the other backends share their fd or port. As with those, only one copy should call deinit().
I know libxev doesn’t really allocate anywhere else, so I wanted to flag it. The no-allocation option I looked at was a Windows event that copies share, tied to the loop’s completion port with NtAssociateWaitCompletionPacket. But that’s an undocumented ntdll call and would need a new completion type in the IOCP backend, so it felt like too much for this fix. Happy to go that way if you’d prefer it.
Testing:
Added a test that notifies through a copy taken before wait(). It fails without the fix and passes with it.
Windows 11, x86_64 and ARM64: 51 passed, 10 skipped, 0 failed
macOS: all pass
Didn’t run Linux locally; CI should cover it
AI disclosure: I used Claude Code to track down the bug, write the fix and test, and run the Windows tests. I’ve reviewed the change and understand it.