io(kqueue): unregister before handing the owner off on close, and bit-test EV_ERROR by robobun · Pull Request #37791 · oven-sh/bun

robobun · 2026-08-12T08:01:29Z

Problem

On the kqueue side of the io request loop (macOS and FreeBSD), closing a polled fd put its EV_DELETE into the next batched kevent() call but handed the owning ReadFile/WriteFile to the work pool first, so the owner could be finished and freed before the delete was submitted.
Registrations are one-shot, so that delete normally fails (ENOENT, or EBADF once the pool closed the fd). The kernel returns the failure as an event whose udata still points into that owner, and the loop dispatched it (on_io_error on FreeBSD, on_ready on macOS) into an object another thread owned or had already freed.
The error check was event.flags == EV_ERROR. FreeBSD replaces the flags word with EV_ERROR; xnu ORs it in, so on macOS a rejected registration would have been dispatched as ready rather than as an error.
Found by reading the code; there is no crash report. On FreeBSD the close path is the normal completion of every polled read or write, but FreeBSD has no test lane; on macOS neither defect is known to be reachable today.

Fix

The close arm now applies the EV_DELETE on the spot with its own kevent() call (nevents = 0), ignores the expected ENOENT/EBADF, and only then hands the owner off, which is what the epoll arm already does. Property to check: once the owner belongs to another thread, kqueue holds nothing that could produce a reply for it.
Because a close no longer dispatches anything, the close request drops its tag; the tag lives on the readable/writable registrations and a cancel is submitted with udata = 0.
The error check becomes a bit test, (flags & EV_ERROR) != 0, the same change Fix panic when FilePoll unregister fails on macOS #31701 made to the sibling event loop. A rejected registration then reports its errno the way an epoll_ctl failure already does.
Verification: the new source lint fails on the unfixed tree and passes with the fix. The new FIFO test drives the kqueue registration path but passes before and after. The close-arm defect has no failing test (unreachable on macOS, no FreeBSD lane); cargo check and clippy were run for the darwin, freebsd and windows targets.

Background

IoRequestLoop (src/io/lib.rs) is the io thread that Bun.file(fd).text() and Bun.write() park on when a pipe, FIFO or socket is not ready; epoll on Linux, kqueue on macOS and FreeBSD. The reads and writes themselves run on the work pool, so the loop must be done with an owner before handing it over.
kevent() takes a changelist (adds and deletes) and returns an eventlist in one call. The loop batches a tick's changes into the same call it then blocks in; with nevents = 0 the kernel applies the changes and returns at once.
A change the kernel rejects is not an errno. It comes back as an eventlist entry with EV_ERROR set in flags, the errno in data, and the caller's udata, so it flows through the same dispatch as a real readiness event.
udata is a tagged pointer to the owner's io_poll field; the tag says whether the owner is a ReadFile or a WriteFile, and dispatch uses it to recover the owner and call into it. Whatever udata a change carries is where its error lands.
EV_ONESHOT registrations are removed by the kernel when they fire, and the loop keeps its own readable/writable flags set afterwards, which is why a later delete of the same registration usually fails.

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/io/bun-write.test.js

Original description

What

Two defects in the kqueue side of IoRequestLoop (src/io/lib.rs), the request loop behind Bun.file(...).text() / Bun.write() on fds that have to be polled (pipes, FIFOs, sockets). Found by reading the code; there is no crash report. FreeBSD is in the Rust target matrix (CI builds it) but has no test lane, and on macOS neither path is known to be reachable today (see "Reachability").

1. The Close arm hands the owner away while its EV_DELETE is still in flight.
tick_kqueue batches all changes of one tick into the single kevent() call it then blocks in. Its Action::Close arm appended the owner's EV_DELETE to that batch (udata = the owner's io_poll) and immediately called close.on_done. on_done (FileCloser::on_io_request_closed) schedules the owner on the work pool; the pool runs on_close_io_request -> update -> on_finish -> do_close, closes the fd and calls io_task.finish(), after which the JS thread frees the ReadFile/WriteFile. All of that can happen before the io thread reaches the kevent() that submits the delete.

