diff --git a/src/runtime/api/bun/js_bun_spawn_bindings.rs b/src/runtime/api/bun/js_bun_spawn_bindings.rs index 70910ff9dbf3..8c6e65c3868c 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,24 @@ 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() + { + 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(