Skip to content
10 changes: 10 additions & 0 deletions src/jsc/VM.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ 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 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
Expand Down Expand Up @@ -58,6 +60,14 @@ impl VM {
JSC__VM__hasExecutionTimeLimit(self)
}

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)) {
Expand Down
18 changes: 11 additions & 7 deletions src/jsc/bindings/NodeVMModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,17 @@ 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));
}
// Otherwise the termination isn't ours; leave it pending so it propagates.
return {};
}
}
Expand Down Expand Up @@ -254,16 +257,17 @@ 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");
}
// Otherwise the termination isn't ours; leave it pending so it propagates.
} else {
setSigintReceived(false);
}
Expand Down
13 changes: 8 additions & 5 deletions src/jsc/bindings/NodeVMScript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,17 +319,20 @@ 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");
}
// Otherwise the termination isn't ours; leave it pending so it propagates.
return true;
}

Expand Down
21 changes: 16 additions & 5 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3047,12 +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.
Comment thread
robobun marked this conversation as resolved.
Outdated
static constexpr WTF::Seconds idleExecutionTimeLimit { static_cast<double>(INT32_MAX) };

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())
return watchdog->getTimeLimit() < idleExecutionTimeLimit;

return false;
}
Expand Down Expand Up @@ -5085,13 +5090,19 @@ 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();
// 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.
Comment thread
robobun marked this conversation as resolved.
Outdated
if (vm->entryScope)
watchdog.enteredVM();
watchdog.setTimeLimit(WTF::Seconds { limit });
}

Expand Down
38 changes: 24 additions & 14 deletions src/runtime/test_runner/Execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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!(
"<d>killed {} dangling process{}<r>",
kill_count.processes,
if kill_count.processes != 1 { "es" } else { "" },
);
bun_core::Output::flush();
}
kill_dangling_processes(end - start, global_this);
}
}
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1085,3 +1077,21 @@ fn step_sequence_one(
return Ok(None); // run again
}
}

/// Skipped under test.concurrent(): the auto-killer is process-global and would
/// take out other in-flight tests' children too.
Comment thread
robobun marked this conversation as resolved.
Outdated
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!(
"<d>killed {} dangling process{}<r>",
kill_count.processes,
if kill_count.processes != 1 { "es" } else { "" },
);
bun_core::Output::flush();
}
}
20 changes: 20 additions & 0 deletions src/runtime/test_runner/bun_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1154,6 +1154,26 @@ 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.
Comment thread
robobun marked this conversation as resolved.
Outdated
const WATCHDOG_GRACE_SECONDS: f64 = 1.0;
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().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(
cfg_callback,
Expand Down
97 changes: 96 additions & 1 deletion test/cli/test/test-timeout-behavior.test.ts
Original file line number Diff line number Diff line change
@@ -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);
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
});

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 {
Expand Down
Loading