Skip to content

FilePoll: support readable+writable on one poll (fixes c-ares TCP DNS assertion) - #29955

Merged
Jarred-Sumner merged 6 commits into
mainfrom
jarred/filepoll-bidirectional
Apr 30, 2026
Merged

FilePoll: support readable+writable on one poll (fixes c-ares TCP DNS assertion)#29955
Jarred-Sumner merged 6 commits into
mainfrom
jarred/filepoll-bidirectional

FilePoll: fix macOS kevent error-check ordering; cut dns.zig resync s…

30af659
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Apr 30, 2026 in 29m 57s

Code review found 2 potential issues

Found 6 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 1
Severity File:Line Issue
🟡 Nit src/async/posix_event_loop.zig:1191-1196 EV_ERROR check uses == instead of bitwise AND
🟣 Pre-existing src/bun.js/api/bun/dns.zig:2557-2562 Windows uv_events precedence drops writable when both directions requested

Annotations

Check warning on line 1196 in src/async/posix_event_loop.zig

See this annotation in the file changed.

@claude claude / Claude Code Review

EV_ERROR check uses == instead of bitwise AND

These per-entry checks use `flags == std.c.EV.ERROR`, but xnu ORs `EV_ERROR` into the existing flags (`kev->flags |= EV_ERROR`), so a failed `EV_DELETE` comes back as `EV_DELETE | EV_ERROR` (0x4002) and the equality test never matches — should be `(changelist[i].flags & std.c.EV.ERROR) != 0`, matching `packages/bun-usockets/src/eventing/epoll_kqueue.c`. Practical impact is near-zero (the only realistic `EV_DELETE` error is `ENOENT`, and callers discard the result anyway), but since this PR expli

Check notice on line 2562 in src/bun.js/api/bun/dns.zig

See this annotation in the file changed.

@claude claude / Claude Code Review

Windows uv_events precedence drops writable when both directions requested

Pre-existing, but worth noting since it's the same bug this PR fixes for POSIX: the Windows branch of `onDNSSocketState` (line 2530) has `if (readable) uv.UV_READABLE else 0 | if (writable) uv.UV_WRITABLE else 0`, which Zig parses as `if (readable) UV_READABLE else (0 | if (writable) UV_WRITABLE else 0)` — when c-ares wants both directions (TCP DNS), `uv_events` ends up as just `UV_READABLE` and writable interest is dropped. Should be `(if (readable) uv.UV_READABLE else 0) | (if (writable) uv.UV