From f29b67c3182c1a3751277d90a1adaa2d2b3a8b25 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 30 Sep 2026 04:44:24 +0000 Subject: [PATCH 1/7] io: POSIX pipe writers hold a ref on their parent from on_error until close() returns `_on_error` in `PosixStreamingWriter` and `PosixBufferedWriter` reports the error to the parent and then closes the writer. The writer is a field of the parent, and `on_error` can release every other ref on the parent: `FileSink` rejects a promise in it, and for a stream piped in by `Bun.write` the rejection handlers close the sink and release the pump's ref. The `close()` that follows then ran on freed memory (ASAN: heap-use-after-free in `PosixStreamingWriter::close`). Both writers now take a ref on the parent before `on_error` and release it after `close()`, through a new `PosixWriterParent` trait (`ref_`/`deref`) that the parent macros stamp from the expressions they already receive for Windows. The failed-registration arm of `PosixStreamingWriter::register_poll` has the same order and takes the same ref. --- src/io/PipeWriter.rs | 110 ++++++++++++++++++++++++++---- src/runtime/api/bun/Terminal.rs | 13 ++++ src/runtime/shell/IOWriter.rs | 4 +- test/js/bun/util/filesink.test.ts | 69 +++++++++++++++++++ 4 files changed, 182 insertions(+), 14 deletions(-) diff --git a/src/io/PipeWriter.rs b/src/io/PipeWriter.rs index 4b949b9ddcfd..d0b488a4bf8b 100644 --- a/src/io/PipeWriter.rs +++ b/src/io/PipeWriter.rs @@ -271,6 +271,46 @@ fn write_to_socket(fd: Fd, buf: &[u8]) -> sys::Result { }) } +// ────────────────────────────────────────────────────────────────────────── +// PosixWriterParent +// ────────────────────────────────────────────────────────────────────────── + +/// The parent's refcount, for both POSIX writers: a writer is a field of its +/// parent and is freed with it. +/// +/// Methods take `*mut Self`: see [`PosixBufferedWriterParent`]. +pub trait PosixWriterParent { + /// # Safety + /// `this` must point to a live `Self`. + unsafe fn ref_(this: *mut Self); + /// May free `this`. + /// + /// # Safety + /// `this` must point to a live `Self`, and the caller must own one ref. + unsafe fn deref(this: *mut Self); +} + +/// One ref on a writer's parent, released on drop. The release can free the +/// parent and the writer in it, so the guard has to outlive every use of the writer. +struct ParentKeepAlive(*mut Parent); + +impl ParentKeepAlive { + /// # Safety + /// `parent` must point to a live `Parent`. + unsafe fn new(parent: *mut Parent) -> Self { + // SAFETY: caller contract. + unsafe { Parent::ref_(parent) }; + Self(parent) + } +} + +impl Drop for ParentKeepAlive { + fn drop(&mut self) { + // SAFETY: the ref taken in `new` kept the parent live until here. + unsafe { Parent::deref(self.0) }; + } +} + // ────────────────────────────────────────────────────────────────────────── // PosixBufferedWriter // ────────────────────────────────────────────────────────────────────────── @@ -283,7 +323,7 @@ fn write_to_socket(fd: Fd, buf: &[u8]) -> sys::Result { /// Materializing `&mut Parent` while a `&mut writer` is live would alias under /// Stacked Borrows, so we use raw /// pointers and never form a `&mut Parent` inside the writer. -pub trait PosixBufferedWriterParent { +pub trait PosixBufferedWriterParent: PosixWriterParent { /// `bun_io::poll_tag` constant for this writer's `FilePoll` owner. The /// per-tag dispatch in `bun_runtime::dispatch::__bun_run_file_poll` /// recovers `*mut PosixBufferedWriter` from this. @@ -433,6 +473,9 @@ impl PosixBufferedWriter { fn _on_error(&mut self, err: sys::Error) { debug_assert!(!err.is_retry()); + // `on_error` can release every other ref, and `close()` is still to run. + // SAFETY: type invariant — set-once parent backref outlives writer. + let _keep_alive = unsafe { ParentKeepAlive::new(self.parent()) }; self.parent_on_error(err); self.close(); @@ -611,7 +654,7 @@ impl PosixBufferedWriter { /// Materializing `&mut Parent` while a `&mut writer` is live would alias under /// Stacked Borrows, so we use raw /// pointers and never form a `&mut Parent` inside the writer. -pub trait PosixStreamingWriterParent { +pub trait PosixStreamingWriterParent: PosixWriterParent { /// `bun_io::poll_tag` constant for this writer's `FilePoll` owner. The /// per-tag dispatch in `bun_runtime::dispatch::__bun_run_file_poll` /// recovers `*mut PosixStreamingWriter` from this. @@ -785,8 +828,13 @@ impl PosixStreamingWriter { self.is_done = true; self.outgoing.reset(); + let parent = self.parent(); + // `on_error` can release every other ref (`FileSink` rejects a promise + // in it), and `close()` is still to run. // SAFETY: parent BACKREF set via set_parent; outlives this writer. - unsafe { Parent::on_error(self.parent(), err) }; + let _keep_alive = unsafe { ParentKeepAlive::new(parent) }; + // SAFETY: as above. + unsafe { Parent::on_error(parent, err) }; self.close(); } @@ -837,10 +885,12 @@ impl PosixStreamingWriter { // ASM-verified PROVEN_CACHED on the `self.close()` path's // field reads. Launder so `close()` sees fresh state. let this: *mut Self = core::hint::black_box(core::ptr::from_mut(self)); + let parent = Self::r(this).parent(); + // As in `_on_error`: `close()` is still to run. + // SAFETY: parent BACKREF valid. + let _keep_alive = unsafe { ParentKeepAlive::new(parent) }; // SAFETY: parent BACKREF valid. - unsafe { Parent::on_error(Self::r(this).parent(), err) }; - // `this` is still live (parent owns this writer; an on_error - // handler may end/detach but never frees mid-call). + unsafe { Parent::on_error(parent, err) }; Self::r(this).close(); } sys::Result::Ok(()) => {} @@ -2620,15 +2670,15 @@ pub type StreamingWriter

