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

docs/test/writing-tests.mdx

@@ -74,7 +74,7 @@ test("wat", async () => {
 }, 500); // test must run in <500ms
 ```
 
-In `bun:test`, a timeout throws an uncatchable exception to force the test to stop running and fail. Bun also kills any child processes spawned in the test, so they don't linger as zombie processes.
+In `bun:test`, a timeout throws an uncatchable exception to force the test to stop running and fail. Bun also kills any child processes the test (or its `beforeEach`/`afterEach` hooks) spawned that are still running, so they don't linger as zombie processes. Processes started in `beforeAll` or by earlier tests are left alone.
 
 The default timeout for each test is 5000ms (5 seconds) unless you override it with this timeout option or `jest.setTimeout()`.
 
@@ -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 (or its `beforeEach`/`afterEach` hooks) 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. Processes spawned in `beforeAll` or by earlier tests are not affected; a `beforeAll` or `afterAll` hook that times out only has its own processes killed.
 
 ## Test Modifiers
 

src/jsc/ProcessAutoKiller.rs

@@ -9,9 +9,11 @@ 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, ()>,
+    pub(crate) processes: ArrayHashMap<*mut Process, u32>,
     pub enabled: bool,
     pub(crate) ever_enabled: bool,
+    /// Map value of each tracked process; the test runner begins a new one per execution group.
+    scope: u32,
 }
 
 impl ProcessAutoKiller {
@@ -24,34 +26,56 @@ impl ProcessAutoKiller {
         self.enabled = false;
     }
 
+    pub fn begin_scope(&mut self) {
+        self.scope = self.scope.wrapping_add(1);
+    }
+
     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 {
+    /// Earlier scopes stay tracked (and alive) so that [`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` was already visited.
+            let (process, _) = self.processes.swap_remove_at(index);
+            count += Self::kill_and_release(process);
         }
-        count
+        Result { processes: count }
+    }
+
+    /// `process` must already be removed from `processes`; this releases its ref.
+    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 +93,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 +123,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,7 @@ 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
-        // when using test.concurrent(), we can't do this because it could kill multiple tests at once.
+        //   kill the dangling processes it spawned
         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,17 +310,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 {
-                        // SAFETY: bun_vm() returns the live per-thread VM.
-                        let kill_count = global_this.bun_vm().as_mut().auto_killer.kill();
-                        if kill_count.processes > 0 {
-                            bun_core::pretty_errorln!(
-                                "<d>killed {} dangling process{}<r>",
-                                kill_count.processes,
-                                if kill_count.processes != 1 { "es" } else { "" },
-                            );
-                            bun_core::Output::flush();
-                        }
+                    // EPOCH: this entry has no timeout; a timer left armed by an earlier entry fired.
+                    if !entry.timespec.eql(&Timespec::EPOCH)
+                        && entry.timespec.order(&now) == core::cmp::Ordering::Less
+                    {
+                        kill_dangling_processes(end - start, global_this);
                     }
                 }
             }
@@ -565,7 +558,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) {
@@ -1037,7 +1032,12 @@ fn step_sequence_one(
             // SAFETY: re-deref after run_test_callback; sequence_ptr still valid (sequences is a
             // Box<[ExecutionSequence]>, never reallocated during execution).
             let sequence = unsafe { &mut *sequence_ptr.as_ptr() };
-            let _ = next_item.evaluate_timeout(sequence, now);
+            if next_item.evaluate_timeout(sequence, now) {
+                // The callback overran its deadline synchronously, so handle_timeout may never have run.
+                // SAFETY: group points into buntest.execution.groups; read-only.
+                let g = unsafe { group.as_ref() };
+                kill_dangling_processes(g.sequence_end - g.sequence_start, global_this);
+            }
 
             // the result is available immediately; advance the sequence and run again.
             Execution::advance_sequence(buntest_ptr, sequence_ptr, group);
@@ -1078,3 +1078,19 @@ fn step_sequence_one(
         return Ok(None); // run again
     }
 }
+
+/// Skipped for test.concurrent() groups: their tests share one scope, so this would hit other tests' children.
+fn kill_dangling_processes(group_sequence_count: usize, global_this: &JSGlobalObject) {
+    if group_sequence_count != 1 {
+        return;
+    }
+    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>",
+            kill_count.processes,
+            if kill_count.processes != 1 { "es" } else { "" },
+        );
+        bun_core::Output::flush();
+    }
+}