bun test: only kill the timed-out test's own subprocesses on timeout

src/jsc/ProcessAutoKiller.rs

@@ -9,9 +9,15 @@ bun_core::declare_scope!(AutoKiller, hidden);
 pub struct ProcessAutoKiller {
     /// Keys are intrusively-refcounted `*Process` (ref()'d on insert, deref()'d
     /// on remove/drop). Stored as raw ptr for identity-hash semantics.
-    pub(crate) processes: ArrayHashMap<*mut Process, ()>,
+    /// The value is the [`Self::scope`] that was current when the process was
+    /// spawned.
+    pub(crate) processes: ArrayHashMap<*mut Process, u32>,
     pub enabled: bool,
     pub(crate) ever_enabled: bool,
+    /// Advanced by [`Self::begin_scope`]. The test runner starts a scope per
+    /// test (or hook) so that [`Self::kill_scope`] on a timeout leaves
+    /// processes spawned by earlier tests and `beforeAll` hooks alone.
+    scope: u32,
 }
 
 impl ProcessAutoKiller {
@@ -24,34 +30,64 @@ impl ProcessAutoKiller {
         self.enabled = false;
     }
 
+    /// Attributes every process spawned from now on to a new scope.
+    /// Processes already tracked keep the scope they were spawned in.
+    pub fn begin_scope(&mut self) {
+        self.scope = self.scope.wrapping_add(1);
+    }
+
+    /// Kills every tracked process and stops tracking all of them.
     pub fn kill(&mut self) -> Result {
-        Result {
-            processes: self.kill_processes(),
+        let mut count: u32 = 0;
+        while let Some(entry) = self.processes.pop() {
+            count += Self::kill_and_release(entry.key);
         }
+        Result { processes: count }
     }
 
-    fn kill_processes(&mut self) -> u32 {
+    /// Kills only the processes spawned since the last [`Self::begin_scope`]
+    /// and stops tracking them. Processes from earlier scopes are left running
+    /// and stay tracked, so a later [`Self::kill`] still covers them.
+    pub fn kill_scope(&mut self) -> Result {
         let mut count: u32 = 0;
-        while let Some(entry) = self.processes.pop() {
-            {
-                // SAFETY: every key in `processes` was ref()'d on insert and is
-                // live until the matching deref() below; popped entry is
-                // exclusively owned for this scope so `&mut Process` is unaliased.
-                let p: &mut Process = unsafe { &mut *entry.key };
-                if !p.has_exited() {
-                    bun_core::scoped_log!(AutoKiller, "process.kill {}", p.pid);
-                    count += p.kill(SignalCode::DEFAULT.0).is_ok() as u32;
-                }
+        let mut index = self.processes.len();
+        while index > 0 {
+            index -= 1;
+            if self.processes.values()[index] != self.scope {
+                continue;
             }
-            // SAFETY: key live until this releases the ref taken on insert.
-            unsafe { Process::deref(entry.key) };
+            // Walking backwards, so the entry swapped into `index` has
+            // already been visited.
+            let (process, _) = self.processes.swap_remove_at(index);
+            count += Self::kill_and_release(process);
         }
-        count
+        Result { processes: count }
+    }
+
+    /// Signals `process` if it is still running, then releases the ref taken in
+    /// [`Self::on_subprocess_spawn`]. The caller must already have removed it
+    /// from `processes`. Returns 1 if a signal was sent.
+    fn kill_and_release(process: *mut Process) -> u32 {
+        let killed = {
+            // SAFETY: every key in `processes` was ref()'d on insert and is
+            // live until the matching deref() below; the entry was removed
+            // from the map by the caller, so `&mut Process` is unaliased.
+            let p: &mut Process = unsafe { &mut *process };
+            if p.has_exited() {
+                false
+            } else {
+                bun_core::scoped_log!(AutoKiller, "process.kill {}", p.pid);
+                p.kill(SignalCode::DEFAULT.0).is_ok()
+            }
+        };
+        // SAFETY: key live until this releases the ref taken on insert.
+        unsafe { Process::deref(process) };
+        killed as u32
     }
 
     pub fn clear(&mut self) {
         for process in self.processes.keys() {
-            // SAFETY: see kill_processes — key is live until deref().
+            // SAFETY: see kill_and_release — key is live until deref().
             unsafe { Process::deref(*process) };
         }
 
@@ -69,7 +105,7 @@ impl ProcessAutoKiller {
         if self.enabled {
             // Alloc failure means we never took
             // a ref, so just bail. `put` here is fallible only on OOM.
-            if self.processes.put(process.as_ptr(), ()).is_err() {
+            if self.processes.put(process.as_ptr(), self.scope).is_err() {
                 return;
             }
             // SAFETY: caller passes a live Process; we take a ref to extend its
@@ -99,7 +135,7 @@ pub struct Result {
 impl Drop for ProcessAutoKiller {
     fn drop(&mut self) {
         for process in self.processes.keys() {
-            // SAFETY: see kill_processes — key is live until deref().
+            // SAFETY: see kill_and_release — key is live until deref().
             unsafe { Process::deref(*process) };
         }
         // `self.processes` storage freed by its own Drop.

src/runtime/test_runner/Execution.rs

@@ -299,8 +299,11 @@ impl Execution {
         let _g = group_begin!();
 
         // if the concurrent group has one sequence and the sequence has an active entry that has timed out,
-        //   kill any dangling processes
+        //   kill the dangling processes spawned by that group (each group is its own auto_killer scope, so
+        //   processes spawned by beforeAll or by earlier tests are left alone)
         // when using test.concurrent(), we can't do this because it could kill multiple tests at once.
+        // timespec == EPOCH means the entry has no timeout; the timer that fired was armed by an earlier
+        // entry (update_min_timeout never unsets a timer), same as in ExecutionEntry::evaluate_timeout.
         if let Some(current_group) = self.active_group() {
             // reshaped for borrowck — capture range, drop &mut group, re-borrow sequences
             let (start, end) = (current_group.sequence_start, current_group.sequence_end);
@@ -311,9 +314,11 @@ impl Execution {
                     // SAFETY: arena-owned entry, alive for lifetime of BunTest
                     let entry = unsafe { entry.as_ref() };
                     let now = Timespec::now_force_real_time();
-                    if entry.timespec.order(&now) == core::cmp::Ordering::Less {
+                    if !entry.timespec.eql(&Timespec::EPOCH)
+                        && entry.timespec.order(&now) == core::cmp::Ordering::Less
+                    {
                         // SAFETY: bun_vm() returns the live per-thread VM.
-                        let kill_count = global_this.bun_vm().as_mut().auto_killer.kill();
+                        let kill_count = global_this.bun_vm().as_mut().auto_killer.kill_scope();
                         if kill_count.processes > 0 {
                             bun_core::pretty_errorln!(
                                 "<d>killed {} dangling process{}<r>",
@@ -565,7 +570,9 @@ impl Execution {
 
     fn on_group_started(global_this: &JSGlobalObject) {
         // SAFETY: bun_vm() returns the live per-thread VM.
-        global_this.bun_vm().as_mut().auto_killer.enable();
+        let auto_killer = &mut global_this.bun_vm().as_mut().auto_killer;
+        auto_killer.begin_scope();
+        auto_killer.enable();
     }
 
     fn on_group_completed(global_this: &JSGlobalObject) {

test/cli/test/test-timeout-behavior.test.ts

@@ -1,5 +1,5 @@
 import { expect, test } from "bun:test";
-import { bunEnv, bunExe, isFlaky, isLinux } from "harness";
+import { bunEnv, bunExe, isFlaky, isLinux, tempDir } from "harness";
 import path from "path";
 
 if (isFlaky && isLinux) {
@@ -33,3 +33,118 @@ if (isFlaky && isLinux) {
     expect(combined).toContain("(pass) slow test after test timeout");
   });
 }
+
+// Shared by the inline test files below. An echo child stays alive until it is
+// killed, and `echo()` only gets its message back while the child is alive: a
+// killed child never runs again, so its stdout just reports EOF.
+const echoChildHelpers = /* ts */ `
+  function spawnEcho() {
+    return Bun.spawn({
+      cmd: [process.execPath, "-e", "process.stdin.pipe(process.stdout)"],
+      stdin: "pipe",
+      stdout: "pipe",
+      stderr: "ignore",
+    });
+  }
+
+  async function echo(child: ReturnType<typeof spawnEcho>, message: string) {
+    child.stdin.write(message);
+    await child.stdin.flush();
+    const reader = child.stdout.getReader();
+    let received = "";
+    while (received.length < message.length) {
+      const { value, done } = await reader.read();
+      if (done) break;
+      received += Buffer.from(value).toString();
+    }
+    reader.releaseLock();
+    return received;
+  }
+`;
+
+async function runTestFile(source: string, args: string[] = []) {
+  using dir = tempDir("test-timeout-kill", { "kill.test.ts": echoChildHelpers + source });
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "test", ...args, "./kill.test.ts"],
+    cwd: String(dir),
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  return { combined: stdout + stderr, exitCode };
+}
+
+test.concurrent.each([[[]], [["--isolate"]]])(
+  "a test timeout only kills the processes spawned by that test (args: %p)",
+  async args => {
+    const { combined, exitCode } = await runTestFile(
+      /* ts */ `
+        import { afterAll, beforeAll, expect, test } from "bun:test";
+
+        const children: ReturnType<typeof spawnEcho>[] = [];
+        afterAll(() => children.forEach(child => child.kill()));
+
+        beforeAll(() => {
+          children.push(spawnEcho());
+        });
+
+        test("spawns a child that outlives the test", () => {
+          children.push(spawnEcho());
+        });
+
+        test("times out", async () => {
+          children.push(spawnEcho());
+          await new Promise(() => {});
+        }, 100);
+
+        test("children spawned by beforeAll and by an earlier test are still running", async () => {
+          const [fromBeforeAll, fromEarlierTest] = children;
+          expect(await Promise.all([echo(fromBeforeAll, "beforeAll"), echo(fromEarlierTest, "earlier test")])).toEqual([
+            "beforeAll",
+            "earlier test",
+          ]);
+        });
+      `,
+      args,
+    );
+
+    // Only the child of the test that timed out is killed.
+    expect(combined).toContain("killed 1 dangling process");
+    expect(combined).not.toContain("dangling processes");
+    expect(combined).toContain("(pass) spawns a child that outlives the test");
+    expect(combined).toContain("(fail) times out");
+    expect(combined).toContain("(pass) children spawned by beforeAll and by an earlier test are still running");
+    expect(combined).toContain(" 2 pass\n");
+    expect(combined).toContain(" 1 fail\n");
+    expect(exitCode).toBe(1);
+  },
+);
+
+test.concurrent("a test without a timeout keeps its processes when an earlier test's timer fires", async () => {
+  const { combined, exitCode } = await runTestFile(/* ts */ `
+    import { expect, test } from "bun:test";
+
+    // Arms a 50ms timer for this test. The runner never disarms it, so it fires
+    // while the next test is running.
+    test("finishes immediately", () => {}, 50);
+
+    test("has no timeout and spawns a child", async () => {
+      const child = spawnEcho();
+      try {
+        // Both timers live in the same heap and fire in deadline order, so the
+        // first test's 50ms timer has fired by the time this sleep resolves.
+        await Bun.sleep(250);
+        expect(await echo(child, "still here")).toBe("still here");
+      } finally {
+        child.kill();
+      }
+    }, 0);
+  `);
+
+  expect(combined).not.toContain("dangling process");
+  expect(combined).toContain("(pass) finishes immediately");
+  expect(combined).toContain("(pass) has no timeout and spawns a child");
+  expect(combined).toContain(" 2 pass\n");
+  expect(exitCode).toBe(0);
+});