From 1fbbf998d771a18063b8ea0e73c0cc0ae0b5a6fa Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 15 Aug 2026 00:30:31 -0700 Subject: [PATCH 1/2] spawnSync: don't run the bun:test timeout callback while the isolated loop is active MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the per-test deadline passed while Bun.spawnSync was blocking, the sync wait loop invoked BunTest::bun_test_timeout_callback in place — re-entering the whole test runner (result queue, junit, worker IPC, timer heap) while vm.event_loop_handle still pointed at the isolated loop and before the child had been reaped. Under bun test --parallel that left the worker spinning with an unreaped zombie child and the coordinator waiting on the file until the runner's idle timeout killed the batch. Kill the dangling processes there (Execution::handle_timeout, so the child dies and the wait drains) and hand the deadline back to spawn_sync, which fires the runner's callback once spawn_maybe_sync has torn the isolated loop down. If an exception is pending by then the file timer is left armed and reports the timeout from the main loop; an exception raised by the callback is propagated. --- src/runtime/api/bun/js_bun_spawn_bindings.rs | 77 +++++++++++--------- 1 file changed, 43 insertions(+), 34 deletions(-) diff --git a/src/runtime/api/bun/js_bun_spawn_bindings.rs b/src/runtime/api/bun/js_bun_spawn_bindings.rs index 70910ff9dbf3..1d48a359b580 100644 --- a/src/runtime/api/bun/js_bun_spawn_bindings.rs +++ b/src/runtime/api/bun/js_bun_spawn_bindings.rs @@ -289,7 +289,7 @@ pub(crate) fn spawn( args: JSValue, secondary_args_value: Option, ) -> JsResult { - spawn_maybe_sync::(global_this, args, secondary_args_value) + spawn_maybe_sync::(global_this, args, secondary_args_value, &mut None) } /// Bun.spawnSync() calls this. @@ -298,13 +298,39 @@ pub(crate) fn spawn_sync( args: JSValue, secondary_args_value: Option, ) -> JsResult { - spawn_maybe_sync::(global_this, args, secondary_args_value) + let mut bun_test_deadline: Option = None; + let result = spawn_maybe_sync::( + global_this, + args, + secondary_args_value, + &mut bun_test_deadline, + ); + // A bun:test deadline that passed while the isolated loop was blocking is reported only now: the loop is torn down and the child reaped, so the runner's callback re-enters nothing that is mid-flight. With an exception pending (spawn failure, termination) the file timer is left armed and reports it from the main loop instead. + if let Some(deadline) = bun_test_deadline + && result.is_ok() + && !global_this.has_exception() + && let Some(runner) = crate::test_runner::jest::Jest::runner() + && let Some(active_file) = runner.bun_test_root.active_file.clone() + { + let vm = global_this.bun_vm().as_mut(); + runner.remove_active_timeout(vm); + crate::test_runner::bun_test::BunTest::bun_test_timeout_callback( + &active_file, + &deadline, + vm, + ); + if global_this.has_exception() { + return Ok(JSValue::ZERO); + } + } + result } fn spawn_maybe_sync( global_this: &JSGlobalObject, args_: JSValue, secondary_args_value: Option, + bun_test_deadline: &mut Option, ) -> JsResult { if IS_SYNC { // We skip this on Windows due to test failures. @@ -1977,39 +2003,22 @@ fn spawn_maybe_sync( // Support bun:test timeouts AND spawnSync() timeout. // There is a scenario where inside of spawnSync() a totally // different test fails, and that SHOULD be okay. - if has_bun_test_timeout { - if bun_test_timeout.order(&now) == core::cmp::Ordering::Less { - bun_test_fired = true; - let mut active_file_strong = crate::test_runner::jest::Jest::runner() - .unwrap() - .bun_test_root - .active_file - // TODO: add a .cloneNonOptional()? - .clone(); - - let taken_active_file = active_file_strong.take().unwrap(); - - // SAFETY: jsc_vm_ptr is the live thread VM. - crate::test_runner::jest::Jest::runner() - .unwrap() - .remove_active_timeout(unsafe { &mut *jsc_vm_ptr }); - - // This might internally call `kill(2)` on this - // spawnSync process. Even if we do that, we still - // need to reap the process. So we may go through - // the event loop again, but it should wake up - // ~instantly so we can drain the events. - crate::test_runner::bun_test::BunTest::bun_test_timeout_callback( - &taken_active_file, - &absolute_timespec, - // SAFETY: jsc_vm_ptr is the live thread VM. - unsafe { &*jsc_vm_ptr }, - ); - // The direct child may already be reaped (and - // gone from the auto-killer), so kill it here too. - let _ = subprocess.try_kill(subprocess.kill_signal); - // active_file_strong / taken_active_file drop here (was `defer .deinit()`). + // Kill the dangling processes now so this loop can drain, but leave the runner's timeout callback to `spawn_sync`: it re-enters the test runner and must not run while this isolated loop is still active. + if has_bun_test_timeout + && bun_test_timeout.order(&now) == core::cmp::Ordering::Less + { + bun_test_fired = true; + *bun_test_deadline = Some(absolute_timespec); + if let Some(active_file) = crate::test_runner::jest::Jest::runner() + .unwrap() + .bun_test_root + .active_file + .clone() + { + // `Err` means the exception is pending on `global_this`; the check after this loop propagates it. + let _ = active_file.get().execution.handle_timeout(global_this); } + let _ = subprocess.try_kill(subprocess.kill_signal); } } } From 0ad1d1e12c81d9cdf03dec292cac9ff072ccd0ac Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 15 Aug 2026 13:12:30 -0700 Subject: [PATCH 2/2] spawnSync: reap dangling processes without enqueuing Start from inside the isolated loop handle_timeout() also does add_result(Start); calling it in the wait loop and again from the deferred timeout callback queued Start twice and wrote to the runner's queue while the isolated loop was live. Split out the kill-only half and call that in the loop. --- src/runtime/api/bun/js_bun_spawn_bindings.rs | 6 ++++-- src/runtime/test_runner/Execution.rs | 13 ++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/runtime/api/bun/js_bun_spawn_bindings.rs b/src/runtime/api/bun/js_bun_spawn_bindings.rs index 1d48a359b580..8c6e65c3868c 100644 --- a/src/runtime/api/bun/js_bun_spawn_bindings.rs +++ b/src/runtime/api/bun/js_bun_spawn_bindings.rs @@ -2015,8 +2015,10 @@ fn spawn_maybe_sync( .active_file .clone() { - // `Err` means the exception is pending on `global_this`; the check after this loop propagates it. - let _ = active_file.get().execution.handle_timeout(global_this); + active_file + .get() + .execution + .kill_dangling_processes_on_timeout(global_this); } let _ = subprocess.try_kill(subprocess.kill_signal); } diff --git a/src/runtime/test_runner/Execution.rs b/src/runtime/test_runner/Execution.rs index a864c8f8b17b..66c7a5da086e 100644 --- a/src/runtime/test_runner/Execution.rs +++ b/src/runtime/test_runner/Execution.rs @@ -297,7 +297,15 @@ impl Execution { pub(crate) fn handle_timeout(&mut self, global_this: &JSGlobalObject) -> JsResult<()> { let _g = group_begin!(); + self.kill_dangling_processes_on_timeout(global_this); + let buntest = self.bun_test(); + // SAFETY: deref parent at point-of-use; `self` is not accessed while this `&mut BunTest` is live. + unsafe { (*buntest).add_result(RefDataValue::Start) }; + Ok(()) + } + /// The kill-only half of [`handle_timeout`]: reaps a timed-out test's spawned processes without touching the runner's queue, so it may run from inside `spawnSync`'s isolated loop. + pub(crate) fn kill_dangling_processes_on_timeout(&mut self, global_this: &JSGlobalObject) { // 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. @@ -326,11 +334,6 @@ impl Execution { } } } - - let buntest = self.bun_test(); - // SAFETY: deref parent at point-of-use; `self` is not accessed while this `&mut BunTest` is live. - unsafe { (*buntest).add_result(RefDataValue::Start) }; - Ok(()) } pub(crate) fn step(