From 978dc5e23d0f02887db397e16a94ad1506e93a6e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 07:39:29 +0000 Subject: [PATCH 1/5] io(kqueue): submit close cancels with no dispatch target and bit-test 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. --- src/io/lib.rs | 71 +++++++---- src/runtime/webcore/Blob.rs | 13 +- src/runtime/webcore/blob/read_file.rs | 1 - .../kevent-ev-error-equality.test.ts | 116 ++++++++++++++++++ 4 files changed, 166 insertions(+), 35 deletions(-) create mode 100644 test/internal/source-lints/kevent-ev-error-equality.test.ts diff --git a/src/io/lib.rs b/src/io/lib.rs index 8a9776c9a637..d302e787bb40 100644 --- a/src/io/lib.rs +++ b/src/io/lib.rs @@ -1117,8 +1117,7 @@ impl IoRequestLoop { match (request.callback)(request) { Action::Readable(readable) => { Poll::apply_kqueue( - ApplyAction::Readable, - readable.tag, + ApplyAction::Readable(readable.tag), readable.poll, readable.fd, add_one(&mut events_list), @@ -1126,8 +1125,7 @@ impl IoRequestLoop { } Action::Writable(writable) => { Poll::apply_kqueue( - ApplyAction::Writable, - writable.tag, + ApplyAction::Writable(writable.tag), writable.poll, writable.fd, add_one(&mut events_list), @@ -1139,12 +1137,15 @@ impl IoRequestLoop { { Poll::apply_kqueue( ApplyAction::Cancel, - close.tag, close.poll, close.fd, add_one(&mut events_list), ); } + // From here on another thread may finish and free + // the owner, before the kevent() below has even + // submitted the cancel; safe only because the cancel + // addresses nobody (see `ApplyAction::Cancel`). (close.on_done)(close.ctx); } } @@ -1158,10 +1159,10 @@ impl IoRequestLoop { self.pollfd().native(), events_list.as_ptr(), c_int::try_from(change_count).expect("int cast"), - // The same array may be used for the changelist and eventlist. + // The same array may be used for the changelist and eventlist; + // a rejected change comes back through it as an EV_ERROR entry + // (see `on_update_kqueue`) instead of failing the call. events_list.as_mut_ptr(), - // we set 0 here so that if we get an error on - // registration, it becomes errno c_int::try_from(capacity).expect("int cast"), core::ptr::null(), ); @@ -1353,11 +1354,12 @@ pub struct FileAction<'a> { pub on_error: fn(*mut (), &sys::Error), } +/// No [`PollableTag`], unlike [`FileAction`]: `on_done` hands the owner to +/// another thread, so nothing a close submits may dispatch back into it. pub struct CloseAction<'a> { pub fd: Fd, pub poll: &'a mut Poll, pub ctx: *mut (), - pub tag: PollableTag, pub on_done: fn(*mut ()), } @@ -1496,8 +1498,15 @@ pub type FlagsSet = enumset::EnumSet; #[cfg(any(target_os = "macos", target_os = "freebsd"))] #[derive(PartialEq, Eq, Clone, Copy)] enum ApplyAction { - Readable, - Writable, + /// One-shot `EV_ADD`; its event, or the `EV_ERROR` entry if the add is + /// rejected, dispatches to the owner the tag names. + Readable(PollableTag), + Writable(PollableTag), + /// `EV_DELETE` of whatever the poll still has armed, submitted with + /// `udata = 0` so that `on_update_kqueue` drops its reply like the waker's. + /// The reply is the rule, not the exception (the one-shot knote is usually + /// gone already: ENOENT; or the owner closed the fd first: EBADF), and by + /// the time it arrives `tick_kqueue` has handed the owner to another thread. Cancel, } @@ -1506,7 +1515,6 @@ impl Poll { #[inline] pub(crate) fn apply_kqueue( action: ApplyAction, - tag: PollableTag, poll: &mut Poll, fd: Fd, kqueue_event: &mut KEvent, @@ -1514,23 +1522,31 @@ impl Poll { log!( "register({}, {})", match action { - ApplyAction::Readable => "readable", - ApplyAction::Writable => "writable", + ApplyAction::Readable(_) => "readable", + ApplyAction::Writable(_) => "writable", ApplyAction::Cancel => "cancel", }, fd ); let one_shot_flag = libc::EV_ONESHOT; - let udata: usize = Pollable::init(tag, std::ptr::from_mut::(poll)).ptr() as usize; - let (filter, flags_): (i16, u16) = match action { - ApplyAction::Readable => (libc::EVFILT_READ, libc::EV_ADD | one_shot_flag), - ApplyAction::Writable => (libc::EVFILT_WRITE, libc::EV_ADD | one_shot_flag), + let poll_ptr = std::ptr::from_mut::(poll); + let (filter, flags_, udata): (i16, u16, usize) = match action { + ApplyAction::Readable(tag) => ( + libc::EVFILT_READ, + libc::EV_ADD | one_shot_flag, + Pollable::init(tag, poll_ptr).ptr() as usize, + ), + ApplyAction::Writable(tag) => ( + libc::EVFILT_WRITE, + libc::EV_ADD | one_shot_flag, + Pollable::init(tag, poll_ptr).ptr() as usize, + ), ApplyAction::Cancel => { if poll.flags.contains(Flags::PollReadable) { - (libc::EVFILT_READ, libc::EV_DELETE) + (libc::EVFILT_READ, libc::EV_DELETE, 0) } else if poll.flags.contains(Flags::PollWritable) { - (libc::EVFILT_WRITE, libc::EV_DELETE) + (libc::EVFILT_WRITE, libc::EV_DELETE, 0) } else { unreachable!() } @@ -1561,10 +1577,10 @@ impl Poll { } match action { - ApplyAction::Readable => { + ApplyAction::Readable(_) => { poll.flags.insert(Flags::PollReadable); } - ApplyAction::Writable => { + ApplyAction::Writable(_) => { poll.flags.insert(Flags::PollWritable); } ApplyAction::Cancel => { @@ -1611,8 +1627,9 @@ impl Poll { let pollable = Pollable::from(event.udata as u64); let tag = pollable.tag(); - // The waker is registered with udata=0 → tag=.empty. The wakeup exists - // only to unblock kevent() so the pending queue drains. + // udata=0 → tag=.empty is both the waker (registered only to unblock + // kevent() so the pending queue drains) and the EV_ERROR reply to an + // `ApplyAction::Cancel`, whose owner may already be gone. if tag == PollableTag::Empty { return; } @@ -1620,7 +1637,11 @@ impl Poll { // CYCLEBREAK: owner (ReadFile/WriteFile) is T6; dispatch via link-time // `extern "Rust"` defined in `bun_runtime::dispatch`. The // container_of(io_poll) recovery happens there. - if event.flags == libc::EV_ERROR { + // + // A rejected change comes back with EV_ERROR set in `flags`; xnu ORs it + // into the bits we submitted (EV_ADD|EV_ONESHOT|EV_ERROR) while FreeBSD + // replaces them, so test the bit rather than the whole word. + if (event.flags & libc::EV_ERROR) != 0 { log!("error({}) = {}", event.ident, event.data); // SAFETY: poll is the `io_poll` field of a live owner; link-time // extern body matches on `tag`. diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 006cf841e312..4834e2169ca4 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -7031,8 +7031,6 @@ pub trait FileOpener: Sized { // TODO: move to bun_sys? pub trait FileCloser: Sized { - const IO_TAG: bun_io::Tag; - fn opened_fd(&self) -> Fd; fn set_opened_fd(&mut self, fd: Fd); fn close_after_io(&self) -> bool; @@ -7109,15 +7107,13 @@ pub trait FileCloser: Sized { } /// Implements [`FileCloser`] for a task struct with the standard field set -/// (`opened_fd`, `close_after_io`, `state`, `io_request`, `io_poll`, `task`), -/// an inherent `update()`, and a [`bun_io::Tag`] variant named after the type. -/// The type must also carry `bun_threading::intrusive_work_task!` and -/// `bun_io::intrusive_io_request!`, which provide the parent-pointer recovery -/// used by the two trampolines. +/// (`opened_fd`, `close_after_io`, `state`, `io_request`, `io_poll`, `task`) +/// and an inherent `update()`. The type must also carry +/// `bun_threading::intrusive_work_task!` and `bun_io::intrusive_io_request!`, +/// which provide the parent-pointer recovery used by the two trampolines. macro_rules! impl_file_closer { ($T:ident) => { impl crate::webcore::blob::FileCloser for $T { - const IO_TAG: ::bun_io::Tag = ::bun_io::Tag::$T; fn opened_fd(&self) -> ::bun_sys::Fd { self.opened_fd } @@ -7164,7 +7160,6 @@ macro_rules! impl_file_closer { fd, poll, ctx: this.cast::<()>(), - tag: ::IO_TAG, on_done, }) } diff --git a/src/runtime/webcore/blob/read_file.rs b/src/runtime/webcore/blob/read_file.rs index 8bc36a6b5805..18c0bcafa0d5 100644 --- a/src/runtime/webcore/blob/read_file.rs +++ b/src/runtime/webcore/blob/read_file.rs @@ -960,7 +960,6 @@ impl<'a> FileOpener for ReadFileUV<'a> { #[cfg(windows)] impl<'a> FileCloser for ReadFileUV<'a> { - const IO_TAG: bun_io::Tag = bun_io::Tag::ReadFile; fn opened_fd(&self) -> Fd { self.opened_fd } diff --git a/test/internal/source-lints/kevent-ev-error-equality.test.ts b/test/internal/source-lints/kevent-ev-error-equality.test.ts new file mode 100644 index 000000000000..7f0bf79018fe --- /dev/null +++ b/test/internal/source-lints/kevent-ev-error-equality.test.ts @@ -0,0 +1,116 @@ +import { file } from "bun"; +import { expect, test } from "bun:test"; +import { realpathSync } from "fs"; +import path from "path"; +import { globAllSources } from "../../../scripts/glob-sources.ts"; + +// A kevent changelist entry the kernel rejects is handed back in the eventlist +// with EV_ERROR set in `flags` and the errno in `data`. How it is "set" differs +// between the two kqueue kernels we build for: +// +// xnu (bsd/kern/kern_event.c, kevent_register) kev->flags |= EV_ERROR; +// FreeBSD (sys/kern/kern_event.c, kqueue_kevent) kevp->flags = EV_ERROR; +// +// so on macOS the reply to a failed `EV_ADD|EV_ONESHOT` reads 0x4011, not +// 0x4000, and `flags == EV_ERROR` silently classifies it as a ready event. +// For the io request loop that meant a rejected registration was dispatched +// as "readable", which re-reads, gets EAGAIN, re-registers, and spins; for +// FilePoll (posix_event_loop.rs) it meant change errors were swallowed. Both +// kernels agree on the bit, so that is what gets tested: +// +// flags == EV_ERROR / flags != EV::ERROR → (flags & EV_ERROR) != 0 +// +// The Zig original carried the equality in both kqueue users; the port fixed +// FilePoll's and this lint keeps the other from coming back. + +const root = path.resolve(import.meta.dir, "..", "..", ".."); +const rustSources = globAllSources().rust.filter(abs => abs.endsWith(".rs")); + +// Only scan files tracked in HEAD (a `git stash` round-trip can leave stray +// `.rs` files in the working tree; CI runs on a clean checkout). +const tracked: Set | null = (() => { + const r = Bun.spawnSync({ + cmd: ["git", "-C", root, "ls-tree", "-r", "--name-only", "-z", "HEAD"], + stdout: "pipe", + stderr: "ignore", + }); + if (!r.success) return null; + return new Set(r.stdout.toString().split("\0").filter(Boolean)); +})(); + +// The constant in any of its spellings: `EV_ERROR`, `libc::EV_ERROR`, +// `bun_sys::darwin::EV::ERROR`, ... +const CONSTANT = String.raw`(?:\w+::)*EV(?:_ERROR|::ERROR)\b`; +// `x & EV_ERROR` is the bit test this lint asks for. It is erased before +// looking for comparisons so that `(flags & EV_ERROR) == 0`, and Rust's +// `flags & EV_ERROR == 0` (`&` binds tighter than `==` in Rust), do not +// register as comparing the whole word. `(? m.replace(/[^\n]/g, "")).replace(/\/\/.*$/gm, ""); + for (const [index, line] of stripped.split("\n").entries()) { + BIT_TEST.lastIndex = 0; + if (BIT_TEST.test(line)) bitTests++; + if (comparesWholeWord(line)) offenders.push(`${source}:${index + 1}: ${line.trim()}`); + } +} + +test("scans a non-empty set of tracked Rust sources", () => { + // Guards against the tracked/realpath filters above over-firing and leaving + // nothing to scan, which would make the ban below pass vacuously. + expect(scanned).toBeGreaterThan(0); +}); + +test("the scan still sees the tree's EV_ERROR bit tests", () => { + // FilePoll's register/unregister (src/io/posix_event_loop.rs) and the + // process reaper (src/spawn/process.rs) test the bit; if none are seen, the + // file set or the comment stripping is broken and the ban below is vacuous. + expect(bitTests).toBeGreaterThan(0); +}); + +test("the pattern recognizes the spellings it claims to", () => { + const banned = [ + "if event.flags == libc::EV_ERROR {", + "if changelist[0].flags == EV::ERROR {", + "if ev.flags != bun_sys::darwin::EV::ERROR {", + "if libc::EV_ERROR == event.flags {", + "let failed = event.flags == (EV::ERROR);", + "if event.flags == EV_ERROR && event.data != 0 {", + ]; + const allowed = [ + "if (event.flags & libc::EV_ERROR) != 0 {", + "if (changelist[0].flags & EV::ERROR) != 0 && changelist[0].data != 0 {", + "if (changelist[i].flags & EV::ERROR) == 0 || changelist[i].data == 0 {", + "if r.flags & libc::EV_ERROR == 0 || r.data == 0 {", + "if event.data != 0 && event.flags & EV_ERROR != 0 {", + "pub const ERROR: u16 = libc::EV_ERROR;", + "let is_error = event.flags & EV::ERROR != 0;", + ]; + expect(banned.filter(s => !comparesWholeWord(s))).toEqual([]); + expect(allowed.filter(comparesWholeWord)).toEqual([]); +}); + +test("kevent EV_ERROR is tested as a bit, never compared against the whole flags word", () => { + expect(offenders).toEqual([]); +}); From f14df321ec6064fe3ca7ce9b494398e2bc849230 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 12:55:42 +0000 Subject: [PATCH 2/5] test: make the EV_ERROR lint line-local and cover the pipe wait paths 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. --- .../kevent-ev-error-equality.test.ts | 82 +++++++++++-------- test/js/bun/io/bun-write.test.js | 41 ++++++++++ 2 files changed, 90 insertions(+), 33 deletions(-) diff --git a/test/internal/source-lints/kevent-ev-error-equality.test.ts b/test/internal/source-lints/kevent-ev-error-equality.test.ts index 7f0bf79018fe..325b923b4c49 100644 --- a/test/internal/source-lints/kevent-ev-error-equality.test.ts +++ b/test/internal/source-lints/kevent-ev-error-equality.test.ts @@ -11,17 +11,18 @@ import { globAllSources } from "../../../scripts/glob-sources.ts"; // xnu (bsd/kern/kern_event.c, kevent_register) kev->flags |= EV_ERROR; // FreeBSD (sys/kern/kern_event.c, kqueue_kevent) kevp->flags = EV_ERROR; // -// so on macOS the reply to a failed `EV_ADD|EV_ONESHOT` reads 0x4011, not -// 0x4000, and `flags == EV_ERROR` silently classifies it as a ready event. -// For the io request loop that meant a rejected registration was dispatched -// as "readable", which re-reads, gets EAGAIN, re-registers, and spins; for -// FilePoll (posix_event_loop.rs) it meant change errors were swallowed. Both -// kernels agree on the bit, so that is what gets tested: +// so on macOS the reply to a rejected `EV_ADD|EV_ONESHOT` reads 0x4011, not +// 0x4000, and `flags == EV_ERROR` is false for it: the io request loop +// (src/io/lib.rs) dispatched such a reply as a ready event, and FilePoll +// (src/io/posix_event_loop.rs) used to swallow its change errors the same way +// (#31701). Both kernels agree on the bit, so that is what gets tested: // -// flags == EV_ERROR / flags != EV::ERROR → (flags & EV_ERROR) != 0 +// flags == EV_ERROR / flags != EV::ERROR -> (flags & EV_ERROR) != 0 // -// The Zig original carried the equality in both kqueue users; the port fixed -// FilePoll's and this lint keeps the other from coming back. +// Both of the io layer's kqueue users inherited the equality from the Zig +// original; this keeps it from coming back in either. Scope is the Rust tree +// because that is what .github/workflows/source-lints.yml runs this for; the C +// kqueue code in packages/bun-usockets is outside its triggers. const root = path.resolve(import.meta.dir, "..", "..", ".."); const rustSources = globAllSources().rust.filter(abs => abs.endsWith(".rs")); @@ -41,21 +42,30 @@ const tracked: Set | null = (() => { // The constant in any of its spellings: `EV_ERROR`, `libc::EV_ERROR`, // `bun_sys::darwin::EV::ERROR`, ... const CONSTANT = String.raw`(?:\w+::)*EV(?:_ERROR|::ERROR)\b`; -// `x & EV_ERROR` is the bit test this lint asks for. It is erased before -// looking for comparisons so that `(flags & EV_ERROR) == 0`, and Rust's -// `flags & EV_ERROR == 0` (`&` binds tighter than `==` in Rust), do not -// register as comparing the whole word. `(?(); let scanned = 0; -let bitTests = 0; for (const abs of rustSources) { const source = path.relative(root, abs).replaceAll(path.sep, "/"); // `src/cli` is a symlink into `src/runtime/cli`; count each file once under @@ -65,14 +75,15 @@ for (const abs of rustSources) { scanned++; const content = await file(abs).text(); if (!content.includes("EV_ERROR") && !content.includes("EV::ERROR")) continue; - // Drop comments (the ones in posix_event_loop.rs and io/lib.rs describe this - // very hazard) without disturbing line numbers: block comments keep their - // newlines, line comments are cut at the `//`. - const stripped = content.replace(/\/\*[\s\S]*?\*\//g, m => m.replace(/[^\n]/g, "")).replace(/\/\/.*$/gm, ""); - for (const [index, line] of stripped.split("\n").entries()) { - BIT_TEST.lastIndex = 0; - if (BIT_TEST.test(line)) bitTests++; - if (comparesWholeWord(line)) offenders.push(`${source}:${index + 1}: ${line.trim()}`); + for (const [index, line] of content.split("\n").entries()) { + switch (classify(line)) { + case "bit-test": + filesWithBitTests.add(source); + break; + case "compared": + offenders.push(`${source}:${index + 1}: ${line.trim()}`); + break; + } } } @@ -82,11 +93,13 @@ test("scans a non-empty set of tracked Rust sources", () => { expect(scanned).toBeGreaterThan(0); }); -test("the scan still sees the tree's EV_ERROR bit tests", () => { - // FilePoll's register/unregister (src/io/posix_event_loop.rs) and the - // process reaper (src/spawn/process.rs) test the bit; if none are seen, the - // file set or the comment stripping is broken and the ban below is vacuous. - expect(bitTests).toBeGreaterThan(0); +test("the scan still sees the bit tests that are known to be in the tree", () => { + // Named files rather than a count: if the file set or the comment handling + // ever stops seeing one of these, the ban below would be vacuous for it. + // FilePoll's register/unregister and the process reaper's kevent loop. + expect([...filesWithBitTests].sort()).toEqual( + expect.arrayContaining(["src/io/posix_event_loop.rs", "src/spawn/process.rs"]), + ); }); test("the pattern recognizes the spellings it claims to", () => { @@ -102,13 +115,16 @@ test("the pattern recognizes the spellings it claims to", () => { "if (event.flags & libc::EV_ERROR) != 0 {", "if (changelist[0].flags & EV::ERROR) != 0 && changelist[0].data != 0 {", "if (changelist[i].flags & EV::ERROR) == 0 || changelist[i].data == 0 {", + "if (event.flags & EV_ERROR) == EV_ERROR {", "if r.flags & libc::EV_ERROR == 0 || r.data == 0 {", "if event.data != 0 && event.flags & EV_ERROR != 0 {", - "pub const ERROR: u16 = libc::EV_ERROR;", "let is_error = event.flags & EV::ERROR != 0;", + "pub const ERROR: u16 = libc::EV_ERROR;", + " // xnu ORs EV_ERROR in, so `flags == EV_ERROR` is the bug this comment is about", + "let rejected = (kev.flags & EV::ERROR) != 0; // not kev.flags == EV::ERROR", ]; - expect(banned.filter(s => !comparesWholeWord(s))).toEqual([]); - expect(allowed.filter(comparesWholeWord)).toEqual([]); + expect(banned.filter(s => classify(s) !== "compared")).toEqual([]); + expect(allowed.filter(s => classify(s) === "compared")).toEqual([]); }); test("kevent EV_ERROR is tested as a bit, never compared against the whole flags word", () => { diff --git a/test/js/bun/io/bun-write.test.js b/test/js/bun/io/bun-write.test.js index f02bdf4f8e17..193a27269390 100644 --- a/test/js/bun/io/bun-write.test.js +++ b/test/js/bun/io/bun-write.test.js @@ -11,6 +11,7 @@ import { tempDir, withoutAggressiveGC, } from "harness"; +import { mkfifo } from "mkfifo"; import path, { join } from "path"; let i = 0; @@ -537,6 +538,46 @@ const IS_UV_FS_COPYFILE_DISABLED = expect(exitCode).toBe(0); }); + // Bun.write(Bun.file(fd), bytes) and Bun.file(fd).text() on a pipe run on + // the thread pool and, whenever poll() says the pipe is not ready, park on + // the io thread (src/io/lib.rs IoRequestLoop: epoll on Linux, kqueue on + // macOS) until it is. The reader below starts on an empty FIFO and the + // writer has several times more than any platform's pipe buffer holds, so + // both of them take that detour, repeatedly, before this resolves. + it.skipIf(isWindows)("Bun.write and Bun.file(fd).text() on a non-blocking FIFO wait for each other", async () => { + using dir = tempDir("bun-write-fifo", {}); + const fifo = join(String(dir), "data.fifo"); + mkfifo(fifo); + // O_RDWR: opening a FIFO for both directions never blocks waiting for a + // peer, and keeps a writer attached so the reader cannot see EOF early. + // O_NONBLOCK: a full pipe reports EAGAIN to the writer instead of blocking + // a pool thread. Two separate fds because epoll registers an fd only once, + // and the reader and the writer each register their own. + const flags = fs.constants.O_RDWR | fs.constants.O_NONBLOCK; + const readFd = fs.openSync(fifo, flags); + const writeFd = fs.openSync(fifo, flags); + try { + // 256 KiB is also the size from which Bun.write() always goes to the + // thread pool instead of first trying a synchronous write. + const payload = Buffer.alloc(256 * 1024, "0123456789abcdef\n").toString(); + + const reading = Bun.file(readFd).slice(0, payload.length).text(); + + const destination = Bun.file(writeFd); + // Bun.write() only waits for an fd destination to become writable once + // the fd's type has been resolved; reading .size does that up front. + destination.size; + const writing = Bun.write(destination, payload); + + const [text, written] = await Promise.all([reading, writing]); + expect({ written, length: text.length }).toEqual({ written: payload.length, length: payload.length }); + expect(text).toBe(payload); + } finally { + fs.closeSync(writeFd); + fs.closeSync(readFd); + } + }); + it("Bun.file(0) survives GC", async () => { for (let i = 0; i < 10; i++) { let f = Bun.file(0); From 2f0855031d185433262f68f9a8d74c027e30c971 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 13:22:14 +0000 Subject: [PATCH 3/5] io(kqueue): apply the close's EV_DELETE synchronously, like the epoll 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. --- src/io/lib.rs | 55 +++++++++-------- .../kevent-ev-error-equality.test.ts | 60 +++++++++++-------- 2 files changed, 63 insertions(+), 52 deletions(-) diff --git a/src/io/lib.rs b/src/io/lib.rs index d302e787bb40..b2223980db4d 100644 --- a/src/io/lib.rs +++ b/src/io/lib.rs @@ -1135,17 +1135,8 @@ impl IoRequestLoop { if close.poll.flags.contains(Flags::PollReadable) || close.poll.flags.contains(Flags::PollWritable) { - Poll::apply_kqueue( - ApplyAction::Cancel, - close.poll, - close.fd, - add_one(&mut events_list), - ); + close.poll.unregister_kqueue(self.pollfd(), close.fd); } - // From here on another thread may finish and free - // the owner, before the kevent() below has even - // submitted the cancel; safe only because the cancel - // addresses nobody (see `ApplyAction::Cancel`). (close.on_done)(close.ctx); } } @@ -1159,9 +1150,7 @@ impl IoRequestLoop { self.pollfd().native(), events_list.as_ptr(), c_int::try_from(change_count).expect("int cast"), - // The same array may be used for the changelist and eventlist; - // a rejected change comes back through it as an EV_ERROR entry - // (see `on_update_kqueue`) instead of failing the call. + // The same array may be used for the changelist and eventlist. events_list.as_mut_ptr(), c_int::try_from(capacity).expect("int cast"), core::ptr::null(), @@ -1354,8 +1343,6 @@ pub struct FileAction<'a> { pub on_error: fn(*mut (), &sys::Error), } -/// No [`PollableTag`], unlike [`FileAction`]: `on_done` hands the owner to -/// another thread, so nothing a close submits may dispatch back into it. pub struct CloseAction<'a> { pub fd: Fd, pub poll: &'a mut Poll, @@ -1498,15 +1485,10 @@ pub type FlagsSet = enumset::EnumSet; #[cfg(any(target_os = "macos", target_os = "freebsd"))] #[derive(PartialEq, Eq, Clone, Copy)] enum ApplyAction { - /// One-shot `EV_ADD`; its event, or the `EV_ERROR` entry if the add is - /// rejected, dispatches to the owner the tag names. + /// One-shot `EV_ADD` whose events dispatch to the owner the tag names. Readable(PollableTag), Writable(PollableTag), - /// `EV_DELETE` of whatever the poll still has armed, submitted with - /// `udata = 0` so that `on_update_kqueue` drops its reply like the waker's. - /// The reply is the rule, not the exception (the one-shot knote is usually - /// gone already: ENOENT; or the owner closed the fd first: EBADF), and by - /// the time it arrives `tick_kqueue` has handed the owner to another thread. + /// `EV_DELETE` of whatever the poll still has armed; dispatches to nobody. Cancel, } @@ -1618,6 +1600,25 @@ impl Poll { self.flags.remove(Flags::Registered); } + /// Applied on the spot rather than with the next batch: the caller hands + /// the owner to another thread as soon as this returns. + #[cfg(any(target_os = "macos", target_os = "freebsd"))] + pub(crate) fn unregister_kqueue(&mut self, watcher_fd: Fd, fd: Fd) { + let mut change: KEvent = bun_core::ffi::zeroed(); + Poll::apply_kqueue(ApplyAction::Cancel, self, fd, &mut change); + // With nevents = 0 the call returns as soon as the change is applied. + // It fails with ENOENT once the one-shot registration has fired and + // with EBADF once the fd is closed; either way nothing is left to remove. + let _ = kevent_call( + watcher_fd.native(), + &raw const change, + 1, + core::ptr::null_mut(), + 0, + core::ptr::null(), + ); + } + #[cfg(any(target_os = "macos", target_os = "freebsd"))] pub(crate) fn on_update_kqueue(event: KEvent) { #[cfg(target_os = "macos")] @@ -1627,9 +1628,8 @@ impl Poll { let pollable = Pollable::from(event.udata as u64); let tag = pollable.tag(); - // udata=0 → tag=.empty is both the waker (registered only to unblock - // kevent() so the pending queue drains) and the EV_ERROR reply to an - // `ApplyAction::Cancel`, whose owner may already be gone. + // The waker is registered with udata=0 → tag=.empty. The wakeup exists + // only to unblock kevent() so the pending queue drains. if tag == PollableTag::Empty { return; } @@ -1638,9 +1638,8 @@ impl Poll { // `extern "Rust"` defined in `bun_runtime::dispatch`. The // container_of(io_poll) recovery happens there. // - // A rejected change comes back with EV_ERROR set in `flags`; xnu ORs it - // into the bits we submitted (EV_ADD|EV_ONESHOT|EV_ERROR) while FreeBSD - // replaces them, so test the bit rather than the whole word. + // xnu ORs EV_ERROR into the flags of a rejected change (EV_ADD|EV_ONESHOT + // |EV_ERROR); FreeBSD replaces them. Only the bit is common to both. if (event.flags & libc::EV_ERROR) != 0 { log!("error({}) = {}", event.ident, event.data); // SAFETY: poll is the `io_poll` field of a live owner; link-time diff --git a/test/internal/source-lints/kevent-ev-error-equality.test.ts b/test/internal/source-lints/kevent-ev-error-equality.test.ts index 325b923b4c49..2291e7794dae 100644 --- a/test/internal/source-lints/kevent-ev-error-equality.test.ts +++ b/test/internal/source-lints/kevent-ev-error-equality.test.ts @@ -43,7 +43,9 @@ const tracked: Set | null = (() => { // `bun_sys::darwin::EV::ERROR`, ... const CONSTANT = String.raw`(?:\w+::)*EV(?:_ERROR|::ERROR)\b`; // `x & EV_ERROR`: the bit test. `(? { test("the scan still sees the bit tests that are known to be in the tree", () => { // Named files rather than a count: if the file set or the comment handling // ever stops seeing one of these, the ban below would be vacuous for it. - // FilePoll's register/unregister and the process reaper's kevent loop. + // The io request loop's event dispatch, FilePoll's register/unregister and + // the process reaper's kevent loop. expect([...filesWithBitTests].sort()).toEqual( - expect.arrayContaining(["src/io/posix_event_loop.rs", "src/spawn/process.rs"]), + expect.arrayContaining(["src/io/lib.rs", "src/io/posix_event_loop.rs", "src/spawn/process.rs"]), ); }); @@ -110,21 +113,30 @@ test("the pattern recognizes the spellings it claims to", () => { "if libc::EV_ERROR == event.flags {", "let failed = event.flags == (EV::ERROR);", "if event.flags == EV_ERROR && event.data != 0 {", + // A bit test elsewhere on the line does not excuse a whole-word compare. + "if (event.flags & libc::EV_ERROR) != 0 && event.flags == libc::EV_ERROR {", + "let whole = flags == EV::ERROR || (flags & EV::ERROR) == EV::ERROR;", ]; - const allowed = [ + // Bit tests: allowed, and what the liveness check above counts. + const bitTests = [ "if (event.flags & libc::EV_ERROR) != 0 {", "if (changelist[0].flags & EV::ERROR) != 0 && changelist[0].data != 0 {", "if (changelist[i].flags & EV::ERROR) == 0 || changelist[i].data == 0 {", "if (event.flags & EV_ERROR) == EV_ERROR {", + "if (event.flags & libc::EV_ERROR) != libc::EV_ERROR {", "if r.flags & libc::EV_ERROR == 0 || r.data == 0 {", "if event.data != 0 && event.flags & EV_ERROR != 0 {", "let is_error = event.flags & EV::ERROR != 0;", + "let rejected = (kev.flags & EV::ERROR) != 0; // not kev.flags == EV::ERROR", + ]; + // Neither: mentions of the constant that are not tests of a flags word. + const neither = [ "pub const ERROR: u16 = libc::EV_ERROR;", " // xnu ORs EV_ERROR in, so `flags == EV_ERROR` is the bug this comment is about", - "let rejected = (kev.flags & EV::ERROR) != 0; // not kev.flags == EV::ERROR", ]; - expect(banned.filter(s => classify(s) !== "compared")).toEqual([]); - expect(allowed.filter(s => classify(s) === "compared")).toEqual([]); + expect(banned.filter(s => !comparesWholeWord(s))).toEqual([]); + expect(bitTests.filter(s => !hasBitTest(s) || comparesWholeWord(s))).toEqual([]); + expect(neither.filter(s => hasBitTest(s) || comparesWholeWord(s))).toEqual([]); }); test("kevent EV_ERROR is tested as a bit, never compared against the whole flags word", () => { From 44ea3ec6e28c293bc315e44614a90417c38e3674 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 13:25:23 +0000 Subject: [PATCH 4/5] io(kqueue): shorten the unregister_kqueue comments --- src/io/lib.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/io/lib.rs b/src/io/lib.rs index b2223980db4d..7c83974ff0b3 100644 --- a/src/io/lib.rs +++ b/src/io/lib.rs @@ -1600,15 +1600,13 @@ impl Poll { self.flags.remove(Flags::Registered); } - /// Applied on the spot rather than with the next batch: the caller hands - /// the owner to another thread as soon as this returns. + /// Not batched into the next `kevent()`: the caller hands the owner off right after this. #[cfg(any(target_os = "macos", target_os = "freebsd"))] pub(crate) fn unregister_kqueue(&mut self, watcher_fd: Fd, fd: Fd) { let mut change: KEvent = bun_core::ffi::zeroed(); Poll::apply_kqueue(ApplyAction::Cancel, self, fd, &mut change); - // With nevents = 0 the call returns as soon as the change is applied. - // It fails with ENOENT once the one-shot registration has fired and - // with EBADF once the fd is closed; either way nothing is left to remove. + // nevents = 0: apply only. ENOENT (one-shot already fired) and EBADF + // (fd already closed) are the usual outcomes; nothing to do for either. let _ = kevent_call( watcher_fd.native(), &raw const change, From f386e87bfd57155d3d57ddea8fae008ccaa1fc17 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 19:46:15 +0000 Subject: [PATCH 5/5] ci: retrigger