test --isolate: restore signal dispositions and kill Bun.$ children at the file swap

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
 

src/jsc/bindings/ZigGlobalObject.cpp

@@ -4377,6 +4377,9 @@ extern "C" void Zig__GlobalObject__forbidExecution(Zig::GlobalObject* globalObje
 extern "C" void Zig__GlobalObject__stopActiveDOMObjectsForTestIsolation(Zig::GlobalObject* globalObject)
 {
     Bun::retireWebViewsForTestIsolation(globalObject);
+    // onDidChangeListeners restores SIG_DFL for the file's process.on(<signal>) listeners.
+    if (globalObject->hasProcessObject())
+        globalObject->processObject()->wrapped().removeAllListeners();
     globalObject->scriptExecutionContext()->prepareForDestruction();
 }
 

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<StdioKind>,
 
+    event_loop: EventLoopHandle,
+
     ctrl_c_child: Option<bun_spawn::ctrl_c::Child>,
 }
 
@@ -234,9 +237,9 @@ 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),
+        // Both forwarded raw, not autoref'd: the callee may free `*this`.
+        on_process_exit(process, status, _rusage) =>
+            ShellSubprocess::on_process_exit(this, process, &status),
     }
 }
 
@@ -787,6 +790,7 @@ impl ShellSubprocess {
                 stderr,
                 cmd_parent,
                 closed: EnumSet::empty(),
+                event_loop,
                 ctrl_c_child,
             });
         }
@@ -898,16 +902,49 @@ impl ShellSubprocess {
             return Err(ShellErr::Sys(sys_err));
         }
 
+        // 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; its exit handler only runs from the event loop.
+            unsafe { (*subprocess).register_with_auto_killer() };
+        }
+
         log!("returning");
 
         Ok(())
     }
 
+    /// `None` on a mini event loop (`bun run` scripts, lifecycle scripts).
+    fn js_vm(&self) -> Option<NonNull<jsc::virtual_machine::VirtualMachine>> {
+        NonNull::new(self.event_loop.bun_vm().cast())
+    }
+
+    /// 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 {
+            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` 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 `;`.
+        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<u8> = 'brk: {

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.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);
+      `,
+      "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.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);
+        });
+      `,
+      "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(<signal>) 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