From 35a4dea6dd9c37e9c72308c359cbffec4a743c5b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:58:04 +0000 Subject: [PATCH] sink: detach JSSink controller when assignToStream throws FileSink__assignToStream (and the other generated ${name}__assignToStream functions) create a JSReadable*SinkController with m_sinkPtr set before calling into the stream pump. When the pump setup throws (for example a direct ReadableStream whose `pull` getter throws), the controller is never started, so nothing ever calls end()/close() to null m_sinkPtr. The caller's error path then frees the native sink, and when the controller is later swept its destructor calls ${name}__controllerDetached / ${name}__finalize on freed memory. Under ASAN this shows up as a heap-use-after-free in JSSink::js_controller_detached from JSReadableFileSinkController's destructor. Bun.spawn({stdio:[stream,..]}) with a throwing `get pull` is enough to reach it; the two existing spawn.test.ts cases for this error path were failing on release-asan lanes for this reason. Fix it in the generic JSSink::assign_to_stream wrapper: when the extern call returns an error, call JSSinkController__detachPtr on the freshly created controller while the sink is still live, and clear the sink's SourceHandle. The controller's later GC then sees m_sinkPtr==null and skips the native finalize. The two spawn tests now force a full GC in the child fixture so the controller destructor runs deterministically under debug+ASAN as well. --- src/runtime/webcore/Sink.rs | 16 ++++++++++++++++ test/js/bun/spawn/spawn.test.ts | 3 +++ 2 files changed, 19 insertions(+) diff --git a/src/runtime/webcore/Sink.rs b/src/runtime/webcore/Sink.rs index b7e1127eeac0..d84639a287a9 100644 --- a/src/runtime/webcore/Sink.rs +++ b/src/runtime/webcore/Sink.rs @@ -209,6 +209,22 @@ impl JSSink { std::ptr::from_mut::(ptr).cast::(), (&raw mut bits).cast::<*mut c_void>(), ); + // `${name}__assignToStream` creates the JSReadable*SinkController with + // m_sinkPtr=ptr before calling into the stream pump. If the pump setup + // throws (e.g. a direct stream's `pull` getter), nothing ever calls + // end()/close() on the controller, so its destructor would run + // `${name}__finalize(m_sinkPtr)` after the caller has freed the sink. + // Detach it now while `ptr` is still live; the controller's later GC + // then sees m_sinkPtr==null and skips the native finalize. + if bits != 0 && result.to_error().is_some() { + if let Some(src) = ptr.source() { + *src = streams::SourceHandle::None; + } + let _ = ::bun_jsc::call_check_slow(global, || { + streams::controller_abi::detach_ptr(JSValue::from_encoded(bits)) + }); + return result; + } if let Some(src) = ptr.source() { if matches!(*src, streams::SourceHandle::JSController(_)) { *src = if bits != 0 { diff --git a/test/js/bun/spawn/spawn.test.ts b/test/js/bun/spawn/spawn.test.ts index 46ec5a28bc28..5b6da5beda1c 100644 --- a/test/js/bun/spawn/spawn.test.ts +++ b/test/js/bun/spawn/spawn.test.ts @@ -1252,6 +1252,7 @@ it.skipIf(isWindows)("leaves a caller-supplied stdout fd open when stdin stream fstatSync(fd); writeSync(fd, "still-open"); closeSync(fd); + Bun.gc(true); console.log(message); process.exit(0); `; @@ -1261,6 +1262,7 @@ it.skipIf(isWindows)("leaves a caller-supplied stdout fd open when stdin stream stdio: ["ignore", "pipe", "pipe"], }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); expect(stdout.trim()).toBe("pull unavailable"); expect(readFileSync(file, "utf8")).toContain("still-open"); expect(exitCode).toBe(0); @@ -1293,6 +1295,7 @@ it.skipIf(isWindows)("leaves a Bun.file(fd) stdout open when stdin stream setup fstatSync(fd); writeSync(fd, "still-open"); closeSync(fd); + Bun.gc(true); console.log(message); process.exit(0); `;