= WindowsStreamingWriter

; // method is `unsafe fn(this: *mut Self, ..)` // that derefs the BACKREF and forwards to an inherent method. Every concrete // parent (FileSink, Terminal, WindowsNamedPipe, shell IOWriter, -// StaticPipeWriter) was hand-stamping the same triple of cfg-gated impls -// (POSIX + WindowsWriterParent + Windows{Streaming,Buffered}WriterParent), +// StaticPipeWriter) was hand-stamping the same cfg-gated impls +// ({Posix,Windows}WriterParent + {Posix,Windows}{Streaming,Buffered}WriterParent), // differing only in: // (a) the inherent-method names the vtable forwards to, // (b) how the callback is dispatched off `*mut Self` — as `&mut`, `&`, or // a raw-ptr method call (re-entrancy under Stacked/Tree Borrows — see // `borrow = shared` / `borrow = ptr` callers), // (c) the `event_loop` / `loop_` / refcount accessor expressions. -// These macros stamp that triple once per parent. +// These macros stamp those impls once per parent. // // `borrow = mut` → bodies form `&mut *this` (unique access for the // callback's duration; the writer never holds @@ -2657,7 +2707,7 @@ pub mod __parent_macro { pub use ::bun_uws_sys::Loop as UwsLoop; } -/// Stamp `PosixStreamingWriterParent` + `WindowsWriterParent` + +/// Stamp `PosixWriterParent` + `PosixStreamingWriterParent` + `WindowsWriterParent` + /// `WindowsStreamingWriterParent` for a parent type. See module comment above. #[macro_export] macro_rules! impl_streaming_writer_parent { @@ -2666,7 +2716,7 @@ macro_rules! impl_streaming_writer_parent { (@call shared $p:expr; $m:ident($($a:tt)*)) => { (&*$p).$m($($a)*) }; (@call ptr $p:expr; $m:ident($($a:tt)*)) => { ::$m($p, $($a)*) }; - // Internal: expand the three impls once generics are normalized. + // Internal: expand the impls once generics are normalized. (@emit [$($gen:tt)*] $Ty:ty; poll_tag = $poll_tag:expr, @@ -2724,6 +2774,24 @@ macro_rules! impl_streaming_writer_parent { } } + #[cfg(unix)] + impl $($gen)* $crate::pipe_writer::PosixWriterParent for $Ty { + #[inline] + unsafe fn ref_(this: *mut Self) { + // SAFETY: BACKREF set via `set_parent`. Intrusive refcount bump. + let $ref_this = this; + #[allow(unused_unsafe)] + unsafe { $ref_ }; + } + #[inline] + unsafe fn deref(this: *mut Self) { + // SAFETY: see ref_. May free `this`. + let $deref_this = this; + #[allow(unused_unsafe)] + unsafe { $deref }; + } + } + #[cfg(windows)] impl $($gen)* $crate::pipe_writer::WindowsWriterParent for $Ty { #[inline] @@ -2795,7 +2863,7 @@ macro_rules! impl_streaming_writer_parent { }; } -/// Stamp `PosixBufferedWriterParent` + `WindowsWriterParent` + +/// Stamp `PosixWriterParent` + `PosixBufferedWriterParent` + `WindowsWriterParent` + /// `WindowsBufferedWriterParent` for a parent type. See module comment above. #[macro_export] macro_rules! impl_buffered_writer_parent { @@ -2852,6 +2920,24 @@ macro_rules! impl_buffered_writer_parent { } } + #[cfg(not(windows))] + impl $($gen)* $crate::pipe_writer::PosixWriterParent for $Ty { + #[inline] + unsafe fn ref_(this: *mut Self) { + // SAFETY: BACKREF set via `set_parent`. Intrusive refcount bump. + let $ref_this = this; + #[allow(unused_unsafe)] + unsafe { $ref_ }; + } + #[inline] + unsafe fn deref(this: *mut Self) { + // SAFETY: see ref_. May free `this`. + let $deref_this = this; + #[allow(unused_unsafe)] + unsafe { $deref }; + } + } + #[cfg(windows)] impl $($gen)* $crate::pipe_writer::WindowsWriterParent for $Ty { #[inline] diff --git a/src/runtime/api/bun/Terminal.rs b/src/runtime/api/bun/Terminal.rs index a41db046700e..b680000c514e 100644 --- a/src/runtime/api/bun/Terminal.rs +++ b/src/runtime/api/bun/Terminal.rs @@ -2077,6 +2077,19 @@ impl bun_io::pipe_writer::PosixStreamingWriterParent for Terminal { } } +#[cfg(unix)] +impl bun_io::pipe_writer::PosixWriterParent for Terminal { + unsafe fn ref_(this: *mut Self) { + // SAFETY: BACKREF set via writer.parent. Through the raw pointer, as the + // Windows `WindowsWriterParent::ref_` below: a `&mut self.writer` is live. + unsafe { bun_ptr::RefCount::::ref_(this) }; + } + unsafe fn deref(this: *mut Self) { + // SAFETY: see ref_. May free `this`. + unsafe { bun_ptr::RefCount::::deref(this) }; + } +} + #[cfg(windows)] impl bun_io::pipe_writer::WindowsWriterParent for Terminal { unsafe fn loop_(this: *mut Self) -> *mut bun_libuv_sys::Loop { diff --git a/src/runtime/shell/IOWriter.rs b/src/runtime/shell/IOWriter.rs index 199f617f2afb..9dd37ae29c35 100644 --- a/src/runtime/shell/IOWriter.rs +++ b/src/runtime/shell/IOWriter.rs @@ -1088,8 +1088,8 @@ bun_io::impl_buffered_writer_parent! { uv_loop = |this| (*(*this).evtloop().loop_()).uv_loop, // INVARIANT: `this` is `Arc::as_ptr` stashed via `writer.set_parent` in // `IOWriter::init` (sole constructor); passing a non-Arc ptr is UB. - ref_ = |this| std::sync::Arc::increment_strong_count(this as *const Self), - deref = |this| std::sync::Arc::decrement_strong_count(this as *const Self), + ref_ = |this| std::sync::Arc::increment_strong_count(this.cast_const()), + deref = |this| std::sync::Arc::decrement_strong_count(this.cast_const()), } // ────────────────────────────────────────────────────────────────────────── diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index 89648f414baa..9276e74f8b92 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -1120,6 +1120,18 @@ describe("a stream piped into a FileSink on a pipe is pumped to its end", () => c.close(); }, }`, + // pull() waits for the sink to write the whole chunk, so the sink holds a promise that + // it has to reject when the reader goes away. + directFlush: `{ + type: "direct", + async pull(c) { + c.write(first); + const flushed = c.flush(); + console.error("parked"); + await flushed; + c.close(); + }, + }`, }; function fixture(source: string, dest: string, countBeforeExit = false) { @@ -1330,6 +1342,63 @@ describe("a stream piped into a FileSink on a pipe is pumped to its end", () => fs.closeSync(readFd); } }); + + // The reader takes part of the stream and goes away while pull() awaits flush(). The child + // finds out in its poll of stdout, and the sink rejects the flush from that callback. + // pull() then fails, which closes the sink and drops the last refs on it, and the pipe + // writer, a field of the sink, still had a close of its own to make (ASAN: + // heap-use-after-free in PosixStreamingWriter::close). + // + // The flush that ends each turn of the child's loop writes too, and a failure there is + // reported from another place. So the reader closes while the child waits in its poll: it + // leaves each refill in the pipe for a pause, and closes in the step that takes the last. + it.concurrent.skipIf(!isPosix)("a reader that goes away while pull() awaits flush() rejects the promise", async () => { + using dir = tempDir("filesink-piped-reader-gone", {}); + const path = join(String(dir), "stdout.fifo"); + mkfifo(path, 0o666); + const readFd = fs.openSync(path, fs.constants.O_RDONLY | fs.constants.O_NONBLOCK); + let readFdOpen = true; + let writeFd: number | undefined; + try { + writeFd = fs.openSync(path, fs.constants.O_WRONLY); + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture(sources.directFlush, "Bun.stdout")], + env: bunEnv, + stdout: writeFd, + stderr: "pipe", + }); + // The child holds the only write end now: a read of 0 bytes means it is gone. + fs.closeSync(writeFd); + writeFd = undefined; + + const rest = await parked(proc.stderr); + const buffer = Buffer.alloc(64 * 1024); + let taken = 0; + while (taken < 4 * buffer.length) { + let n: number; + try { + n = fs.readSync(readFd, buffer); + } catch (e: any) { + if (e.code !== "EAGAIN") throw e; + await Bun.sleep(10); + continue; + } + if (n === 0) break; + taken += n; + } + fs.closeSync(readFd); + readFdOpen = false; + + const [stderr, exitCode] = await Promise.all([rest(), proc.exited]); + expect({ stderr, exitCode }).toEqual({ + stderr: "parked\n" + JSON.stringify({ settled: "rejected EPIPE: broken pipe, write", code: 0 }) + "\n", + exitCode: 0, + }); + } finally { + if (writeFd !== undefined) fs.closeSync(writeFd); + if (readFdOpen) fs.closeSync(readFd); + } + }); }); // FileSink::on_close tells the owner of the sink that it closed, and a Subprocess then drops its ref From b50d318efaba3c5c97c04dce7337a9666d6788cc Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 22:58:41 +0000 Subject: [PATCH 2/7] [autofix.ci] apply automated fixes --- test/js/bun/util/filesink.test.ts | 91 ++++++++++++++++--------------- 1 file changed, 47 insertions(+), 44 deletions(-) diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index 9276e74f8b92..d5bf04f6a42d 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -1352,53 +1352,56 @@ describe("a stream piped into a FileSink on a pipe is pumped to its end", () => // The flush that ends each turn of the child's loop writes too, and a failure there is // reported from another place. So the reader closes while the child waits in its poll: it // leaves each refill in the pipe for a pause, and closes in the step that takes the last. - it.concurrent.skipIf(!isPosix)("a reader that goes away while pull() awaits flush() rejects the promise", async () => { - using dir = tempDir("filesink-piped-reader-gone", {}); - const path = join(String(dir), "stdout.fifo"); - mkfifo(path, 0o666); - const readFd = fs.openSync(path, fs.constants.O_RDONLY | fs.constants.O_NONBLOCK); - let readFdOpen = true; - let writeFd: number | undefined; - try { - writeFd = fs.openSync(path, fs.constants.O_WRONLY); - await using proc = Bun.spawn({ - cmd: [bunExe(), "-e", fixture(sources.directFlush, "Bun.stdout")], - env: bunEnv, - stdout: writeFd, - stderr: "pipe", - }); - // The child holds the only write end now: a read of 0 bytes means it is gone. - fs.closeSync(writeFd); - writeFd = undefined; + it.concurrent.skipIf(!isPosix)( + "a reader that goes away while pull() awaits flush() rejects the promise", + async () => { + using dir = tempDir("filesink-piped-reader-gone", {}); + const path = join(String(dir), "stdout.fifo"); + mkfifo(path, 0o666); + const readFd = fs.openSync(path, fs.constants.O_RDONLY | fs.constants.O_NONBLOCK); + let readFdOpen = true; + let writeFd: number | undefined; + try { + writeFd = fs.openSync(path, fs.constants.O_WRONLY); + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture(sources.directFlush, "Bun.stdout")], + env: bunEnv, + stdout: writeFd, + stderr: "pipe", + }); + // The child holds the only write end now: a read of 0 bytes means it is gone. + fs.closeSync(writeFd); + writeFd = undefined; - const rest = await parked(proc.stderr); - const buffer = Buffer.alloc(64 * 1024); - let taken = 0; - while (taken < 4 * buffer.length) { - let n: number; - try { - n = fs.readSync(readFd, buffer); - } catch (e: any) { - if (e.code !== "EAGAIN") throw e; - await Bun.sleep(10); - continue; + const rest = await parked(proc.stderr); + const buffer = Buffer.alloc(64 * 1024); + let taken = 0; + while (taken < 4 * buffer.length) { + let n: number; + try { + n = fs.readSync(readFd, buffer); + } catch (e: any) { + if (e.code !== "EAGAIN") throw e; + await Bun.sleep(10); + continue; + } + if (n === 0) break; + taken += n; } - if (n === 0) break; - taken += n; - } - fs.closeSync(readFd); - readFdOpen = false; + fs.closeSync(readFd); + readFdOpen = false; - const [stderr, exitCode] = await Promise.all([rest(), proc.exited]); - expect({ stderr, exitCode }).toEqual({ - stderr: "parked\n" + JSON.stringify({ settled: "rejected EPIPE: broken pipe, write", code: 0 }) + "\n", - exitCode: 0, - }); - } finally { - if (writeFd !== undefined) fs.closeSync(writeFd); - if (readFdOpen) fs.closeSync(readFd); - } - }); + const [stderr, exitCode] = await Promise.all([rest(), proc.exited]); + expect({ stderr, exitCode }).toEqual({ + stderr: "parked\n" + JSON.stringify({ settled: "rejected EPIPE: broken pipe, write", code: 0 }) + "\n", + exitCode: 0, + }); + } finally { + if (writeFd !== undefined) fs.closeSync(writeFd); + if (readFdOpen) fs.closeSync(readFd); + } + }, + ); }); // FileSink::on_close tells the owner of the sink that it closed, and a Subprocess then drops its ref From 4d0c22a1863b23e667106a68f0df0e734ccbdb36 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 30 Sep 2026 23:38:11 +0000 Subject: [PATCH 3/7] PipeWriter: one-line comments for the parent ref No code change: the docs of PosixWriterParent and ParentKeepAlive are one line each, the section banner is gone, and the module comment above the parent macros is back to the text on main. --- src/io/PipeWriter.rs | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/src/io/PipeWriter.rs b/src/io/PipeWriter.rs index d0b488a4bf8b..0df385cfcaa0 100644 --- a/src/io/PipeWriter.rs +++ b/src/io/PipeWriter.rs @@ -271,31 +271,18 @@ fn write_to_socket(fd: Fd, buf: &[u8]) -> sys::Result { }) } -// ────────────────────────────────────────────────────────────────────────── -// PosixWriterParent -// ────────────────────────────────────────────────────────────────────────── - -/// The parent's refcount, for both POSIX writers: a writer is a field of its -/// parent and is freed with it. -/// -/// Methods take `*mut Self`: see [`PosixBufferedWriterParent`]. +/// The refcount of the parent that a POSIX writer is a field of. pub trait PosixWriterParent { - /// # Safety /// `this` must point to a live `Self`. unsafe fn ref_(this: *mut Self); - /// May free `this`. - /// - /// # Safety - /// `this` must point to a live `Self`, and the caller must own one ref. + /// `this` must point to a live `Self` that the caller holds a ref on. May free it. unsafe fn deref(this: *mut Self); } -/// One ref on a writer's parent, released on drop. The release can free the -/// parent and the writer in it, so the guard has to outlive every use of the writer. +/// A ref on the parent of a writer. Its drop can free the parent and the writer in it. struct ParentKeepAlive(*mut Parent); impl ParentKeepAlive { - /// # Safety /// `parent` must point to a live `Parent`. unsafe fn new(parent: *mut Parent) -> Self { // SAFETY: caller contract. @@ -829,8 +816,7 @@ impl PosixStreamingWriter { self.outgoing.reset(); let parent = self.parent(); - // `on_error` can release every other ref (`FileSink` rejects a promise - // in it), and `close()` is still to run. + // `on_error` can release every other ref on the parent, and `close()` is still to run. // SAFETY: parent BACKREF set via set_parent; outlives this writer. let _keep_alive = unsafe { ParentKeepAlive::new(parent) }; // SAFETY: as above. @@ -2670,15 +2656,15 @@ pub type StreamingWriter

