From 47f0a691a9d526abe06842f9b56156279ab07aa3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 15 May 2026 10:00:11 +0000 Subject: [PATCH 1/8] bun test: interrupt synchronous infinite loops on --timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-test timeout was enforced by an event-loop timer, which can only fire once control returns to the event loop. A test body that never yields — e.g. `test("x", () => { while (true); })` — blocked the thread forever and the timeout never fired. Arm JSC's Watchdog around each test/hook callback in run_test_callback (src/runtime/test_runner/bun_test.rs). The watchdog schedules on a separate VMTraps queue thread and raises a TerminationException at the next JS safepoint, breaking out of the loop; the existing clear_termination_exception() + evaluate_timeout() path then reports the test as timed out. The watchdog is a hang detector, not the precise timer — callbacks that yield are still handled by the event-loop timer. A one-second grace over the deadline keeps it from racing the event-loop path for tests with very short timeouts whose synchronous prologue (spawn, fixture setup) would otherwise be cut off before the first await. On return the limit is relaxed to a large finite sentinel rather than noTimeLimit so the un-cancellable dispatchAfter can't drive shouldTerminate into startTimer(∞) and trip ASSERT(hasTimeLimit()). Hoist the dangling-process kill into kill_dangling_processes shared by handle_timeout and the synchronous-return path in step_sequence_one so children spawned by a watchdog-interrupted callback are still cleaned up (the event-loop timer never fires in that case). JSC__VM__setExecutionTimeLimit now calls watchdog.enteredVM() only when vm->entryScope is non-null, so the timer arms from inside an existing VMEntryScope without making Watchdog::isActive() lie when armed from native code (which would make VMTraps::handleTraps dereference the null vm.entryScope when servicing a stale NeedWatchdogCheck trap from autoTick). node:vm's checkForTermination / NodeVMModule::evaluate now let an external TerminationException propagate instead of RELEASE_ASSERT_NOT_REACHED when it came from neither SIGINT nor the user's {timeout} option — the test-runner watchdog is now a third source. Fixes #21277 --- src/jsc/bindings/NodeVMModule.cpp | 12 ++- src/jsc/bindings/NodeVMScript.cpp | 16 ++-- src/jsc/bindings/bindings.cpp | 15 ++++ src/runtime/test_runner/Execution.rs | 43 ++++++--- src/runtime/test_runner/bun_test.rs | 42 +++++++++ test/cli/test/test-timeout-behavior.test.ts | 97 ++++++++++++++++++++- 6 files changed, 201 insertions(+), 24 deletions(-) diff --git a/src/jsc/bindings/NodeVMModule.cpp b/src/jsc/bindings/NodeVMModule.cpp index 5be76aa6e04e..b281983c8807 100644 --- a/src/jsc/bindings/NodeVMModule.cpp +++ b/src/jsc/bindings/NodeVMModule.cpp @@ -254,16 +254,20 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b } if (vm.hasTerminationRequest() || vm.hasPendingTerminationException()) { vm.drainMicrotasksForGlobalObject(nodeVmGlobalObject); - DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); - vm.clearHasTerminationRequest(); if (getSigintReceived()) { + DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); + vm.clearHasTerminationRequest(); setSigintReceived(false); throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); } else if (timeout != 0) { + DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); + vm.clearHasTerminationRequest(); throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s)); - } else { - RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE("vm.SourceTextModule evaluation terminated due neither to SIGINT nor to timeout"); } + // else: termination came from outside this module evaluation (the + // bun:test watchdog around the test body, Worker.terminate(), etc.). + // Leave the TerminationException pending so it propagates to + // whoever armed it; VM_RETURN_IF_EXCEPTION below bails out. } else { setSigintReceived(false); } diff --git a/src/jsc/bindings/NodeVMScript.cpp b/src/jsc/bindings/NodeVMScript.cpp index 1a8653a1184a..51a64b780389 100644 --- a/src/jsc/bindings/NodeVMScript.cpp +++ b/src/jsc/bindings/NodeVMScript.cpp @@ -319,17 +319,23 @@ static bool checkForTermination(JSC::VM& vm, JSC::JSGlobalObject* globalObject, // The termination may have fired inside an afterEvaluate microtask // checkpoint, leaving the termination exception pending; clear it so // the ERR_SCRIPT_EXECUTION_* error below replaces it. - if (vm.hasPendingTerminationException()) - DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); - vm.clearHasTerminationRequest(); + auto clearPendingTermination = [&] { + if (vm.hasPendingTerminationException()) + DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); + vm.clearHasTerminationRequest(); + }; if (script->getSigintReceived()) { + clearPendingTermination(); script->setSigintReceived(false); throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); } else if (timeout) { + clearPendingTermination(); throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, *timeout, "ms"_s)); - } else { - RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE("vm.Script terminated due neither to SIGINT nor to timeout"); } + // else: termination came from outside this Script (the bun:test + // watchdog around the test body, Worker.terminate(), etc.). Leave + // the request set and the TerminationException in the scope so it + // propagates to whoever armed it. return true; } diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index b024bc42ca6c..7d7a2beb5190 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -5092,6 +5092,21 @@ void JSC__VM__setExecutionTimeLimit(JSC::VM* vm, double limit) { JSC::JSLockHolder locker(vm); JSC::Watchdog& watchdog = vm->ensureWatchdog(); + // When called from inside an existing VMEntryScope, start the watchdog + // timer now: the scope's own Watchdog::enteredVM() only runs on the + // outermost entry, so if the watchdog is being created for the first + // time here, m_hasEnteredVM would otherwise stay false and the timer + // would never arm. Mirrors setupWatchdog() in NodeVMScript.cpp. + // + // When called from *outside* any VMEntryScope (the test runner arming + // before callback.call()), leave m_hasEnteredVM alone — the next + // VMEntryScope's setUpSlow() will call enteredVM() and start the timer. + // Forcing it true here with no scope active would make + // Watchdog::isActive() lie, and VMTraps::handleTraps() would then + // dereference the null vm.entryScope when servicing a stale + // NeedWatchdogCheck trap from native code. + if (vm->entryScope) + watchdog.enteredVM(); watchdog.setTimeLimit(WTF::Seconds { limit }); } diff --git a/src/runtime/test_runner/Execution.rs b/src/runtime/test_runner/Execution.rs index a67b00fcc37a..fc75144e1f57 100644 --- a/src/runtime/test_runner/Execution.rs +++ b/src/runtime/test_runner/Execution.rs @@ -306,9 +306,6 @@ impl Execution { /// 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. 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); @@ -320,16 +317,7 @@ impl Execution { 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!( - "killed {} dangling process{}", - kill_count.processes, - if kill_count.processes != 1 { "es" } else { "" }, - ); - bun_core::Output::flush(); - } + kill_dangling_processes(end - start, global_this); } } } @@ -1044,7 +1032,11 @@ 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) { + // SAFETY: group points into this.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); @@ -1085,3 +1077,26 @@ fn step_sequence_one( return Ok(None); // run again } } + +/// Kill child processes spawned by the timed-out test so they don't outlive +/// it. Skipped under test.concurrent() because the auto-killer tracks +/// processes globally and we'd take out other still-running tests' children. +/// Called from both the event-loop timer path (handle_timeout) and the +/// synchronous-return path in step_sequence_one when the JSC watchdog has +/// interrupted a busy-looping callback — in that case the event-loop timer +/// never fires, so this is the only chance to clean up. +fn kill_dangling_processes(group_sequence_count: usize, global_this: &JSGlobalObject) { + if group_sequence_count != 1 { + return; + } + // 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!( + "killed {} dangling process{}", + kill_count.processes, + if kill_count.processes != 1 { "es" } else { "" }, + ); + bun_core::Output::flush(); + } +} diff --git a/src/runtime/test_runner/bun_test.rs b/src/runtime/test_runner/bun_test.rs index 8f2ee1f522c2..981cac057b89 100644 --- a/src/runtime/test_runner/bun_test.rs +++ b/src/runtime/test_runner/bun_test.rs @@ -1154,6 +1154,48 @@ impl BunTest { // SAFETY: `UnsafeCell`-derived; sole `&mut` at this point (before JS re-entry). unsafe { (*this).update_min_timeout(global_this, timeout) }; + + // Arm the JSC watchdog so synchronous infinite loops (e.g. + // `while (true);`) in the test body are interrupted. The event-loop + // timer above can only fire once control returns to the event loop, + // which never happens if the callback never yields. JSC's Watchdog + // schedules on a separate VMTraps queue thread and raises a + // TerminationException at the next safepoint; the Err arm below + // clears it so subsequent tests can run, and evaluate_timeout() in + // step_sequence_one() reports FailBecauseTimeout. + // + // The watchdog is a hang detector, not the precise timer — that's the + // event-loop timer's job for callbacks that yield. A one-second grace + // over the test deadline avoids interrupting a synchronous prologue + // (spawning a child, building fixtures) that would have yielded in + // time for the event-loop timer to handle the timeout on the next + // tick; without it, very short per-test timeouts would be cut off + // before they reach their first await. + // + // On return, the limit is relaxed to a large finite sentinel rather + // than cleared to noTimeLimit. Watchdog::startTimer()'s dispatchAfter + // can't be cancelled; if m_timeLimit were infinity then the next + // VMEntryScope's enteredVM() would skip startTimer (no hasTimeLimit), + // leaving m_cpuDeadline at the infinity exitedVM() parked it at, and + // when the stale dispatch fires shouldTerminate() would call + // startTimer(∞) and trip ASSERT(hasTimeLimit()). Keeping a finite + // limit makes every enteredVM() refresh m_cpuDeadline so the stale + // timer resolves to a harmless early return. + const WATCHDOG_GRACE_SECONDS: f64 = 1.0; + const WATCHDOG_IDLE_SECONDS: f64 = i32::MAX as f64; + let watchdog_armed = !timeout.eql(&Timespec::EPOCH); + if watchdog_armed { + let now = Timespec::now_force_real_time(); + let remaining_ns: u64 = if timeout.order(&now).is_gt() { timeout.duration(&now).ns() } else { 0 }; + let remaining_seconds = remaining_ns as f64 / bun_core::time::NS_PER_S as f64; + vm.jsc_vm().set_execution_time_limit(remaining_seconds + WATCHDOG_GRACE_SECONDS); + } + let _watchdog_relax = scopeguard::guard(watchdog_armed, |armed| { + if armed { + vm.jsc_vm().set_execution_time_limit(WATCHDOG_IDLE_SECONDS); + } + }); + let args_slice: &[JSValue] = if !done_arg.is_empty() { core::slice::from_ref(&done_arg) } else { &[] }; let result: JSValue = match vm.event_loop_mut().run_callback_with_result_and_forcefully_drain_microtasks( cfg_callback, diff --git a/test/cli/test/test-timeout-behavior.test.ts b/test/cli/test/test-timeout-behavior.test.ts index 07e02e3f039c..64452fc33db0 100644 --- a/test/cli/test/test-timeout-behavior.test.ts +++ b/test/cli/test/test-timeout-behavior.test.ts @@ -1,7 +1,102 @@ 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"; +// https://github.com/oven-sh/bun/issues/21277 +// A synchronous infinite loop in a test body must be interrupted by the +// per-test timeout. The event-loop timer alone cannot fire while JS is +// running, so the JSC watchdog is armed around the callback to raise a +// TerminationException at the next safepoint. +test.concurrent("synchronous infinite loop is interrupted by --timeout", async () => { + using dir = tempDir("timeout-sync-loop", { + "loop.test.ts": ` + import { test } from "bun:test"; + test("spins forever", () => { + while (true); + }); + test("runs after the timed-out test", () => {}); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--timeout=500", "loop.test.ts"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const combined = stdout + stderr; + + // The spinning test is reported as a timeout (not a generic failure), + // and the next test in the file still runs. + expect(combined).toContain("(fail) spins forever"); + expect(combined).toContain("timed out after 500ms"); + expect(combined).toContain("(pass) runs after the timed-out test"); + expect(exitCode).toBe(1); +}); + +test.concurrent("synchronous infinite loop after awaited microtask is interrupted by --timeout", async () => { + using dir = tempDir("timeout-sync-loop-microtask", { + "loop.test.ts": ` + import { test } from "bun:test"; + test("spins after await", async () => { + await Promise.resolve(); + while (true); + }); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--timeout=500", "loop.test.ts"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const combined = stdout + stderr; + + expect(combined).toContain("(fail) spins after await"); + expect(combined).toContain("timed out after 500ms"); + expect(exitCode).toBe(1); +}); + +// The outer watchdog's TerminationException must propagate through +// node:vm's Script/Module evaluation when the user didn't pass a +// {timeout} option — NodeVMScript::checkForTermination previously +// RELEASE_ASSERT'd that the termination came from its own watchdog. +test.concurrent("synchronous infinite loop inside node:vm without {timeout} is interrupted", async () => { + using dir = tempDir("timeout-sync-loop-nodevm", { + "loop.test.ts": ` + import { test } from "bun:test"; + import vm from "node:vm"; + test("spins inside runInThisContext", () => { + vm.runInThisContext("while (true);"); + }); + test("runs after the timed-out test", () => {}); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--timeout=500", "loop.test.ts"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const combined = stdout + stderr; + + expect(combined).toContain("(fail) spins inside runInThisContext"); + expect(combined).toContain("timed out after 500ms"); + expect(combined).toContain("(pass) runs after the timed-out test"); + expect(exitCode).toBe(1); +}); + if (isFlaky && isLinux) { test.todo("processes get killed"); } else { From 16114d4ae11fdb9a9ea8fcfd8c4465d9af0423c7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 22 May 2026 00:27:29 +0000 Subject: [PATCH 2/8] treat the test-runner idle watchdog sentinel as 'no limit' in hasExecutionTimeLimit run_test_callback relaxes the JSC watchdog to INT32_MAX seconds between callbacks instead of noTimeLimit (to keep Watchdog::startTimer's internal asserts happy across stale dispatchAfter callbacks). That left JSC__VM__hasExecutionTimeLimit() returning true for the rest of the process, which permanently opted spawnSync out of its blocking fast path in top-level / describe() code where auto_killer.enabled is false. Gate on getTimeLimit() < INT32_MAX so the idle sentinel reads as 'no limit' while a real node:vm {timeout} still disables the fast path. --- src/jsc/bindings/bindings.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 7d7a2beb5190..5ce211582142 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -3050,8 +3050,15 @@ void JSC__VM__collectAsync(JSC::VM* vm) extern "C" bool JSC__VM__hasExecutionTimeLimit(JSC::VM* vm) { JSC::JSLockHolder locker(vm); - if (vm->watchdog()) { - return vm->watchdog()->hasTimeLimit(); + if (auto* watchdog = vm->watchdog()) { + // The bun:test runner relaxes the watchdog to INT32_MAX seconds + // between test callbacks (see run_test_callback in bun_test.rs) + // instead of clearing it, to keep Watchdog's internal state + // consistent across stale dispatchAfter callbacks. Treat that idle + // sentinel as "no limit" so it doesn't permanently opt spawnSync + // out of its blocking fast path; no real caller sets a limit in + // this range. + return watchdog->hasTimeLimit() && watchdog->getTimeLimit() < WTF::Seconds { static_cast(INT32_MAX) }; } return false; From 8044e18e2998bcba3e37b1c6fc3e682839dea89b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 23 Jun 2026 05:43:49 +0000 Subject: [PATCH 3/8] node:vm: propagate external termination in the already-evaluated re-drain path too The microtaskMode: 'afterEvaluate' re-evaluation branch of NodeVMModule::evaluate has a second termination block that was still clearing the exception/request unconditionally and throwing a 'timed out after 0ms' error when the outer test-runner watchdog (or Worker.terminate) fires during drainOwnMicrotasks with no user {timeout}. Apply the same fix as the main-evaluation block: only clear and convert to ERR_SCRIPT_EXECUTION_* when SIGINT or the user's {timeout} caused it; otherwise leave the TerminationException pending. --- src/jsc/bindings/NodeVMModule.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/jsc/bindings/NodeVMModule.cpp b/src/jsc/bindings/NodeVMModule.cpp index b281983c8807..4fb67f3953df 100644 --- a/src/jsc/bindings/NodeVMModule.cpp +++ b/src/jsc/bindings/NodeVMModule.cpp @@ -108,14 +108,20 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b } if (vm.hasTerminationRequest() || vm.hasPendingTerminationException()) { vm.drainMicrotasksForGlobalObject(nodeVmGlobalObject); - DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); - vm.clearHasTerminationRequest(); if (getSigintReceived()) { + DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); + vm.clearHasTerminationRequest(); setSigintReceived(false); throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); - } else { + } else if (timeout != 0) { + DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); + vm.clearHasTerminationRequest(); throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s)); } + // else: termination came from outside this module evaluation + // (the bun:test watchdog around the test body, + // Worker.terminate(), etc.). Leave the TerminationException + // pending so it propagates to whoever armed it. return {}; } } From 6b4e318db8d534cb056ef681c9a8eb76aec164a6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 22:46:58 +0000 Subject: [PATCH 4/8] restore VM::set_execution_time_limit wrapper The dead-Rust sweep (#35002) removed this binding while it had no callers; the test runner's watchdog arming in run_test_callback now uses it. The C++ JSC__VM__setExecutionTimeLimit is unchanged. --- src/jsc/VM.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/jsc/VM.rs b/src/jsc/VM.rs index 3081278db388..888f017e731b 100644 --- a/src/jsc/VM.rs +++ b/src/jsc/VM.rs @@ -14,6 +14,7 @@ use crate::{JSGlobalObject, JSValue, JsError}; unsafe extern "C" { safe fn JSC__VM__enableControlFlowProfiler(vm: &VM); safe fn JSC__VM__hasExecutionTimeLimit(vm: &VM) -> bool; + safe fn JSC__VM__setExecutionTimeLimit(vm: &VM, timeout_seconds: f64); // safe: `VM` is an opaque `UnsafeCell`-backed ZST handle (`&` is ABI-identical // to non-null `*const`); `ctx` is an opaque round-trip pointer C++ only forwards // to `callback` (never dereferenced as Rust data) — same contract as @@ -58,6 +59,13 @@ impl VM { JSC__VM__hasExecutionTimeLimit(self) } + /// Arms JSC's `Watchdog` so running JS is interrupted with a + /// `TerminationException` at the next safepoint once `timeout_seconds` + /// of CPU time elapse. Used by the test runner around each test callback. + pub fn set_execution_time_limit(&self, timeout_seconds: f64) { + JSC__VM__setExecutionTimeLimit(self, timeout_seconds) + } + /// deprecated in favor of `get_api_lock` to avoid an annoying callback wrapper #[deprecated = "use get_api_lock"] pub fn hold_api_lock(&self, ctx: *mut c_void, callback: extern "C" fn(ctx: *mut c_void)) { From 8e4902d79e2ba73a09d7a68f01f7ca5a6f83d9ff Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 22:56:27 +0000 Subject: [PATCH 5/8] watchdog: own the idle sentinel in clearExecutionTimeLimit; trim comments Define the parked 'cleared' limit once in bindings.cpp and have both JSC__VM__clearExecutionTimeLimit and JSC__VM__hasExecutionTimeLimit use it, so the test runner just calls clear_execution_time_limit() and no longer carries its own copy of the magic value. Shorten the remaining comments to the invariants that aren't recoverable from the code. --- src/jsc/VM.rs | 8 +++--- src/jsc/bindings/NodeVMModule.cpp | 10 ++----- src/jsc/bindings/NodeVMScript.cpp | 5 +--- src/jsc/bindings/bindings.cpp | 39 ++++++++++------------------ src/runtime/test_runner/Execution.rs | 9 ++----- src/runtime/test_runner/bun_test.rs | 34 +++++------------------- 6 files changed, 30 insertions(+), 75 deletions(-) diff --git a/src/jsc/VM.rs b/src/jsc/VM.rs index 888f017e731b..0e08e55fe538 100644 --- a/src/jsc/VM.rs +++ b/src/jsc/VM.rs @@ -15,6 +15,7 @@ unsafe extern "C" { safe fn JSC__VM__enableControlFlowProfiler(vm: &VM); safe fn JSC__VM__hasExecutionTimeLimit(vm: &VM) -> bool; safe fn JSC__VM__setExecutionTimeLimit(vm: &VM, timeout_seconds: f64); + safe fn JSC__VM__clearExecutionTimeLimit(vm: &VM); // safe: `VM` is an opaque `UnsafeCell`-backed ZST handle (`&` is ABI-identical // to non-null `*const`); `ctx` is an opaque round-trip pointer C++ only forwards // to `callback` (never dereferenced as Rust data) — same contract as @@ -59,13 +60,14 @@ impl VM { JSC__VM__hasExecutionTimeLimit(self) } - /// Arms JSC's `Watchdog` so running JS is interrupted with a - /// `TerminationException` at the next safepoint once `timeout_seconds` - /// of CPU time elapse. Used by the test runner around each test callback. pub fn set_execution_time_limit(&self, timeout_seconds: f64) { JSC__VM__setExecutionTimeLimit(self, timeout_seconds) } + pub fn clear_execution_time_limit(&self) { + JSC__VM__clearExecutionTimeLimit(self) + } + /// deprecated in favor of `get_api_lock` to avoid an annoying callback wrapper #[deprecated = "use get_api_lock"] pub fn hold_api_lock(&self, ctx: *mut c_void, callback: extern "C" fn(ctx: *mut c_void)) { diff --git a/src/jsc/bindings/NodeVMModule.cpp b/src/jsc/bindings/NodeVMModule.cpp index 4fb67f3953df..cb86295be01a 100644 --- a/src/jsc/bindings/NodeVMModule.cpp +++ b/src/jsc/bindings/NodeVMModule.cpp @@ -118,10 +118,7 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b vm.clearHasTerminationRequest(); throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s)); } - // else: termination came from outside this module evaluation - // (the bun:test watchdog around the test body, - // Worker.terminate(), etc.). Leave the TerminationException - // pending so it propagates to whoever armed it. + // Otherwise the termination isn't ours; leave it pending so it propagates. return {}; } } @@ -270,10 +267,7 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b vm.clearHasTerminationRequest(); throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s)); } - // else: termination came from outside this module evaluation (the - // bun:test watchdog around the test body, Worker.terminate(), etc.). - // Leave the TerminationException pending so it propagates to - // whoever armed it; VM_RETURN_IF_EXCEPTION below bails out. + // Otherwise the termination isn't ours; leave it pending so it propagates. } else { setSigintReceived(false); } diff --git a/src/jsc/bindings/NodeVMScript.cpp b/src/jsc/bindings/NodeVMScript.cpp index 51a64b780389..6c562a766966 100644 --- a/src/jsc/bindings/NodeVMScript.cpp +++ b/src/jsc/bindings/NodeVMScript.cpp @@ -332,10 +332,7 @@ static bool checkForTermination(JSC::VM& vm, JSC::JSGlobalObject* globalObject, clearPendingTermination(); throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, *timeout, "ms"_s)); } - // else: termination came from outside this Script (the bun:test - // watchdog around the test body, Worker.terminate(), etc.). Leave - // the request set and the TerminationException in the scope so it - // propagates to whoever armed it. + // Otherwise the termination isn't ours; leave it pending so it propagates. return true; } diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 5ce211582142..427653bf87b2 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -3047,19 +3047,17 @@ void JSC__VM__collectAsync(JSC::VM* vm) vm->heap.collectAsync(); } +// What JSC__VM__clearExecutionTimeLimit parks the watchdog at. Watchdog's +// already-dispatched timer can't be cancelled and asserts hasTimeLimit() when +// it fires, so "cleared" is a finite limit that never elapses rather than +// Watchdog::noTimeLimit. +static constexpr WTF::Seconds idleExecutionTimeLimit { static_cast(INT32_MAX) }; + extern "C" bool JSC__VM__hasExecutionTimeLimit(JSC::VM* vm) { JSC::JSLockHolder locker(vm); - if (auto* watchdog = vm->watchdog()) { - // The bun:test runner relaxes the watchdog to INT32_MAX seconds - // between test callbacks (see run_test_callback in bun_test.rs) - // instead of clearing it, to keep Watchdog's internal state - // consistent across stale dispatchAfter callbacks. Treat that idle - // sentinel as "no limit" so it doesn't permanently opt spawnSync - // out of its blocking fast path; no real caller sets a limit in - // this range. - return watchdog->hasTimeLimit() && watchdog->getTimeLimit() < WTF::Seconds { static_cast(INT32_MAX) }; - } + if (auto* watchdog = vm->watchdog()) + return watchdog->getTimeLimit() < idleExecutionTimeLimit; return false; } @@ -5092,26 +5090,17 @@ size_t JSC__VM__runGC(JSC::VM* vm, bool sync) void JSC__VM__clearExecutionTimeLimit(JSC::VM* vm) { JSC::JSLockHolder locker(vm); - if (vm->watchdog()) - vm->watchdog()->setTimeLimit(JSC::Watchdog::noTimeLimit); + if (auto* watchdog = vm->watchdog()) + watchdog->setTimeLimit(idleExecutionTimeLimit); } void JSC__VM__setExecutionTimeLimit(JSC::VM* vm, double limit) { JSC::JSLockHolder locker(vm); JSC::Watchdog& watchdog = vm->ensureWatchdog(); - // When called from inside an existing VMEntryScope, start the watchdog - // timer now: the scope's own Watchdog::enteredVM() only runs on the - // outermost entry, so if the watchdog is being created for the first - // time here, m_hasEnteredVM would otherwise stay false and the timer - // would never arm. Mirrors setupWatchdog() in NodeVMScript.cpp. - // - // When called from *outside* any VMEntryScope (the test runner arming - // before callback.call()), leave m_hasEnteredVM alone — the next - // VMEntryScope's setUpSlow() will call enteredVM() and start the timer. - // Forcing it true here with no scope active would make - // Watchdog::isActive() lie, and VMTraps::handleTraps() would then - // dereference the null vm.entryScope when servicing a stale - // NeedWatchdogCheck trap from native code. + // Watchdog::enteredVM() normally runs from VMEntryScope setup, so a watchdog + // created while JS is already on the stack must be entered by hand. With no + // entry scope the next one enters it; entering here instead would make + // VMTraps::handleTraps deref a null vm.entryScope on a stale check. if (vm->entryScope) watchdog.enteredVM(); watchdog.setTimeLimit(WTF::Seconds { limit }); diff --git a/src/runtime/test_runner/Execution.rs b/src/runtime/test_runner/Execution.rs index fc75144e1f57..e3bb53470310 100644 --- a/src/runtime/test_runner/Execution.rs +++ b/src/runtime/test_runner/Execution.rs @@ -1078,13 +1078,8 @@ fn step_sequence_one( } } -/// Kill child processes spawned by the timed-out test so they don't outlive -/// it. Skipped under test.concurrent() because the auto-killer tracks -/// processes globally and we'd take out other still-running tests' children. -/// Called from both the event-loop timer path (handle_timeout) and the -/// synchronous-return path in step_sequence_one when the JSC watchdog has -/// interrupted a busy-looping callback — in that case the event-loop timer -/// never fires, so this is the only chance to clean up. +/// Skipped under test.concurrent(): the auto-killer is process-global and would +/// take out other in-flight tests' children too. fn kill_dangling_processes(group_sequence_count: usize, global_this: &JSGlobalObject) { if group_sequence_count != 1 { return; diff --git a/src/runtime/test_runner/bun_test.rs b/src/runtime/test_runner/bun_test.rs index 981cac057b89..0299465020ad 100644 --- a/src/runtime/test_runner/bun_test.rs +++ b/src/runtime/test_runner/bun_test.rs @@ -1155,34 +1155,12 @@ impl BunTest { // SAFETY: `UnsafeCell`-derived; sole `&mut` at this point (before JS re-entry). unsafe { (*this).update_min_timeout(global_this, timeout) }; - // Arm the JSC watchdog so synchronous infinite loops (e.g. - // `while (true);`) in the test body are interrupted. The event-loop - // timer above can only fire once control returns to the event loop, - // which never happens if the callback never yields. JSC's Watchdog - // schedules on a separate VMTraps queue thread and raises a - // TerminationException at the next safepoint; the Err arm below - // clears it so subsequent tests can run, and evaluate_timeout() in - // step_sequence_one() reports FailBecauseTimeout. - // - // The watchdog is a hang detector, not the precise timer — that's the - // event-loop timer's job for callbacks that yield. A one-second grace - // over the test deadline avoids interrupting a synchronous prologue - // (spawning a child, building fixtures) that would have yielded in - // time for the event-loop timer to handle the timeout on the next - // tick; without it, very short per-test timeouts would be cut off - // before they reach their first await. - // - // On return, the limit is relaxed to a large finite sentinel rather - // than cleared to noTimeLimit. Watchdog::startTimer()'s dispatchAfter - // can't be cancelled; if m_timeLimit were infinity then the next - // VMEntryScope's enteredVM() would skip startTimer (no hasTimeLimit), - // leaving m_cpuDeadline at the infinity exitedVM() parked it at, and - // when the stale dispatch fires shouldTerminate() would call - // startTimer(∞) and trip ASSERT(hasTimeLimit()). Keeping a finite - // limit makes every enteredVM() refresh m_cpuDeadline so the stale - // timer resolves to a harmless early return. + // The event-loop timer above can't fire while the callback spins + // synchronously; JSC's watchdog throws a TerminationException at the + // next safepoint instead, which the Err arm below clears and + // evaluate_timeout() reports. The grace keeps it from racing the + // event-loop timer for callbacks that do yield. const WATCHDOG_GRACE_SECONDS: f64 = 1.0; - const WATCHDOG_IDLE_SECONDS: f64 = i32::MAX as f64; let watchdog_armed = !timeout.eql(&Timespec::EPOCH); if watchdog_armed { let now = Timespec::now_force_real_time(); @@ -1192,7 +1170,7 @@ impl BunTest { } let _watchdog_relax = scopeguard::guard(watchdog_armed, |armed| { if armed { - vm.jsc_vm().set_execution_time_limit(WATCHDOG_IDLE_SECONDS); + vm.jsc_vm().clear_execution_time_limit(); } }); From 160573621cd5d6ff82fc3ca141caa28fa924b098 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 23:11:04 +0000 Subject: [PATCH 6/8] node:vm: route external terminations through the existing propagate path Each termination check in NodeVMScript/NodeVMModule already had an escape for terminations that aren't the script's own (VM being stopped). Widen that same escape to cover a termination requested by neither the script's SIGINT nor its {timeout} (e.g. the enclosing bun:test watchdog), instead of falling through after the SIGINT/timeout branches. In NodeVMModule::evaluate the fall-through reached VM_RETURN_IF_EXCEPTION, which stamped the module Errored with the TerminationException as its permanent evaluation error. The now-unreachable RELEASE_ASSERTs go away, and the node:vm fixture gains a SourceTextModule.evaluate case. --- src/jsc/bindings/NodeVMModule.cpp | 49 ++++++++++---------- src/jsc/bindings/NodeVMScript.cpp | 50 ++++++++++----------- src/jsc/bindings/bindings.cpp | 10 +---- src/runtime/test_runner/Execution.rs | 3 +- src/runtime/test_runner/bun_test.rs | 7 +-- test/cli/test/test-timeout-behavior.test.ts | 12 +++-- 6 files changed, 60 insertions(+), 71 deletions(-) diff --git a/src/jsc/bindings/NodeVMModule.cpp b/src/jsc/bindings/NodeVMModule.cpp index cb86295be01a..3ad8df79505f 100644 --- a/src/jsc/bindings/NodeVMModule.cpp +++ b/src/jsc/bindings/NodeVMModule.cpp @@ -100,25 +100,23 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b // exception-check validator is satisfied before the TOP scope // below, then convert it to ERR_SCRIPT_EXECUTION_*. std::ignore = scope.exception(); - if ((vm.hasTerminationRequest() || vm.hasPendingTerminationException()) && !Bun__VmHandle__scriptAllowed(WebCore::clientData(vm)->vmHandle)) { - // The VM itself is being stopped; not ours to consume. Propagate the termination. - if (!vm.hasPendingTerminationException()) - vm.throwTerminationException(); - return {}; - } if (vm.hasTerminationRequest() || vm.hasPendingTerminationException()) { + bool sigint = getSigintReceived(); + if (!Bun__VmHandle__scriptAllowed(WebCore::clientData(vm)->vmHandle) || (!sigint && timeout == 0)) { + // Not a termination this evaluation requested; propagate it untouched. + if (!vm.hasPendingTerminationException()) + vm.throwTerminationException(); + return {}; + } vm.drainMicrotasksForGlobalObject(nodeVmGlobalObject); - if (getSigintReceived()) { - DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); - vm.clearHasTerminationRequest(); + DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); + vm.clearHasTerminationRequest(); + if (sigint) { setSigintReceived(false); throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); - } else if (timeout != 0) { - DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); - vm.clearHasTerminationRequest(); + } else { throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s)); } - // Otherwise the termination isn't ours; leave it pending so it propagates. return {}; } } @@ -249,25 +247,24 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b // termination one is converted to ERR_SCRIPT_EXECUTION_* here. Observe it // so the exception-check validator is satisfied before the TOP scope. std::ignore = scope.exception(); - if ((vm.hasTerminationRequest() || vm.hasPendingTerminationException()) && !Bun__VmHandle__scriptAllowed(WebCore::clientData(vm)->vmHandle)) { - // The VM itself is being stopped; not ours to consume. Propagate the termination. - if (!vm.hasPendingTerminationException()) - vm.throwTerminationException(); - return {}; - } if (vm.hasTerminationRequest() || vm.hasPendingTerminationException()) { + bool sigint = getSigintReceived(); + if (!Bun__VmHandle__scriptAllowed(WebCore::clientData(vm)->vmHandle) || (!sigint && timeout == 0)) { + // Not a termination this evaluation requested; propagate it untouched + // (and don't record it as the module's evaluation error below). + if (!vm.hasPendingTerminationException()) + vm.throwTerminationException(); + return {}; + } vm.drainMicrotasksForGlobalObject(nodeVmGlobalObject); - if (getSigintReceived()) { - DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); - vm.clearHasTerminationRequest(); + DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); + vm.clearHasTerminationRequest(); + if (sigint) { setSigintReceived(false); throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); - } else if (timeout != 0) { - DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); - vm.clearHasTerminationRequest(); + } else { throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s)); } - // Otherwise the termination isn't ours; leave it pending so it propagates. } else { setSigintReceived(false); } diff --git a/src/jsc/bindings/NodeVMScript.cpp b/src/jsc/bindings/NodeVMScript.cpp index 6c562a766966..80d336d4260f 100644 --- a/src/jsc/bindings/NodeVMScript.cpp +++ b/src/jsc/bindings/NodeVMScript.cpp @@ -309,34 +309,30 @@ void NodeVMScript::destroy(JSCell* cell) static bool checkForTermination(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::ThrowScope& scope, NodeVMScript* script, std::optional timeout) { - if (vm.hasTerminationRequest()) { - // The whole VM is being stopped (worker terminate()/exit): that - // termination is not ours to consume. The caller rethrows what - // evaluate() caught like any other exception. - if (!Bun__VmHandle__scriptAllowed(WebCore::clientData(vm)->vmHandle)) - return false; - vm.drainMicrotasksForGlobalObject(globalObject); - // The termination may have fired inside an afterEvaluate microtask - // checkpoint, leaving the termination exception pending; clear it so - // the ERR_SCRIPT_EXECUTION_* error below replaces it. - auto clearPendingTermination = [&] { - if (vm.hasPendingTerminationException()) - DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); - vm.clearHasTerminationRequest(); - }; - if (script->getSigintReceived()) { - clearPendingTermination(); - script->setSigintReceived(false); - throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); - } else if (timeout) { - clearPendingTermination(); - throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, *timeout, "ms"_s)); - } - // Otherwise the termination isn't ours; leave it pending so it propagates. - return true; + if (!vm.hasTerminationRequest()) + return false; + + // Only a termination this script requested (its own SIGINT/timeout) is ours + // to convert; anything else (worker stop, an enclosing watchdog) stays + // pending and the caller rethrows it like any other exception. + bool sigint = script->getSigintReceived(); + if (!Bun__VmHandle__scriptAllowed(WebCore::clientData(vm)->vmHandle) || (!sigint && !timeout)) + return false; + + vm.drainMicrotasksForGlobalObject(globalObject); + // The termination may have fired inside an afterEvaluate microtask + // checkpoint, leaving the termination exception pending; clear it so + // the ERR_SCRIPT_EXECUTION_* error below replaces it. + if (vm.hasPendingTerminationException()) + DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException(); + vm.clearHasTerminationRequest(); + if (sigint) { + script->setSigintReceived(false); + throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); + } else { + throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, *timeout, "ms"_s)); } - - return false; + return true; } void setupWatchdog(VM& vm, double timeout, double* oldTimeout, double* newTimeout) diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 427653bf87b2..f2ae22788b2d 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -3047,10 +3047,7 @@ void JSC__VM__collectAsync(JSC::VM* vm) vm->heap.collectAsync(); } -// What JSC__VM__clearExecutionTimeLimit parks the watchdog at. Watchdog's -// already-dispatched timer can't be cancelled and asserts hasTimeLimit() when -// it fires, so "cleared" is a finite limit that never elapses rather than -// Watchdog::noTimeLimit. +// Finite, not noTimeLimit: an in-flight Watchdog timer asserts hasTimeLimit() when it fires. static constexpr WTF::Seconds idleExecutionTimeLimit { static_cast(INT32_MAX) }; extern "C" bool JSC__VM__hasExecutionTimeLimit(JSC::VM* vm) @@ -5097,10 +5094,7 @@ void JSC__VM__setExecutionTimeLimit(JSC::VM* vm, double limit) { JSC::JSLockHolder locker(vm); JSC::Watchdog& watchdog = vm->ensureWatchdog(); - // Watchdog::enteredVM() normally runs from VMEntryScope setup, so a watchdog - // created while JS is already on the stack must be entered by hand. With no - // entry scope the next one enters it; entering here instead would make - // VMTraps::handleTraps deref a null vm.entryScope on a stale check. + // VMEntryScope enters the watchdog itself; a scopeless enteredVM() makes VMTraps deref a null entryScope. if (vm->entryScope) watchdog.enteredVM(); watchdog.setTimeLimit(WTF::Seconds { limit }); diff --git a/src/runtime/test_runner/Execution.rs b/src/runtime/test_runner/Execution.rs index e3bb53470310..b2dd466d26bc 100644 --- a/src/runtime/test_runner/Execution.rs +++ b/src/runtime/test_runner/Execution.rs @@ -1078,9 +1078,8 @@ fn step_sequence_one( } } -/// Skipped under test.concurrent(): the auto-killer is process-global and would -/// take out other in-flight tests' children too. fn kill_dangling_processes(group_sequence_count: usize, global_this: &JSGlobalObject) { + // The auto-killer is process-global; under test.concurrent() it would hit other tests' children. if group_sequence_count != 1 { return; } diff --git a/src/runtime/test_runner/bun_test.rs b/src/runtime/test_runner/bun_test.rs index 0299465020ad..197c01a71e1b 100644 --- a/src/runtime/test_runner/bun_test.rs +++ b/src/runtime/test_runner/bun_test.rs @@ -1155,11 +1155,8 @@ impl BunTest { // SAFETY: `UnsafeCell`-derived; sole `&mut` at this point (before JS re-entry). unsafe { (*this).update_min_timeout(global_this, timeout) }; - // The event-loop timer above can't fire while the callback spins - // synchronously; JSC's watchdog throws a TerminationException at the - // next safepoint instead, which the Err arm below clears and - // evaluate_timeout() reports. The grace keeps it from racing the - // event-loop timer for callbacks that do yield. + // JSC's watchdog catches callbacks that never yield back to the event-loop timer above. + // The grace lets callbacks that do yield be timed by that timer instead of racing it. const WATCHDOG_GRACE_SECONDS: f64 = 1.0; let watchdog_armed = !timeout.eql(&Timespec::EPOCH); if watchdog_armed { diff --git a/test/cli/test/test-timeout-behavior.test.ts b/test/cli/test/test-timeout-behavior.test.ts index 64452fc33db0..d892a24d0c1c 100644 --- a/test/cli/test/test-timeout-behavior.test.ts +++ b/test/cli/test/test-timeout-behavior.test.ts @@ -76,7 +76,12 @@ test.concurrent("synchronous infinite loop inside node:vm without {timeout} is i test("spins inside runInThisContext", () => { vm.runInThisContext("while (true);"); }); - test("runs after the timed-out test", () => {}); + test("spins inside SourceTextModule.evaluate", async () => { + const mod = new vm.SourceTextModule("while (true);"); + await mod.link(() => {}); + await mod.evaluate(); + }); + test("runs after the timed-out tests", () => {}); `, }); @@ -92,8 +97,9 @@ test.concurrent("synchronous infinite loop inside node:vm without {timeout} is i const combined = stdout + stderr; expect(combined).toContain("(fail) spins inside runInThisContext"); - expect(combined).toContain("timed out after 500ms"); - expect(combined).toContain("(pass) runs after the timed-out test"); + expect(combined).toContain("(fail) spins inside SourceTextModule.evaluate"); + expect(combined.match(/timed out after 500ms/g)).toHaveLength(2); + expect(combined).toContain("(pass) runs after the timed-out tests"); expect(exitCode).toBe(1); }); From cbe5d15009ce8b92f4ff13aff2f396b71d84f1de Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 23:26:34 +0000 Subject: [PATCH 7/8] test runner: relax the watchdog before reporting a failed callback Reporting re-enters user JS (error message/stack getters). With the watchdog still armed there, a termination raised during reporting landed after the Err arm had already cleared, stayed pending, and made run_callback_with_result_and_forcefully_drain_microtasks skip every later callback in the step, which were then recorded as passing. Arm and relax around the callback invocation only. Also fold the three spawn stanzas in the fixture into a helper and add a case whose error message getter burns past the limit; without this change its second test, which fails an assertion, is reported as passed. --- src/runtime/test_runner/bun_test.rs | 15 +-- test/cli/test/test-timeout-behavior.test.ts | 100 ++++++++++++-------- 2 files changed, 66 insertions(+), 49 deletions(-) diff --git a/src/runtime/test_runner/bun_test.rs b/src/runtime/test_runner/bun_test.rs index 197c01a71e1b..d17b9416a813 100644 --- a/src/runtime/test_runner/bun_test.rs +++ b/src/runtime/test_runner/bun_test.rs @@ -1165,19 +1165,20 @@ impl BunTest { let remaining_seconds = remaining_ns as f64 / bun_core::time::NS_PER_S as f64; vm.jsc_vm().set_execution_time_limit(remaining_seconds + WATCHDOG_GRACE_SECONDS); } - let _watchdog_relax = scopeguard::guard(watchdog_armed, |armed| { - if armed { - vm.jsc_vm().clear_execution_time_limit(); - } - }); let args_slice: &[JSValue] = if !done_arg.is_empty() { core::slice::from_ref(&done_arg) } else { &[] }; - let result: JSValue = match vm.event_loop_mut().run_callback_with_result_and_forcefully_drain_microtasks( + let call_result = vm.event_loop_mut().run_callback_with_result_and_forcefully_drain_microtasks( cfg_callback, global_this, JSValue::UNDEFINED, args_slice, - ) { + ); + // Relax before reporting: error printing re-enters user JS (e.g. Error.prepareStackTrace), + // and a termination raised there would outlive the clear below and silently skip later tests. + if watchdog_armed { + vm.jsc_vm().clear_execution_time_limit(); + } + let result: JSValue = match call_result { Ok(v) => v, Err(_) => { global_this.clear_termination_exception(); diff --git a/test/cli/test/test-timeout-behavior.test.ts b/test/cli/test/test-timeout-behavior.test.ts index d892a24d0c1c..b6abc64cf8c1 100644 --- a/test/cli/test/test-timeout-behavior.test.ts +++ b/test/cli/test/test-timeout-behavior.test.ts @@ -2,32 +2,36 @@ import { expect, test } from "bun:test"; import { bunEnv, bunExe, isFlaky, isLinux, tempDir } from "harness"; import path from "path"; +// Runs `source` as a test file under `bun test --timeout=500` and returns its merged output. +async function runWithTimeout(prefix: string, source: string) { + using dir = tempDir(prefix, { "loop.test.ts": source }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--timeout=500", "loop.test.ts"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { combined: stdout + stderr, exitCode }; +} + // https://github.com/oven-sh/bun/issues/21277 // A synchronous infinite loop in a test body must be interrupted by the // per-test timeout. The event-loop timer alone cannot fire while JS is // running, so the JSC watchdog is armed around the callback to raise a // TerminationException at the next safepoint. test.concurrent("synchronous infinite loop is interrupted by --timeout", async () => { - using dir = tempDir("timeout-sync-loop", { - "loop.test.ts": ` + const { combined, exitCode } = await runWithTimeout( + "timeout-sync-loop", + ` import { test } from "bun:test"; test("spins forever", () => { while (true); }); test("runs after the timed-out test", () => {}); `, - }); - - await using proc = Bun.spawn({ - cmd: [bunExe(), "test", "--timeout=500", "loop.test.ts"], - env: bunEnv, - cwd: String(dir), - stdout: "pipe", - stderr: "pipe", - }); - - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - const combined = stdout + stderr; + ); // The spinning test is reported as a timeout (not a generic failure), // and the next test in the file still runs. @@ -38,26 +42,16 @@ test.concurrent("synchronous infinite loop is interrupted by --timeout", async ( }); test.concurrent("synchronous infinite loop after awaited microtask is interrupted by --timeout", async () => { - using dir = tempDir("timeout-sync-loop-microtask", { - "loop.test.ts": ` + const { combined, exitCode } = await runWithTimeout( + "timeout-sync-loop-microtask", + ` import { test } from "bun:test"; test("spins after await", async () => { await Promise.resolve(); while (true); }); `, - }); - - await using proc = Bun.spawn({ - cmd: [bunExe(), "test", "--timeout=500", "loop.test.ts"], - env: bunEnv, - cwd: String(dir), - stdout: "pipe", - stderr: "pipe", - }); - - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - const combined = stdout + stderr; + ); expect(combined).toContain("(fail) spins after await"); expect(combined).toContain("timed out after 500ms"); @@ -69,8 +63,9 @@ test.concurrent("synchronous infinite loop after awaited microtask is interrupte // {timeout} option — NodeVMScript::checkForTermination previously // RELEASE_ASSERT'd that the termination came from its own watchdog. test.concurrent("synchronous infinite loop inside node:vm without {timeout} is interrupted", async () => { - using dir = tempDir("timeout-sync-loop-nodevm", { - "loop.test.ts": ` + const { combined, exitCode } = await runWithTimeout( + "timeout-sync-loop-nodevm", + ` import { test } from "bun:test"; import vm from "node:vm"; test("spins inside runInThisContext", () => { @@ -83,18 +78,7 @@ test.concurrent("synchronous infinite loop inside node:vm without {timeout} is i }); test("runs after the timed-out tests", () => {}); `, - }); - - await using proc = Bun.spawn({ - cmd: [bunExe(), "test", "--timeout=500", "loop.test.ts"], - env: bunEnv, - cwd: String(dir), - stdout: "pipe", - stderr: "pipe", - }); - - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - const combined = stdout + stderr; + ); expect(combined).toContain("(fail) spins inside runInThisContext"); expect(combined).toContain("(fail) spins inside SourceTextModule.evaluate"); @@ -103,6 +87,38 @@ test.concurrent("synchronous infinite loop inside node:vm without {timeout} is i expect(exitCode).toBe(1); }); +// The watchdog must be disarmed before a failure is reported: printing the +// error re-enters user JS (here, a `message` getter). If the watchdog fired in +// there, the termination outlived the runner's clear and the next callback was +// skipped and reported as passing. +test.concurrent("watchdog does not fire while a failure is being reported", async () => { + const { combined, exitCode } = await runWithTimeout( + "timeout-slow-error-message", + ` + import { test, expect } from "bun:test"; + test("throws", () => { + const error = new Error("boom"); + Object.defineProperty(error, "message", { + get() { + // Burn more CPU than the 500ms timeout plus the watchdog's grace. + const start = process.cpuUsage(); + while (process.cpuUsage(start).user < 2_000_000) {} + return "boom"; + }, + }); + throw error; + }); + test("still runs and fails on its own assertion", () => { + expect(1).toBe(2); + }); + `, + ); + + expect(combined).toContain("(fail) throws"); + expect(combined).toContain("(fail) still runs and fails on its own assertion"); + expect(exitCode).toBe(1); +}); + if (isFlaky && isLinux) { test.todo("processes get killed"); } else { From 45da6e5a0255a849179aa4f94b50ce2496c12e4a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 23:30:00 +0000 Subject: [PATCH 8/8] fix comment: the reporting re-entry is via error getters, not prepareStackTrace --- src/runtime/test_runner/bun_test.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/runtime/test_runner/bun_test.rs b/src/runtime/test_runner/bun_test.rs index d17b9416a813..6721c1b9d1a0 100644 --- a/src/runtime/test_runner/bun_test.rs +++ b/src/runtime/test_runner/bun_test.rs @@ -1173,8 +1173,7 @@ impl BunTest { JSValue::UNDEFINED, args_slice, ); - // Relax before reporting: error printing re-enters user JS (e.g. Error.prepareStackTrace), - // and a termination raised there would outlive the clear below and silently skip later tests. + // Relax before reporting: printing the error runs user getters, after the clear below. if watchdog_armed { vm.jsc_vm().clear_execution_time_limit(); }