Both registrations are EV_ONESHOT and nothing clears PollReadable/PollWritable when the one-shot fires, so by the time a close arrives the knote is already gone and the delete fails: ENOENT, or EBADF if the pool thread closed the fd first. A failed change is returned in the eventlist with EV_ERROR and the udata we submitted, i.e. a pointer into the handed-off owner, and on_update_kqueue dispatched it: on FreeBSD as on_io_error (which overwrites and re-schedules the owner's task while the pool may be running it, or after it was freed), on xnu as on_ready (same thing via the other entry point). Either way it is a dispatch into an object another thread owns or has already freed. The epoll arm does not have this problem: it applies EPOLL_CTL_DEL synchronously and ignores the result before calling on_done.

2. on_update_kqueue compared the whole flags word against EV_ERROR.
The kernels differ in how they report a rejected change:

// FreeBSD sys/kern/kern_event.c, kqueue_kevent()
kevp->flags = EV_ERROR;
kevp->data = error;
// xnu bsd/kern/kern_event.c, kevent_register()
kev->flags |= EV_ERROR;
kev->data = error;

So on macOS the reply to a rejected EV_ADD|EV_ONESHOT has flags == 0x4011, event.flags == libc::EV_ERROR is false, and a rejected registration would be dispatched as on_ready instead of on_io_error. posix_event_loop.rs already bit-tests for exactly this reason (#31701); this site was the remaining equality test, inherited from the Zig original (io.zig, event.flags == std.c.EV.ERROR).

Fix

The Close arm now does what the epoll arm does: Poll::unregister_kqueue applies the EV_DELETE on the spot with its own kevent(..., nevents = 0) call (both kernels return as soon as the changelist is processed when nevents is 0; FreeBSD kqueue_scan returns immediately for maxevents == 0, xnu skips the scan for nevents == 0), ignoring the expected ENOENT/EBADF, and only then calls on_done. Once the owner belongs to another thread nothing about the close is pending anywhere, so there is no reply that could be delivered into it, and the order of on_done relative to the next batch no longer matters. The first revision of this PR instead kept the delete in the batch and made it anonymous (udata = 0); that also works, but its safety rested on an argument about what the kernel can hand back, and review rightly asked for code that does not need the argument.
Because a close no longer has anything to dispatch, CloseAction loses its tag field and the tag moves into ApplyAction::Readable(tag) / Writable(tag); Cancel writes udata = 0. FileCloser::IO_TAG had no other consumer and goes with the field (trait, impl_file_closer!, and the Windows ReadFileUV impl).
on_update_kqueue: (event.flags & libc::EV_ERROR) != 0. With that, a rejected registration takes the path the epoll arm already takes on epoll_ctl failure: on_io_error records the errno and the pool task finishes with it; on macOS close_after_io is cleared on the way there (ReadFile::on_io_error, WriteFile::do_write_loop_task), so the error reaches JS without going through the Close arm.

Reachability

Defect 1: on FreeBSD the Close arm is the normal completion path of every ReadFile/WriteFile that waited on the io thread (wait_for_readable/wait_for_writable set close_after_io, and only #[cfg(target_os = "macos")] code in ReadFile::on_ready/on_io_error and WriteFile::do_write_loop_task clears it), e.g. Bun.stdin.text() from a pipe. On macOS those cfg blocks clear close_after_io, so the Close arm is not reached today.

Defect 2: not known to be reachable on macOS today either. Every registration is preceded by a poll() pre-check (bun_core::is_readable/is_writable, called from read_file.rs and write_file.rs before each wait_for_*), and xnu's poll() (bsd/kern/sys_generic.c) is implemented by registering the same kqueue filters, reporting an fd they reject as POLLNVAL, which those helpers count as ready. So the fds kqueue would reject never reach apply_kqueue; what is left is a registration failing after a successful pre-check (fd closed underneath us, resource exhaustion). On FreeBSD the exact comparison happened to be right. The bit test is the correct classification rather than a fix for an observed failure; it is the same change #31701 made to the sibling loop, and the lint below is what keeps it.

Tests

test/internal/source-lints/kevent-ev-error-equality.test.ts bans comparing a flags word against EV_ERROR (any spelling) in src/**/*.rs. Bit tests, including the (flags & EV_ERROR) == EV_ERROR spelling, are erased before looking for a comparison, so a whole-word compare is reported even on a line that also bit-tests; comments are cut per line only (stripping /* */ spans let a /* inside a line comment blind an earlier revision of this file to the Darwin section of src/sys/lib.rs); the liveness check names the files that must contain bit tests (src/io/lib.rs among them) and the self-check covers the accepted and rejected spellings. On the unfixed tree it reports src/io/lib.rs:1623: if event.flags == libc::EV_ERROR { (and src/io/lib.rs missing from the bit-test files); it passes with this change. This is the test that fails before and passes after.
test/js/bun/io/bun-write.test.js, "Bun.write and Bun.file(fd).text() on a non-blocking FIFO wait for each other": a ReadFile started on an empty FIFO and a WriteFile given 256 KiB (more than any platform's pipe buffer) on a second fd of the same FIFO, resolving against each other. With the io loop's debug logging on, one run of this body on Linux shows 8 readable and 11 writable registrations and 54 ready dispatches; on the macOS lanes the same body goes through apply_kqueue(Readable(tag)) / apply_kqueue(Writable(tag)) and on_update_kqueue's ready branch, i.e. the code restructured here. Nothing else in the suite drove wait_for_writable (the existing Bun.write(Bun.stdout, Bun.stdin) test is a CopyFile). This test passes before and after the change on every platform; it is runtime coverage for the refactored registration path, not the discriminator. Two fds because epoll registers a given fd once; destination.size because Bun.write() only treats an fd destination as pollable once its type has been resolved (without it the writer currently spins on a stale EAGAIN instead of waiting; that pre-existing bug, which also affects Bun.write(Bun.stdout, ...) onto a non-blocking pipe and Bun.write(fifoPath, ...), turned up while writing this test and is tracked separately).

The Close arm itself (defect 1) cannot be exercised by a test that fails before the fix anywhere CI runs: the code is compiled only for macOS and FreeBSD, it is unreachable on macOS, and there is no FreeBSD lane.

Verification

cargo check -p bun_runtime (pulls in bun_io) for x86_64-apple-darwin, aarch64-apple-darwin (bun_io), x86_64-unknown-freebsd and x86_64-pc-windows-msvc; cargo clippy --no-deps for bun_io + bun_runtime on the host and on darwin, and for bun_io on freebsd; cargo fmt --check.
bun bd builds; test/internal/source-lints/ passes both as the workflow runs it (released bun) and under bun bd test; the FIFO test passes repeatedly on the debug build (about 50 ms) and bun-write.test.js as a whole has no new failures.

… EV_ERROR The Close arm of IoRequestLoop::tick_kqueue queues an EV_DELETE for the owner's stale one-shot registration and then calls on_done, which hands the owner (ReadFile/WriteFile) to the work pool before the batch reaches kevent(). The delete normally fails (ENOENT once the one-shot knote has fired, EBADF once the pool closed the fd) and comes back as an EV_ERROR entry still carrying the owner's udata, which on_update_kqueue then dispatched into an owner that was being finished or had already been freed. Cancel changes now go out with udata = 0, which on_update_kqueue already drops, so a close can never be dispatched back to its owner; CloseAction loses its tag field and ApplyAction::Readable/Writable carry the tag instead, so the type makes that structural. FileCloser::IO_TAG, whose only consumer was that field, goes with it. on_update_kqueue also tests the EV_ERROR bit instead of comparing the whole flags word: xnu ORs EV_ERROR into the submitted flags, so on macOS a rejected EV_ADD|EV_ONESHOT (0x4011) was being dispatched as a ready event. A source lint keeps `== EV_ERROR` out of the tree.

robobun · 2026-08-12T08:01:39Z

^{Updated 1:03 PM PT - Aug 12th, 2026}

❌ @robobun, your commit f386e87 has 2 failures in Build #93536 (All Failures):

/opt/winsysroot/Windows Kits/10/Include/10.0.26100/um/winver.h - clang warning on 🪟 x64 - build-bun
scripts/build/ci.ts - build failed on :freebsd: x64 - build-bun
scripts/build/ci.ts - build failed on 🍎 x64 - build-bun
scripts/build/ci.ts - build failed on 🍎 aarch64 - build-bun
scripts/build/ci.ts - build failed on 🐧 aarch64 - build-bun

🧪 To try this PR locally:

bunx bun-pr 37791

That installs a local version of the PR into your bun-37791 executable, so you can run:

bun-37791 --bun

robobun · 2026-08-12T08:01:52Z

Status: found by static analysis of IoRequestLoop::tick_kqueue / on_update_kqueue (no crash report; FreeBSD has no test lane and the Close arm is unreachable on macOS today). Kernel behaviour cited in the description was checked against FreeBSD's kqueue_kevent() / kqueue_scan(), xnu's kevent_register() / kevent_internal(), and xnu's poll() in sys_generic.c.

Fail-before proof is the source lint test/internal/source-lints/kevent-ev-error-equality.test.ts, which reports src/io/lib.rs:1623 on the unfixed tree; the FIFO test in test/js/bun/io/bun-write.test.js is runtime coverage of the registration paths on every platform (passes before and after, by design). After review the close path was reworked from an anonymous batched delete to a synchronous one (2f08550); all review threads are answered.

CI: the code under review (44ea3ec) built and passed on every lane in build 93210 except the two darwin 26 aarch64 - test-bun jobs, which expired without getting an agent; the other macOS lanes, which run the same tests, passed. The retry (build 93536, identical source) failed before compiling anything: every build-bun lane hit "Failed to download after 5 attempts" fetching c-ares, mimalloc, WebKit and lol-html from github.com. Nothing in either build implicates this diff. Not pushing further retriggers; this is ready for a maintainer to review and, if wanted, retry from Buildkite.

coderabbitai · 2026-08-12T08:02:33Z

Walkthrough

Kqueue registrations now carry pollable tags, while cancellation and close completion omit owner metadata. Kqueue error detection uses bitwise checks. FileCloser removes the unused IO_TAG contract. Tests cover source patterns and concurrent FIFO I/O.

Changes

Kqueue I/O lifecycle

Layer / File(s)	Summary
Tagged actions and cancellation `src/io/lib.rs`	Readable and writable actions carry `PollableTag` values. Cancellation submits zero user data. Close actions no longer retain tags.
Event errors and regression coverage `src/io/lib.rs`, `test/internal/source-lints/kevent-ev-error-equality.test.ts`, `test/js/bun/io/bun-write.test.js`	Kqueue reuses its event buffer and detects `EV_ERROR` with bitwise checks. Source lint and FIFO integration tests validate the behavior.
FileCloser and CloseAction contract `src/runtime/webcore/Blob.rs`, `src/runtime/webcore/blob/read_file.rs`	The `FileCloser::IO_TAG` requirement and generated CloseAction tag field are removed.

Possibly related PRs

oven-sh/bun#37787: Directly overlaps the CloseAction and FileCloser ownership and cancellation interface changes.

Suggested reviewers: jarred-sumner, alii

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the two primary kqueue fixes: unregistering before owner handoff and bit-testing EV_ERROR.
Description check	✅ Passed	The description fully explains the problem, fix, background, tests, and verification, although it uses different headings than the template.

_{Comment @coderabbitai help to get the list of available commands.}

claude

I reviewed this PR and didn't find any bugs. Because it changes cross-thread ownership handoff in the kqueue io loop — reasoning about why on_done before kevent() is safe once the cancel is anonymous, and the one-shot-knote lifecycle on FreeBSD where no CI lane runs — a human look would still be worthwhile.

What was reviewed:

apply_kqueue Cancel arm: udata = 0 matches the waker's PollableTag::Empty drop path; poll is only borrowed before on_done, and the stored kevent carries only copied scalars (fd, generation number).
EV_ERROR bit-test vs equality: matches the fix already applied in posix_event_loop.rs (#31701) and the cited xnu |= behavior.
FileCloser::IO_TAG removal: only the macro's own CloseAction { tag } and the Windows ReadFileUV impl consumed it; the inherent ReadFile::IO_TAG/WriteFile::IO_TAG used by FileAction are untouched.
Source-lint test: follows the test/internal/source-lints/ pattern with liveness and self-checks guarding against vacuous passing.

Extended reasoning...

Overview

Fixes two defects in IoRequestLoop::tick_kqueue / on_update_kqueue (src/io/lib.rs): (1) the Action::Close arm submitted an EV_DELETE with udata pointing at the owner's io_poll, then immediately called on_done which hands the owner to another thread — a rejected delete (ENOENT/EBADF, the common case since the one-shot knote is already gone) came back as an EV_ERROR entry that dispatched into a possibly-freed owner; (2) event.flags == EV_ERROR failed on macOS where xnu ORs the bit into the submitted flags. Fix: cancel entries carry udata = 0 (dropped by the existing PollableTag::Empty check), PollableTag moves into ApplyAction::Readable/Writable so Cancel structurally cannot address an owner, and EV_ERROR is bit-tested. CloseAction and the FileCloser trait/macro drop the now-dead tag/IO_TAG. A new source-lint test bans the equality pattern.

Security risks

None user-facing. The fix closes a use-after-free / cross-thread double-schedule window in an internal io loop; it does not touch input validation, auth, or any externally-reachable parsing.

Level of scrutiny

High. This is memory-safety-critical code at the intersection of kqueue semantics, one-shot knote lifecycle, and cross-thread ownership handoff (io thread → work pool → JS thread). The Close arm is unreachable on macOS today and FreeBSD has no CI lane, so correctness rests entirely on the argument in the PR description — which is thorough and, as far as I can verify against the code, sound: after apply_kqueue returns, the only reference into the owner is the &mut Poll borrow (already dropped) and the kevent entry (now udata = 0; ident/ext are copied scalars). The EV_ERROR bit-test half is the same fix already landed in posix_event_loop.rs (#31701).

Other factors

I confirmed CloseAction is only constructed in impl_file_closer! (updated), and FileCloser::IO_TAG had no other consumers — the inherent ReadFile::IO_TAG/WriteFile::IO_TAG used by the readable/writable FileAction path remain. ApplyAction still derives PartialEq/Eq/Copy (PollableTag derives them), so the action == ApplyAction::Cancel check in the debug generation-number path still compiles. The new source-lint test follows the existing directory conventions (uses globAllSources, git-tracked filter, comment stripping, liveness guard, and a self-test of the regex). Given no CI lane exercises the primary defect and the reasoning about kqueue ordering is subtle, deferring rather than approving.

… at runtime The lint stripped /* */ spans before line comments, so a "/*" inside a line comment opened a bogus span and hid whole regions (the Darwin section of src/sys/lib.rs among them). Cut comments per line only, accept any line that masks with the constant, and have the liveness check name the files that are known to contain bit tests instead of counting them. Add a bun-write test that runs a ReadFile and a WriteFile against each other on a non-blocking FIFO, so the readable and writable registration paths of the io request loop are exercised on every platform; nothing in the suite drove wait_for_writable before.

coderabbitai

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/internal/source-lints/kevent-ev-error-equality.test.ts`:
- Around line 96-103: Add "src/io/lib.rs" to the expected file list in the test
named "the scan still sees the bit tests that are known to be in the tree",
ensuring source discovery must include the changed kqueue handler while
preserving the existing assertions.
- Around line 55-63: Update classify() to detect COMPARED matches independently
when a line also contains a bit test, while excluding comparisons whose operand
is directly produced by the & EV_ERROR mask. Preserve "bit-test" classification
for valid masked checks, add the combined-flags invalid equality case to the
table test, and ensure the test invokes the production guard and asserts that
the invalid condition is rejected.

In `@test/js/bun/io/bun-write.test.js`:
- Around line 572-578: Update the concurrent reading/writing flow around
Promise.all so both I/O promises settle before the finally block closes readFd
and writeFd. Use Promise.allSettled to await both operations, preserve the
successful text and written values, and then propagate any rejection after
cleanup so the original failure remains observable.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 42a7396f-11e9-42a0-a960-4ae40a737940

📥 Commits

Reviewing files that changed from the base of the PR and between 626034f and f14df32.

📒 Files selected for processing (5)

src/io/lib.rs
src/runtime/webcore/Blob.rs
src/runtime/webcore/blob/read_file.rs
test/internal/source-lints/kevent-ev-error-equality.test.ts
test/js/bun/io/bun-write.test.js

💤 Files with no reviewable changes (1)

src/runtime/webcore/blob/read_file.rs

claude

I reviewed this PR and didn't find any bugs. Because it changes cross-thread ownership handoff in the kqueue io loop — a memory-safety path whose primary fix (the Close-arm UAF) is not exercisable on any CI lane — a human look at the correctness argument would still be worthwhile. Note also that comment-cop has 8 open flags on the new multi-line comments.

What was reviewed:

apply_kqueue Cancel arm: udata=0 decodes to PollableTag::Empty and is dropped in on_update_kqueue; poll.flags reads/writes complete before on_done hands the owner off.
ApplyAction still derives PartialEq (PollableTag is Eq), so the debug-mode action == ApplyAction::Cancel generation-number checks compile.
FileCloser::IO_TAG removal: the inherent ReadFile::IO_TAG/WriteFile::IO_TAG used by FileAction are separate and untouched; only the trait const and CloseAction.tag are gone, and impl_file_closer! is the only CloseAction constructor.
Source lint follows the existing test/internal/source-lints/ pattern (globAllSources + git ls-tree filter) and self-checks its regex against known bit-test sites.

Extended reasoning...

Overview

Two fixes to the kqueue arm of IoRequestLoop in src/io/lib.rs: (1) ApplyAction::Cancel now submits with udata=0 so an EV_ERROR reply to a stale EV_DELETE cannot dispatch into an owner another thread may already have freed, structurally enforced by moving the PollableTag into ApplyAction::Readable(tag)/Writable(tag) and removing CloseAction.tag + FileCloser::IO_TAG; (2) on_update_kqueue bit-tests EV_ERROR instead of comparing the whole flags word, matching posix_event_loop.rs (#31701). Ripple edits in Blob.rs (macro + trait) and read_file.rs (Windows ReadFileUV impl). New source lint in test/internal/source-lints/ and a FIFO wait test in bun-write.test.js.

Security risks

None identified. No user-controlled input reaches the changed code paths; this is internal io-thread ↔ work-pool coordination.

Level of scrutiny

High. This is native cross-thread memory safety in the kqueue event loop. The Close-arm defect is a use-after-free / double-schedule race whose fix rests on an argument about kernel semantics (one-shot knote lifetime, EV_DELETE applied before the same kevent() scans for events, xnu OR-ing vs FreeBSD replacing flags) and thread ordering (on_done runs before kevent() submits the batch). The argument in the PR description is careful and I did not find a hole in it, but it is exactly the class of reasoning a maintainer familiar with this subsystem should confirm — especially since the fix is compiled only for macOS/FreeBSD, is unreachable on macOS today, and there is no FreeBSD CI lane, so the compiler is the only check.

Other factors

The EV_ERROR bit-test half is straightforward, precedented in-tree, and covered by the new source lint (verified to fail-before on the unfixed line).
I confirmed CloseAction is only constructed by impl_file_closer! and that the inherent IO_TAG constants used by FileAction in read_file.rs/write_file.rs are separate from the removed trait-level const.
ApplyAction retains #[derive(PartialEq, Eq)] and PollableTag is Eq, so the debug-only action == ApplyAction::Cancel generation-number gates on macOS still compile.
comment-cop (github-actions) left 8 unaddressed flags on the new multi-line comments in lib.rs/Blob.rs. The comments document kernel behaviour the fix depends on rather than justify a workaround, but the flags are outstanding and the author may want to trim or dismiss them.

… arm Instead of queueing the delete into the batch that the next kevent() call submits after on_done has already handed the owner away, apply it on the spot with nevents = 0, so nothing about the close is in flight once the owner belongs to another thread. The comments that argued the queued variant was safe go away with it. The lint now flags a whole-word compare even on a line that also bit-tests, and requires src/io/lib.rs among the files it sees bit tests in.

claude

I reviewed this PR and didn't find any bugs. Because it changes cross-thread ownership handoff in the kqueue io loop (a memory-safety fix whose correctness rests on xnu/FreeBSD kernel semantics that CI cannot exercise — the Close arm is unreachable on macOS and there is no FreeBSD lane), a human look would still be worthwhile.

What was reviewed:

unregister_kqueue applies EV_DELETE synchronously (nevents=0) before on_done, mirroring the epoll arm's EPOLL_CTL_DEL; verified nothing about the owner remains pending in the batch after handoff.
ApplyAction::Cancel now writes udata=0, and on_update_kqueue early-returns on tag == Empty, so a stray cancel reply cannot dispatch.
FileCloser::IO_TAG / CloseAction.tag removal traced to all consumers (macro, Windows ReadFileUV) — no remaining reader.
The EV_ERROR bit-test matches the sibling fix in posix_event_loop.rs (#31701); the new source lint's liveness check names src/io/lib.rs.

Extended reasoning...

Overview

This PR fixes two defects in the kqueue arm of IoRequestLoop (src/io/lib.rs), the epoll/kqueue loop behind Bun.file().text() / Bun.write() on pollable fds:

The Action::Close arm previously appended an EV_DELETE (with udata pointing into the owner's io_poll) to the tick's batched changelist and immediately called on_done, which hands the owner to the work pool and eventually frees it. A rejected delete (ENOENT after the one-shot fired, EBADF after the pool closed the fd) would come back in the eventlist with the owner's udata and be dispatched into a possibly-freed object. The fix moves the delete out of the batch into a synchronous kevent(nevents=0) in a new Poll::unregister_kqueue, mirroring the epoll arm's synchronous EPOLL_CTL_DEL.
on_update_kqueue compared event.flags == EV_ERROR, which is wrong on xnu (which ORs the bit in). Now bit-tests (flags & EV_ERROR) != 0, matching posix_event_loop.rs (#31701).

Follow-on cleanup: CloseAction loses its tag field, the tag moves into ApplyAction::Readable(tag)/Writable(tag), Cancel writes udata=0, and FileCloser::IO_TAG is deleted from the trait, impl_file_closer!, and the Windows ReadFileUV impl. A new source-lint test bans whole-word EV_ERROR comparisons, and a new FIFO test in bun-write.test.js exercises the readable/writable registration paths at runtime.

Security risks

None. This is internal I/O loop plumbing with no user-controlled input reaching the changed code paths beyond fds already validated upstream. The fix closes a potential use-after-free race, which is a hardening improvement.

Level of scrutiny

High. This is native memory-safety code at the intersection of REVIEW.md's most-blocked categories: cross-thread ownership handoff (io thread → work pool → JS thread), pointers into objects another thread may free, and platform-specific kernel behavior (xnu vs FreeBSD kevent semantics for rejected changes, nevents=0 return-immediately behavior). The PR description cites specific kernel source lines for each claim, and the fix structurally mirrors the epoll arm rather than relying on an argument about what the kernel can hand back — but the defect it fixes is by the author's own account not reachable on any CI lane (macOS clears close_after_io before the Close arm; there is no FreeBSD test lane), so the fix's correctness cannot be verified by CI passing.

Other factors

All comment-cop and CodeRabbit threads are resolved; the earlier revision's paragraph-long safety comments were replaced by restructuring the code (2f08550), and the remaining two-line comments document kernel facts rather than justify workarounds.
The IO_TAG removal is a clean dead-code deletion with all consumers updated in the same PR.
The new FIFO test is runtime coverage of the restructured registration path (passes before and after by design); the source lint is the fail-before/pass-after discriminator.
I checked that ApplyAction::Cancel's udata=0 composes correctly with on_update_kqueue's existing tag == Empty early return — even if a cancel reply somehow surfaced, it would be a no-op.
The removed two-line comment at the batched kevent() call site ("we set 0 here so that if we get an error on registration, it becomes errno") was stale/wrong (nevents is capacity, not 0), so its deletion is correct.

Given the subtlety of the cross-thread ownership reasoning and the inability of CI to exercise the actual defect path, a maintainer familiar with IoRequestLoop should confirm the kernel-behavior claims and the thread-ordering argument.

claude

I reviewed this PR and didn't find any bugs. Because it restructures the kqueue Close arm's ownership hand-off (a cross-thread lifetime concern in native io-loop code, with the primary defect only reachable on FreeBSD where there is no test lane), a human look would still be worthwhile.

What was reviewed:

unregister_kqueue applies EV_DELETE synchronously with nevents=0 before on_done, mirroring the epoll arm's EPOLL_CTL_DEL; discarded result matches epoll's.
ApplyAction::Cancel now writes udata=0, so a rejected delete cannot dispatch into an owner; on_update_kqueue's tag == Empty early return covers it.
FileCloser::IO_TAG removal — verified the remaining IO_TAG uses are inherent consts feeding FileAction.tag, not the trait constant; CloseAction has no other constructors.
The source-lint's classify erases masked compares before checking whole-word compares, and the liveness assertion names src/io/lib.rs.

Extended reasoning...

Overview

This PR fixes two defects in the kqueue side of IoRequestLoop (src/io/lib.rs), the request loop behind Bun.file(...).text() / Bun.write() on pollable fds:

The Action::Close arm previously batched the owner's EV_DELETE into the next kevent() and immediately called on_done, which hands the owner to the work pool. The pool could free the owner before the io thread submitted the delete; a rejected delete (ENOENT/EBADF, since the one-shot knote is already gone) would then be returned with udata pointing into the freed owner and dispatched. The fix applies the EV_DELETE synchronously in a new Poll::unregister_kqueue (nevents=0, result discarded) before on_done, mirroring the epoll arm.
on_update_kqueue compared event.flags == EV_ERROR instead of bit-testing; xnu ORs EV_ERROR into the submitted flags, so a rejected registration would misroute to on_ready. Changed to (flags & EV_ERROR) != 0.

Supporting changes: ApplyAction::Readable/Writable now carry the PollableTag; Cancel writes udata=0; CloseAction.tag and FileCloser::IO_TAG (trait const + macro + Windows ReadFileUV impl) are removed as dead. A new source-lint bans whole-word EV_ERROR compares, and a new FIFO test in bun-write.test.js exercises the wait_for_readable/wait_for_writable registration paths on every platform.

Security risks

None identified. This is internal io-loop plumbing with no user-controlled input reaching the changed code paths; the fix narrows a latent UAF window rather than adding any new parsing or trust boundary.

Level of scrutiny

High. This is native Rust in a concurrency-sensitive path: the io thread hands an owner to the work pool, and the correctness of the fix rests on ordering the synchronous kevent() delete before that hand-off so nothing about the close remains pending when another thread may free the owner. The primary defect is only reachable on FreeBSD (macOS clears close_after_io before the Close arm), and FreeBSD has no CI test lane, so the fix cannot be exercised end-to-end by a fail-before test anywhere CI runs. The PR description is thorough and cites the kernel sources for both the EV_ERROR OR-vs-assign difference and the nevents=0 apply-only semantics, and the change went through a review iteration (the first revision batched an anonymous delete; this revision applies it synchronously per comment-cop feedback that the safety argument was too load-bearing).

Other factors

All review threads (comment-cop's paragraph-comment flags, CodeRabbit's three findings) are resolved; the author addressed the two lint-test suggestions in 2f08550/f14df32 and gave a reasoned rebuttal to the Promise.allSettled suggestion that CodeRabbit accepted.
I checked that the removed FileCloser::IO_TAG trait constant has no remaining consumers — the surviving IO_TAG references in read_file.rs/write_file.rs are inherent pub(crate) consts used for FileAction.tag in the Readable/Writable arms, unrelated to the removed trait item.
The ApplyAction::Cancel path still hits unreachable!() if neither PollReadable nor PollWritable is set, but the only caller (unregister_kqueue, via the Close arm) is guarded by exactly that condition in tick_kqueue.
CI build #93536 is in progress per the robobun status comment; the author's verification section covers cargo check/clippy across darwin/freebsd/windows targets and local test runs.

Given the cross-thread lifetime reasoning, the platform-gated code with no fail-before coverage on the platform where the defect is reachable, and the repo guidance on native memory-safety changes, this warrants a human reviewer's sign-off rather than auto-approval.

github-actions Bot added the claude label Aug 12, 2026

claude Bot reviewed Aug 12, 2026

View reviewed changes