= WindowsStreamingWriter

; // method is `unsafe fn(this: *mut Self, ..)` // that derefs the BACKREF and forwards to an inherent method. Every concrete // parent (FileSink, Terminal, WindowsNamedPipe, shell IOWriter, -// StaticPipeWriter) was hand-stamping the same cfg-gated impls -// ({Posix,Windows}WriterParent + {Posix,Windows}{Streaming,Buffered}WriterParent), +// StaticPipeWriter) was hand-stamping the same triple of cfg-gated impls +// (POSIX + WindowsWriterParent + Windows{Streaming,Buffered}WriterParent), // differing only in: // (a) the inherent-method names the vtable forwards to, // (b) how the callback is dispatched off `*mut Self` — as `&mut`, `&`, or // a raw-ptr method call (re-entrancy under Stacked/Tree Borrows — see // `borrow = shared` / `borrow = ptr` callers), // (c) the `event_loop` / `loop_` / refcount accessor expressions. -// These macros stamp those impls once per parent. +// These macros stamp that triple once per parent. // // `borrow = mut` → bodies form `&mut *this` (unique access for the // callback's duration; the writer never holds From 0fb95d10a47c650244b06dc95d724a3a91622ce6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:04:50 +0000 Subject: [PATCH 4/7] test(filesink): give the reader's poll a no-progress deadline --- test/js/bun/util/filesink.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index d5bf04f6a42d..3cd877cc3379 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -1376,17 +1376,22 @@ describe("a stream piped into a FileSink on a pipe is pumped to its end", () => const rest = await parked(proc.stderr); const buffer = Buffer.alloc(64 * 1024); let taken = 0; + let progressAt = performance.now(); while (taken < 4 * buffer.length) { let n: number; try { n = fs.readSync(readFd, buffer); } catch (e: any) { if (e.code !== "EAGAIN") throw e; + if (performance.now() - progressAt > 10_000) { + throw new Error(`the child holds the FIFO open and wrote nothing for 10 s, after ${taken} bytes`); + } await Bun.sleep(10); continue; } if (n === 0) break; taken += n; + progressAt = performance.now(); } fs.closeSync(readFd); readFdOpen = false; From d0628b8840b252fde12c1cd080354b643e7e24a6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 02:47:21 +0000 Subject: [PATCH 5/7] PipeWriter: keep the failed-registration arm of register_poll out of line register_poll runs on every buffered write. With the parent ref in its error arm it was no longer inlined into write(). The arm is now a #[cold] function, so the hot path is as small as before. --- src/io/PipeWriter.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/io/PipeWriter.rs b/src/io/PipeWriter.rs index 0df385cfcaa0..b5cdf3896bb6 100644 --- a/src/io/PipeWriter.rs +++ b/src/io/PipeWriter.rs @@ -871,18 +871,25 @@ impl PosixStreamingWriter { // ASM-verified PROVEN_CACHED on the `self.close()` path's // field reads. Launder so `close()` sees fresh state. let this: *mut Self = core::hint::black_box(core::ptr::from_mut(self)); - let parent = Self::r(this).parent(); - // As in `_on_error`: `close()` is still to run. - // SAFETY: parent BACKREF valid. - let _keep_alive = unsafe { ParentKeepAlive::new(parent) }; - // SAFETY: parent BACKREF valid. - unsafe { Parent::on_error(parent, err) }; - Self::r(this).close(); + Self::on_register_poll_error(this, err); } sys::Result::Ok(()) => {} } } + /// Out of line so that `register_poll`, which runs on every buffered write, stays small. + #[cold] + #[inline(never)] + fn on_register_poll_error(this: *mut Self, err: sys::Error) { + let parent = Self::r(this).parent(); + // As in `_on_error`: `close()` is still to run. + // SAFETY: parent BACKREF valid. + let _keep_alive = unsafe { ParentKeepAlive::new(parent) }; + // SAFETY: parent BACKREF valid. + unsafe { Parent::on_error(parent, err) }; + Self::r(this).close(); + } + pub fn write_utf16(&mut self, buf: &[u16]) -> WriteResult { if self.is_done || self.closed_without_reporting { return WriteResult::Done(0); From 944c8bb76b87ea3464bbc830eff93a5ade2bd398 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 04:20:41 +0000 Subject: [PATCH 6/7] test(filesink): let the child go back to its poll before the reader goes away The reader closed in the same step as its last read. That only finds the bug when the close lands before the child's next write: on a fast build the child often writes first, and the flush at the end of that turn reports the error instead. The reader now takes its bytes through drain(), waits a moment, and then closes. --- test/js/bun/util/filesink.test.ts | 35 ++++++++++--------------------- 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index 3cd877cc3379..618936479e4f 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -1170,8 +1170,9 @@ describe("a stream piped into a FileSink on a pipe is pumped to its end", () => } // read(2) in a poll on the non-blocking read end the test holds: it returns 0 once the - // writer is gone, on every POSIX, and needs no readiness notification. - async function drain(fd: number) { + // writer is gone, on every POSIX, and needs no readiness notification. With a `limit` it + // stops once that many bytes have arrived. + async function drain(fd: number, limit = Infinity) { const buffer = Buffer.alloc(64 * 1024); const parts: Buffer[] = []; let total = 0; @@ -1192,6 +1193,7 @@ describe("a stream piped into a FileSink on a pipe is pumped to its end", () => parts.push(Buffer.from(buffer.subarray(0, n))); total += n; progressAt = performance.now(); + if (total >= limit) return Buffer.concat(parts); } } @@ -1350,8 +1352,9 @@ describe("a stream piped into a FileSink on a pipe is pumped to its end", () => // heap-use-after-free in PosixStreamingWriter::close). // // The flush that ends each turn of the child's loop writes too, and a failure there is - // reported from another place. So the reader closes while the child waits in its poll: it - // leaves each refill in the pipe for a pause, and closes in the step that takes the last. + // reported from another place. So the read end has to close while the child waits in its + // poll, and no signal says when it does. The reader takes a few refills, which puts the + // child past the turn that printed "parked", and then gives it a moment to handle the last. it.concurrent.skipIf(!isPosix)( "a reader that goes away while pull() awaits flush() rejects the promise", async () => { @@ -1374,30 +1377,14 @@ describe("a stream piped into a FileSink on a pipe is pumped to its end", () => writeFd = undefined; const rest = await parked(proc.stderr); - const buffer = Buffer.alloc(64 * 1024); - let taken = 0; - let progressAt = performance.now(); - while (taken < 4 * buffer.length) { - let n: number; - try { - n = fs.readSync(readFd, buffer); - } catch (e: any) { - if (e.code !== "EAGAIN") throw e; - if (performance.now() - progressAt > 10_000) { - throw new Error(`the child holds the FIFO open and wrote nothing for 10 s, after ${taken} bytes`); - } - await Bun.sleep(10); - continue; - } - if (n === 0) break; - taken += n; - progressAt = performance.now(); - } + const received = await drain(readFd, 4 * 64 * 1024); + await Bun.sleep(10); fs.closeSync(readFd); readFdOpen = false; const [stderr, exitCode] = await Promise.all([rest(), proc.exited]); - expect({ stderr, exitCode }).toEqual({ + expect({ intact: expected.subarray(0, received.length).equals(received), stderr, exitCode }).toEqual({ + intact: true, stderr: "parked\n" + JSON.stringify({ settled: "rejected EPIPE: broken pipe, write", code: 0 }) + "\n", exitCode: 0, }); From 6861b2523c7034c8d1b122e05396a83fea4f9a99 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 1 Oct 2026 14:22:10 +0000 Subject: [PATCH 7/7] PipeWriter: document what on_error may release, update two comments The SAFETY note in StaticPipeWriter::on_close said that its deref frees the writer on POSIX. After a failed write the buffered writer's _on_error now holds a ref until close() returns, so that deref is not the last one. FileSink::on_error no longer claims that the keep-alive ref alone keeps the sink alive until close(). No code change. --- src/io/PipeWriter.rs | 2 ++ src/runtime/webcore/FileSink.rs | 3 +-- src/spawn/static_pipe_writer.rs | 5 ++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/io/PipeWriter.rs b/src/io/PipeWriter.rs index b5cdf3896bb6..b18de4eb6731 100644 --- a/src/io/PipeWriter.rs +++ b/src/io/PipeWriter.rs @@ -318,6 +318,7 @@ pub trait PosixBufferedWriterParent: PosixWriterParent { /// # Safety /// `this` must point to a live `Self`. unsafe fn on_write(this: *mut Self, amount: usize, status: WriteStatus); + /// May release every other ref on `this`: the writer holds one until its `close()` returns. /// # Safety /// `this` must point to a live `Self`. unsafe fn on_error(this: *mut Self, err: sys::Error); @@ -649,6 +650,7 @@ pub trait PosixStreamingWriterParent: PosixWriterParent { /// # Safety /// `this` must point to a live `Self`. unsafe fn on_write(this: *mut Self, amount: usize, status: WriteStatus); + /// May release every other ref on `this`: the writer holds one until its `close()` returns. /// # Safety /// `this` must point to a live `Self`. unsafe fn on_error(this: *mut Self, err: sys::Error); diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index cd1d3348604b..5abdedef43ac 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -464,8 +464,7 @@ impl FileSink { bun_core::scoped_log!(FileSink, "onError({:?})", err); // The streaming writer follows every `onError` with `close()` → // `onClose` (on both platforms), which fires `source.close()` and - // releases the keep-alive ref. Releasing the ref here instead could - // drop the last reference and free `this` before that `close()` runs. + // releases the keep-alive ref. `run_pending` can release every other ref on `this`. // SAFETY: caller contract — `this` is live with write+dealloc provenance. unsafe { (*this).record_stream_error(streams::StreamError::Error(err.clone())); diff --git a/src/spawn/static_pipe_writer.rs b/src/spawn/static_pipe_writer.rs index 68db15028e23..91fd8690018a 100644 --- a/src/spawn/static_pipe_writer.rs +++ b/src/spawn/static_pipe_writer.rs @@ -280,9 +280,8 @@ impl StaticPipeWriter

{ // for the lifetime of this writer (the process owns/outlives its stdio writers). unsafe { P::on_close_io(self.process, StdioKind::Stdin) }; if release_start_ref { - // SAFETY: token taken above. On POSIX this frees `self`: it is the - // last use here, and the writer's `close()` frames below do nothing - // after this callback. On Windows the in-flight write's ref outlives it. + // SAFETY: token taken above. Not the last ref: on POSIX the writer's `_on_error` + // holds one until its `close()` returns, on Windows the in-flight write does. unsafe { RefCount::::deref(std::ptr::from_mut::(self)) }; } }