From f3f6c64ff9ec782da38c7874180db903d3832f3c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 6 Sep 2026 21:38:21 +0000 Subject: [PATCH 1/5] test --isolate: restore signal dispositions and kill Bun.$ children at the file swap A process.on() listener left behind by a finished file kept the native handler installed with no JS target, so a later SIGTERM/SIGINT was swallowed and the run finished rc 0. Remove the outgoing process emitter's listeners through removeAllListeners() so the existing count-reached-zero teardown restores SIG_DFL. Shell (Bun.$) subprocesses never registered with the VM's auto-killer, so neither the isolation swap nor the test-timeout kill reached them. Register them next to Bun.spawn children. --- src/jsc/bindings/ZigGlobalObject.cpp | 7 ++ src/runtime/shell/subproc.rs | 46 ++++++++++++-- test/cli/test/isolation.test.ts | 95 +++++++++++++++++++++++++++- 3 files changed, 142 insertions(+), 6 deletions(-) diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index 1bcd27a1a078..69d8eadfc498 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -4377,6 +4377,13 @@ extern "C" void Zig__GlobalObject__forbidExecution(Zig::GlobalObject* globalObje extern "C" void Zig__GlobalObject__stopActiveDOMObjectsForTestIsolation(Zig::GlobalObject* globalObject) { Bun::retireWebViewsForTestIsolation(globalObject); + // A `process.on()` listener installs a process-wide sigaction (a uv_signal_t on + // Windows) that is only torn down when the listener count drops to zero. Dropping the + // listeners with the global skips that, so the next file inherits a caught signal with no + // JS target. Remove them through the emitter so onDidChangeListeners restores SIG_DFL and + // releases the memoryPressure and IPC hooks, exactly as process.off() would have. + if (globalObject->hasProcessObject()) + globalObject->processObject()->wrapped().removeAllListeners(); globalObject->scriptExecutionContext()->prepareForDestruction(); } diff --git a/src/runtime/shell/subproc.rs b/src/runtime/shell/subproc.rs index a7a6afd18b58..4dc6bdbedb01 100644 --- a/src/runtime/shell/subproc.rs +++ b/src/runtime/shell/subproc.rs @@ -1,4 +1,5 @@ use core::ffi::{c_char, c_void}; +use core::ptr::NonNull; use std::sync::Arc; #[cfg(unix)] @@ -193,6 +194,8 @@ pub struct ShellSubprocess { pub closed: EnumSet, + event_loop: EventLoopHandle, + ctrl_c_child: Option, } @@ -234,9 +237,11 @@ impl JscSubprocess::static_pipe_writer::StaticPipeWriterProcess for ShellSubproc bun_spawn::link_impl_ProcessExit! { Shell for ShellSubprocess => |this| { - // Forwarded raw, not autoref'd: the callee may free `*this`. - on_process_exit(_process, status, _rusage) => - ShellSubprocess::on_process_exit(this, &status), + // Forwarded raw, not autoref'd: the callee may free `*this`. `process` + // stays raw too so `VirtualMachine::on_subprocess_exit` gets its + // mutable provenance. + on_process_exit(process, status, _rusage) => + ShellSubprocess::on_process_exit(this, process, &status), } } @@ -787,6 +792,7 @@ impl ShellSubprocess { stderr, cmd_parent, closed: EnumSet::empty(), + event_loop, ctrl_c_child, }); } @@ -843,7 +849,10 @@ impl ShellSubprocess { // SAFETY: scoped access; `watch` does not re-enter the subprocess. match unsafe { (*subprocess).proc().watch() } { - bun_sys::Result::Ok(()) => {} + bun_sys::Result::Ok(()) => { + // SAFETY: `subprocess` is live; scoped access. + unsafe { (*subprocess).register_with_auto_killer() }; + } bun_sys::Result::Err(_) => { *notify_caller_process_already_exited = true; spawn_args.lazy = false; @@ -903,11 +912,38 @@ impl ShellSubprocess { Ok(()) } + /// The owning VM when the shell runs on a JS event loop, `None` on a mini + /// event loop (`bun run` scripts, lifecycle scripts). + fn js_vm(&self) -> Option> { + NonNull::new(self.event_loop.bun_vm().cast()) + } + + /// `bun test` kills the subprocesses that a timed-out test or a finished + /// `--isolate` file left running. This puts a shell command on that list, + /// next to `Bun.spawn` children; `on_process_exit` takes it off again. + fn register_with_auto_killer(&self) { + let Some(vm) = self.js_vm() else { return }; + let Some(handle) = self.process.as_ref() else { return }; + if handle.has_exited() { + return; + } + let Some(process) = NonNull::new(handle.as_ptr()) else { return }; + // SAFETY: `vm` is the live VM that owns `event_loop`; JS thread only. + unsafe { (*vm.as_ptr()).on_subprocess_spawn(process) }; + } + /// # Safety /// `this` must be live and unborrowed; the Yield run here may free it. - unsafe fn on_process_exit(this: *mut Self, status: &Status) { + /// `process` is the live `*mut Process` threaded from the + /// `link_impl_ProcessExit!` vtable thunk. + unsafe fn on_process_exit(this: *mut Self, process: *mut Process, status: &Status) { log!("onProcessExit({:x})", this as usize); // SAFETY: caller contract; the borrow ends at the `;`. + if let (Some(vm), Some(process)) = (unsafe { (*this).js_vm() }, NonNull::new(process)) { + // SAFETY: `vm` is the live VM that owns `event_loop`; JS thread only. + unsafe { (*vm.as_ptr()).on_subprocess_exit(process) }; + } + // SAFETY: caller contract; the borrow ends at the `;`. let interrupted = unsafe { (*this).ctrl_c_child.take() }.is_some() && bun_spawn::ctrl_c::child_died_of_it(status); let exit_code: Option = 'brk: { diff --git a/test/cli/test/isolation.test.ts b/test/cli/test/isolation.test.ts index 7678f5d99b7c..ddecf09a6d52 100644 --- a/test/cli/test/isolation.test.ts +++ b/test/cli/test/isolation.test.ts @@ -1,5 +1,5 @@ import { describe, expect, setDefaultTimeout, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, normalizeBunSnapshot, tempDir, tls } from "harness"; +import { bunEnv, bunExe, isASAN, isWindows, normalizeBunSnapshot, tempDir, tls } from "harness"; import fs from "node:fs"; import net from "node:net"; import { join } from "node:path"; @@ -695,6 +695,99 @@ describe.concurrent("bun test --isolate", () => { } } }); + + test("with --isolate, a leaked Bun.$ subprocess is killed before the next file", async () => { + using dir = tempDir("isolate-shell-subprocess", { + "sleeper.js": ` + const fs = require("fs"); + fs.writeFileSync(process.env.PID_FILE + ".tmp", String(process.pid)); + fs.renameSync(process.env.PID_FILE + ".tmp", process.env.PID_FILE); + setInterval(() => {}, 1e6); + `, + "a-shell.test.ts": ` + import { test, expect } from "bun:test"; + import { $ } from "bun"; + import fs from "node:fs"; + test("leak a sleeper through Bun.$", async () => { + // Not awaited: the shell, and the sleeper under it, are still running when this file ends. + $\`\${process.execPath} sleeper.js\`.quiet().then(() => {}, () => {}); + while (!fs.existsSync(process.env.PID_FILE!)) await Bun.sleep(10); + expect(Number(fs.readFileSync(process.env.PID_FILE!, "utf8"))).toBeGreaterThan(0); + }); + `, + "b-check.test.ts": ` + import { test, expect } from "bun:test"; + import fs from "node:fs"; + const isAlive = (pid: number) => { try { process.kill(pid, 0); return true; } catch { return false; } }; + test("the previous file's Bun.$ sleeper was killed by isolation", async () => { + const pid = Number(fs.readFileSync(process.env.PID_FILE!, "utf8")); + expect(pid).toBeGreaterThan(0); + // The swap sends SIGTERM; the sleeper may still be getting reaped. + const deadline = Date.now() + 3_000; + while (isAlive(pid) && Date.now() < deadline) await Bun.sleep(10); + expect(isAlive(pid)).toBe(false); + }); + `, + }); + const pidFile = join(String(dir), "pid.txt"); + try { + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--isolate", "./a-shell.test.ts", "./b-check.test.ts"], + env: { ...bunEnv, PID_FILE: pidFile }, + cwd: String(dir), + stderr: "pipe", + stdout: "pipe", + }); + const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(normalizeBunSnapshot(stderr, dir)).toContain("2 pass"); + expect(normalizeBunSnapshot(stderr, dir)).toContain("0 fail"); + expect(exitCode).toBe(0); + } finally { + try { + process.kill(Number(fs.readFileSync(pidFile, "utf8"))); + } catch {} + } + }); + + // process.on() installs one process-wide handler. The listener goes + // away with the finished file's global, so the handler has to go too, or the + // next file inherits a caught signal that no JS code observes. + test.skipIf(isWindows)( + "with --isolate, a leaked process.on('SIGTERM') listener does not leave SIGTERM caught for the next file", + async () => { + using dir = tempDir("isolate-signal-listener", { + "a-signal.test.ts": ` + import { test, expect } from "bun:test"; + test("leak a SIGTERM listener", () => { + process.on("SIGTERM", () => console.error("[A listener] got SIGTERM")); + expect(process.listenerCount("SIGTERM")).toBe(1); + }); + `, + "b-signal.test.ts": ` + import { test, expect } from "bun:test"; + test("SIGTERM with no listener ends the run", () => { + console.error("[B] listenerCount=" + process.listenerCount("SIGTERM")); + // Default disposition: the kernel terminates the process inside this call. + process.kill(process.pid, "SIGTERM"); + console.error("[B] survived SIGTERM"); + expect.unreachable(); + }); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--isolate", "./a-signal.test.ts", "./b-signal.test.ts"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + stdout: "pipe", + }); + const [, stderr] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toContain("[B] listenerCount=0"); + expect(stderr).not.toContain("[A listener] got SIGTERM"); + expect(stderr).not.toContain("[B] survived SIGTERM"); + expect(proc.signalCode).toBe("SIGTERM"); + }, + ); }); // --isolate raises JSC's FTL warm-up threshold. JSC options are set once per From 0905e681050e3a46c8f85957060aaf853d9dc799 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 6 Sep 2026 21:40:56 +0000 Subject: [PATCH 2/5] docs: Bun.$ children are covered by the test-timeout process killer --- docs/test/writing-tests.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/test/writing-tests.mdx b/docs/test/writing-tests.mdx index 02e117455330..2b9f601c4989 100644 --- a/docs/test/writing-tests.mdx +++ b/docs/test/writing-tests.mdx @@ -117,7 +117,7 @@ test( ### 🧟 Zombie Process Killer -When a test times out, Bun kills any still-running processes that the test spawned with `Bun.spawn`, `Bun.spawnSync`, or `node:child_process`, and logs a message to the console. This prevents zombie processes from lingering after timed-out tests. +When a test times out, Bun kills any still-running processes that the test spawned with `Bun.spawn`, `Bun.spawnSync`, `Bun.$`, or `node:child_process`, and logs a message to the console. This prevents zombie processes from lingering after timed-out tests. ## Test Modifiers From 6453c6cd9b8bbebf204d3cbc78df67ba07baec59 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 00:49:03 +0000 Subject: [PATCH 3/5] [autofix.ci] apply automated fixes --- src/runtime/shell/subproc.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/runtime/shell/subproc.rs b/src/runtime/shell/subproc.rs index 4dc6bdbedb01..292a1c07299d 100644 --- a/src/runtime/shell/subproc.rs +++ b/src/runtime/shell/subproc.rs @@ -923,11 +923,15 @@ impl ShellSubprocess { /// next to `Bun.spawn` children; `on_process_exit` takes it off again. fn register_with_auto_killer(&self) { let Some(vm) = self.js_vm() else { return }; - let Some(handle) = self.process.as_ref() else { return }; + let Some(handle) = self.process.as_ref() else { + return; + }; if handle.has_exited() { return; } - let Some(process) = NonNull::new(handle.as_ptr()) else { return }; + let Some(process) = NonNull::new(handle.as_ptr()) else { + return; + }; // SAFETY: `vm` is the live VM that owns `event_loop`; JS thread only. unsafe { (*vm.as_ptr()).on_subprocess_spawn(process) }; } From 40f8e6b86afc058c9e597375ba6b866d692ef8cf Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 8 Sep 2026 02:08:20 +0000 Subject: [PATCH 4/5] shell: register with the auto-killer after the last fallible start; trim comments A stdin or pipe-reader start failure goes through abort_after_failed_start, which drops the exit handler, so an entry registered before that point would stay in the auto-killer until the next kill/clear. Register at the end instead, like Bun.spawn does. Also use an ESM fixture in the test. --- src/jsc/bindings/ZigGlobalObject.cpp | 7 ++----- src/runtime/shell/subproc.rs | 28 ++++++++++++++-------------- test/cli/test/isolation.test.ts | 6 +++--- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index 69d8eadfc498..a2dfc887d4d8 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -4377,11 +4377,8 @@ extern "C" void Zig__GlobalObject__forbidExecution(Zig::GlobalObject* globalObje extern "C" void Zig__GlobalObject__stopActiveDOMObjectsForTestIsolation(Zig::GlobalObject* globalObject) { Bun::retireWebViewsForTestIsolation(globalObject); - // A `process.on()` listener installs a process-wide sigaction (a uv_signal_t on - // Windows) that is only torn down when the listener count drops to zero. Dropping the - // listeners with the global skips that, so the next file inherits a caught signal with no - // JS target. Remove them through the emitter so onDidChangeListeners restores SIG_DFL and - // releases the memoryPressure and IPC hooks, exactly as process.off() would have. + // Through the emitter, so onDidChangeListeners restores SIG_DFL for process.on() + // listeners and drops the memoryPressure/IPC hooks, as the last process.off() would. if (globalObject->hasProcessObject()) globalObject->processObject()->wrapped().removeAllListeners(); globalObject->scriptExecutionContext()->prepareForDestruction(); diff --git a/src/runtime/shell/subproc.rs b/src/runtime/shell/subproc.rs index 292a1c07299d..619afb20d033 100644 --- a/src/runtime/shell/subproc.rs +++ b/src/runtime/shell/subproc.rs @@ -237,9 +237,7 @@ impl JscSubprocess::static_pipe_writer::StaticPipeWriterProcess for ShellSubproc bun_spawn::link_impl_ProcessExit! { Shell for ShellSubprocess => |this| { - // Forwarded raw, not autoref'd: the callee may free `*this`. `process` - // stays raw too so `VirtualMachine::on_subprocess_exit` gets its - // mutable provenance. + // Both forwarded raw, not autoref'd: the callee may free `*this`. on_process_exit(process, status, _rusage) => ShellSubprocess::on_process_exit(this, process, &status), } @@ -849,10 +847,7 @@ impl ShellSubprocess { // SAFETY: scoped access; `watch` does not re-enter the subprocess. match unsafe { (*subprocess).proc().watch() } { - bun_sys::Result::Ok(()) => { - // SAFETY: `subprocess` is live; scoped access. - unsafe { (*subprocess).register_with_auto_killer() }; - } + bun_sys::Result::Ok(()) => {} bun_sys::Result::Err(_) => { *notify_caller_process_already_exited = true; spawn_args.lazy = false; @@ -907,20 +902,26 @@ impl ShellSubprocess { return Err(ShellErr::Sys(sys_err)); } + // After the last fallible start, so `abort_after_failed_start` (which + // drops the exit handler) never leaves a registered entry behind. + if !*notify_caller_process_already_exited { + // SAFETY: `subprocess` is live; the exit handler only runs from the + // event loop, after this returns. + unsafe { (*subprocess).register_with_auto_killer() }; + } + log!("returning"); Ok(()) } - /// The owning VM when the shell runs on a JS event loop, `None` on a mini - /// event loop (`bun run` scripts, lifecycle scripts). + /// `None` on a mini event loop (`bun run` scripts, lifecycle scripts). fn js_vm(&self) -> Option> { NonNull::new(self.event_loop.bun_vm().cast()) } - /// `bun test` kills the subprocesses that a timed-out test or a finished - /// `--isolate` file left running. This puts a shell command on that list, - /// next to `Bun.spawn` children; `on_process_exit` takes it off again. + /// Lets `bun test` kill this child on a test timeout or `--isolate` swap, + /// like a `Bun.spawn` child. `on_process_exit` unregisters it. fn register_with_auto_killer(&self) { let Some(vm) = self.js_vm() else { return }; let Some(handle) = self.process.as_ref() else { @@ -938,8 +939,7 @@ impl ShellSubprocess { /// # Safety /// `this` must be live and unborrowed; the Yield run here may free it. - /// `process` is the live `*mut Process` threaded from the - /// `link_impl_ProcessExit!` vtable thunk. + /// `process` is the live `*mut Process` from the exit-handler thunk. unsafe fn on_process_exit(this: *mut Self, process: *mut Process, status: &Status) { log!("onProcessExit({:x})", this as usize); // SAFETY: caller contract; the borrow ends at the `;`. diff --git a/test/cli/test/isolation.test.ts b/test/cli/test/isolation.test.ts index ddecf09a6d52..a3f47fe28242 100644 --- a/test/cli/test/isolation.test.ts +++ b/test/cli/test/isolation.test.ts @@ -698,8 +698,8 @@ describe.concurrent("bun test --isolate", () => { test("with --isolate, a leaked Bun.$ subprocess is killed before the next file", async () => { using dir = tempDir("isolate-shell-subprocess", { - "sleeper.js": ` - const fs = require("fs"); + "sleeper.mjs": ` + import fs from "node:fs"; fs.writeFileSync(process.env.PID_FILE + ".tmp", String(process.pid)); fs.renameSync(process.env.PID_FILE + ".tmp", process.env.PID_FILE); setInterval(() => {}, 1e6); @@ -710,7 +710,7 @@ describe.concurrent("bun test --isolate", () => { import fs from "node:fs"; test("leak a sleeper through Bun.$", async () => { // Not awaited: the shell, and the sleeper under it, are still running when this file ends. - $\`\${process.execPath} sleeper.js\`.quiet().then(() => {}, () => {}); + $\`\${process.execPath} sleeper.mjs\`.quiet().then(() => {}, () => {}); while (!fs.existsSync(process.env.PID_FILE!)) await Bun.sleep(10); expect(Number(fs.readFileSync(process.env.PID_FILE!, "utf8"))).toBeGreaterThan(0); }); From 2f552252aefded492dcaae48d2373c5d08928500 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 8 Sep 2026 02:30:55 +0000 Subject: [PATCH 5/5] trim comments --- src/jsc/bindings/ZigGlobalObject.cpp | 3 +-- src/runtime/shell/subproc.rs | 9 +++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index a2dfc887d4d8..04ad80d842ac 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -4377,8 +4377,7 @@ extern "C" void Zig__GlobalObject__forbidExecution(Zig::GlobalObject* globalObje extern "C" void Zig__GlobalObject__stopActiveDOMObjectsForTestIsolation(Zig::GlobalObject* globalObject) { Bun::retireWebViewsForTestIsolation(globalObject); - // Through the emitter, so onDidChangeListeners restores SIG_DFL for process.on() - // listeners and drops the memoryPressure/IPC hooks, as the last process.off() would. + // onDidChangeListeners restores SIG_DFL for the file's process.on() listeners. if (globalObject->hasProcessObject()) globalObject->processObject()->wrapped().removeAllListeners(); globalObject->scriptExecutionContext()->prepareForDestruction(); diff --git a/src/runtime/shell/subproc.rs b/src/runtime/shell/subproc.rs index 619afb20d033..6a49f565b525 100644 --- a/src/runtime/shell/subproc.rs +++ b/src/runtime/shell/subproc.rs @@ -902,11 +902,9 @@ impl ShellSubprocess { return Err(ShellErr::Sys(sys_err)); } - // After the last fallible start, so `abort_after_failed_start` (which - // drops the exit handler) never leaves a registered entry behind. + // Last, so a failed start (which drops the exit handler) never leaves an entry behind. if !*notify_caller_process_already_exited { - // SAFETY: `subprocess` is live; the exit handler only runs from the - // event loop, after this returns. + // SAFETY: `subprocess` is live; its exit handler only runs from the event loop. unsafe { (*subprocess).register_with_auto_killer() }; } @@ -920,8 +918,7 @@ impl ShellSubprocess { NonNull::new(self.event_loop.bun_vm().cast()) } - /// Lets `bun test` kill this child on a test timeout or `--isolate` swap, - /// like a `Bun.spawn` child. `on_process_exit` unregisters it. + /// Lets `bun test` kill this child on a timeout or `--isolate` swap, like a `Bun.spawn` child. fn register_with_auto_killer(&self) { let Some(vm) = self.js_vm() else { return }; let Some(handle) = self.process.as_ref() else {