node:vm: enforce timeout as a wall-clock deadline that interrupts Atomics.wait

[robobun]

Problem

node:vm's timeout option is not enforced against a guest blocked in Atomics.wait, and the missed deadline leaves JSC's Watchdog in an inconsistent state:

const vm = require("node:vm");
const ia = new Int32Array(new SharedArrayBuffer(8));

let t = Date.now();
try { vm.runInNewContext("Atomics.wait(ia, 0, 0, 600)", { ia }, { timeout: 60 }); }
catch (e) { console.log(e.code, Date.now() - t); }
// node:  ERR_SCRIPT_EXECUTION_TIMEOUT ~65ms
// bun:   (no throw) returns "timed-out" after ~600ms

// On an assert-enabled build the next timed evaluation aborts:
//   ASSERTION FAILED: hasTimeLimit()
//   JavaScriptCore/runtime/Watchdog.cpp(133) JSC::Watchdog::startTimer
vm.runInNewContext("6*7", {}, { timeout: 5000 });

t = Date.now();
try { vm.runInNewContext("for(;;) Atomics.wait(ia, 0, 0, 10);", { ia }, { timeout: 80 }); }
catch (e) { console.log(e.code, Date.now() - t); }
// node:  ~81ms
// bun:   ~12000ms (150x past the budget)

timeout is the whole point of running untrusted code through node:vm, so a guest can currently evade the budget entirely with one Atomics.wait.

Cause

setupWatchdog drove timeout through JSC::Watchdog, which is the wrong tool on two axes:

1. The Watchdog's timer only sets the NeedWatchdogCheck trap bit, which is polled at JS back-edges. A thread parked in WaiterListManager::waitForSync has no back-edge, so the fire is never observed while the wait runs; the script returns "timed-out" after the full wait.
2. Watchdog::shouldTerminate decides on CPU time (m_cpuDeadline), not wall-clock. A sleeping thread consumes ~zero CPU, so chunked short waits under an 80 ms budget only terminate once 80 ms of CPU has accumulated, ~150x later.

After the miss, setTimeLimit(oldLimit) resets m_timeLimit to infinity but m_deadline (wall-clock, now in the past) and the pending NeedWatchdogCheck trap bit remain. The next trap check calls shouldTerminate, finds cpuTime < m_cpuDeadline, and calls startTimer(remaining) with hasTimeLimit() false, tripping the assert.

Fix

Replace the JSC Watchdog with a per-evaluation TimeoutWatchdog that spawns a worker thread parked on its own condvar until the wall-clock deadline. On fire it:

• sets vm.setHasTerminationRequest() so waitForSync's loop predicate (!vm.hasTerminationRequest()) falls through and Atomics.wait returns Terminated,
• fires vm.notifyNeedTermination() so running JS terminates at the next back-edge,
• wakes vm.syncWaiter()->condition() (and keeps re-notifying at 1 ms until disarmed, covering the lost-wakeup window where the first notify lands between the waiter's predicate check and park).

Each scope only translates a termination it initiated (its own didFire() or a received SIGINT); a termination from an enclosing scope propagates unchanged. This matches Node's Watchdog semantics and keeps the nested-timeout tests reporting the correct (outer) limit.

clearTerminationState wipes the request flag, the pending termination exception, and the NeedTermination/NeedWatchdogCheck trap bits, so the next timed evaluation starts clean.

This is the same off-thread termination contract as #32802 (which does it for worker.terminate()), applied to node:vm timeout.

Verification

# before
1: returned timed-out after 601ms
3: threw after 11907ms

# after
1: threw ERR_SCRIPT_EXECUTION_TIMEOUT after ~100ms
2: 42
3: threw after ~110ms

• New tests in test/js/node/vm/vm.test.ts cover: single long wait, chunked short waits, runInThisContext, and the follow-on timed evaluation. All four fail on the system bun (three by 5 s test timeout, one by subprocess abort on the assert build) and pass on this branch.
• All 98 test/js/node/test/{parallel,sequential}/test-vm-*.js pass, including test-vm-timeout.js (nested timeouts) and the three test-vm-timeout-escape-promise-module* variants.
• BUN_JSC_validateExceptionChecks=1 clean on the repro and on test-vm-timeout.js.

[robobun]

^{Updated 11:54 AM PT - Jul 8th, 2026}

❌ @robobun, your commit 7051bc3 has 3 failures in Build #70543 (All Failures):

• test/js/sql/sql-prepare-false.test.ts - timeout on 🍎 14 x64
• test/regression/issue/26030.test.ts - code 1 on 🐧 3.23 x64-baseline
• test/regression/issue/26030.test.ts - code 1 on 🐧 3.23 x64
• test/js/bun/http/proxy-stress-concurrent.test.ts - code 1 on 🍎 14 x64

---

🧪   To try this PR locally:

bunx bun-pr 33764

That installs a local version of the PR into your bun-33764 executable, so you can run:

bun-33764 --bun

[github-actions]

This PR may be a duplicate of:

1. node:vm: replace the JSC Watchdog with a cancellable deadline for the timeout option #32773 - Both replace JSC::Watchdog with a custom wall-clock deadline mechanism in node:vm to fix timeout not interrupting Atomics.wait

🤖 Generated with Claude Code

[robobun]

#32773 overlaps on the Watchdog-state half of this (the hasTimeLimit() assert / stale deadline) but does not address the Atomics.wait case that is the primary subject here: its NodeVMEvalTimeout only calls vm.notifyNeedTermination(), which sets the NeedTermination trap bit and notifies the sync waiter once via requestThreadStopIfNeeded, but WaiterListManager::waitForSync re-checks vm.hasTerminationRequest() (the boolean, not the trap bit), finds it still false, and goes back to sleep. A single Atomics.wait(ia, 0, 0, 600) under { timeout: 60 } still runs to completion on that branch.

This PR's TimeoutWatchdog additionally sets vm.setHasTerminationRequest() and keeps re-notifying vm.syncWaiter()->condition() until disarmed, so a blocked Atomics.wait returns Terminated at the deadline (the same off-thread contract #32802 establishes for worker.terminate()). The "single long wait" and "chunked short waits" tests here fail on both main and the #32773 branch.

Happy to fold the syncWaiter wake into #32773 instead if that one is preferred; either way one of the two should land.

[coderabbitai]

Caution

Review failed

An error occurred during the review process. Please try again later.

Walkthrough

This PR replaces the VM timeout path in node:vm bindings with Bun::TimeoutWatchdog, updates evaluation and termination handling in NodeVMModule and NodeVMScript, and adds tests that verify wall-clock timeout behavior for Atomics.wait.

Changes

VM timeout watchdog refactor

Layer / File(s)	Summary
TimeoutWatchdog class implementation src/jsc/bindings/vm/TimeoutWatchdog.h, src/jsc/bindings/vm/TimeoutWatchdog.cpp	New Bun::TimeoutWatchdog class starts a deadline-based worker thread, supports disarm(), reports didFire(), and clears VM termination state and traps with clearTerminationState(vm).
NodeVMModule evaluation using TimeoutWatchdog src/jsc/bindings/NodeVMModule.cpp	Switches module evaluation and microtask draining to scoped TimeoutWatchdog instances and converts termination into ERR_SCRIPT_EXECUTION_INTERRUPTED or ERR_SCRIPT_EXECUTION_TIMEOUT before clearing VM termination state.
NodeVMScript execution using TimeoutWatchdog src/jsc/bindings/NodeVMScript.cpp	Reworks termination checking around TimeoutWatchdog, constructs and disarms watchdogs around runInContext and scriptRunInThisContext, and removes the old watchdog time-limit save/restore flow.
Wall-clock timeout tests against Atomics.wait test/js/node/vm/vm.test.ts	Adds subprocess-based timeout tests for vm.runInNewContext and vm.runInThisContext that assert Atomics.wait is interrupted by the VM timeout and later evaluations still succeed.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely states the main change: enforcing node:vm timeouts as a wall-clock deadline for Atomics.wait.
Description check	✅ Passed	It clearly describes the problem, fix, and verification; the template headings differ, but the required content is present.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/jsc/bindings/NodeVMModule.cpp (1)

238-256: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Propagate enclosing terminations before marking this module errored.

If an outer watchdog fires while evaluating a dependency, the inner evaluation has didTimeOut == false and falls through to VM_RETURN_IF_EXCEPTION, which stores the raw termination sentinel in m_evaluationException. Return with the pending termination instead, so the initiating scope translates it.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jsc/bindings/NodeVMModule.cpp` around lines 238 - 256, The evaluation
path in NodeVMModule::evaluate is swallowing an enclosing termination when
didTimeOut is false, causing a raw termination sentinel to be stored instead of
propagating to the initiating scope. Update the post-evaluation handling around
scope.exception(), getSigintReceived(), and didTimeOut so any pending
termination from an outer watchdog is returned immediately before
VM_RETURN_IF_EXCEPTION, letting the outer scope translate it consistently
instead of marking the module errored.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/bindings/NodeVMModule.cpp`:
- Around line 101-104: The new exception-scope comments in NodeVMModule::drain
and the similar block at the later exception-handling site are too long and
exceed the 3-line comment limit. Condense each comment to three lines or fewer
by keeping only the essential summary, and move any extra explanation into
surrounding code or a nearby descriptive identifier if needed. Use the existing
exception-scope handling code in NodeVMModule.cpp to locate both comment blocks.
- Around line 244-245: The timeout cleanup in NodeVMModule should only drain
microtasks when there is an actual VM global object to target. Update the code
around nodeVmGlobalObject in the cleanup path to guard the call to
vm.drainMicrotasksForGlobalObject so it runs only when that object exists, while
still always clearing the termination state with
TimeoutWatchdog::clearTerminationState.

In `@src/jsc/bindings/NodeVMScript.cpp`:
- Around line 295-296: The microtask drain in checkForTermination is being
applied too broadly and can clear the caller’s main queue when runInThisContext
is used. Update the logic around vm.drainMicrotasksForGlobalObject so it only
runs for contextified NodeVMGlobalObject instances that own their own microtask
queue, and skip it for the main/caller globalObject while keeping
TimeoutWatchdog::clearTerminationState unchanged.

In `@src/jsc/bindings/vm/TimeoutWatchdog.cpp`:
- Around line 16-20: The new explanatory comments in TimeoutWatchdog should be
shortened to comply with the 3-line comment limit while keeping the same
invariants. Update the comment blocks around vm.ensureTerminationException() and
the other cited sections in TimeoutWatchdog so each explanatory note fits within
three lines without losing the key rationale tied to
throwTerminationException(), VMTraps::handleTraps, and Atomics.wait termination
handling.

In `@src/jsc/bindings/vm/TimeoutWatchdog.h`:
- Around line 11-14: The block comment in TimeoutWatchdog should be trimmed to
fit the 3-line limit while keeping the essential summary of its purpose. Update
the comment above the watchdog definition/constructor to briefly state that it
enforces node:vm timeout by requesting VM termination, waking Atomics.wait, and
joining the worker thread in the destructor. Keep the relevant context tied to
TimeoutWatchdog and JSC::evaluate, but remove extra wording so the comment is no
longer than three lines.

In `@test/js/node/vm/vm.test.ts`:
- Around line 1239-1253: This test does not verify that the first
vm.runInNewContext call actually hit the timeout, because the empty catch
swallows any outcome and lets the test pass even if the watchdog never fired. In
the vm.test.ts case for the “blocked deadline” scenario, assert the first
evaluation fails with the expected timeout/error before running the second
vm.runInNewContext call, so the precondition is confirmed and the test only
exercises the post-timeout path when the watchdog was actually triggered.
- Around line 1181-1195: The subprocess-based VM timeout tests are still running
serially even though they are independent. Update the suite around the existing
describe block and each affected test case to use describe.concurrent and
test.concurrent so the subprocess runs happen in parallel, while keeping the
shared helper run unchanged since it has no mutable shared state.

---

Outside diff comments:
In `@src/jsc/bindings/NodeVMModule.cpp`:
- Around line 238-256: The evaluation path in NodeVMModule::evaluate is
swallowing an enclosing termination when didTimeOut is false, causing a raw
termination sentinel to be stored instead of propagating to the initiating
scope. Update the post-evaluation handling around scope.exception(),
getSigintReceived(), and didTimeOut so any pending termination from an outer
watchdog is returned immediately before VM_RETURN_IF_EXCEPTION, letting the
outer scope translate it consistently instead of marking the module errored.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1a1fc848-ef59-40c1-a3fb-8e73458239b3

📥 Commits

Reviewing files that changed from the base of the PR and between 332f744 and 79dc203.

📒 Files selected for processing (5)

• src/jsc/bindings/NodeVMModule.cpp
• src/jsc/bindings/NodeVMScript.cpp
• src/jsc/bindings/vm/TimeoutWatchdog.cpp
• src/jsc/bindings/vm/TimeoutWatchdog.h
• test/js/node/vm/vm.test.ts

[robobun]

CI status after two runs (build 70539, retrigger 70543): the node:vm / Atomics.wait tests pass on every lane that ran them. The remaining red is unrelated to this diff:

build	lane	test	failure
70539	Windows 2019 x64 / x64-baseline	test/js/sql/postgres-binary-array-bounds.test.ts	ERR_POSTGRES_CONNECTION_REFUSED (mock server port)
70539, 70543	Windows 2019 x64	test/bake/dev-and-prod.test.ts	HMR "render sentinel" timeout (annotated flaky by CI)
70543	macOS 14 x64	test/js/bun/http/proxy-stress-concurrent.test.ts	1-2/1200 requests dropped under stress
70543	Alpine 3.23 x64-baseline	test/regression/issue/26030.test.ts	mysql_plain docker container "not healthy after 1m0s"

None of these touch node:vm, Atomics, or anything in src/jsc/bindings/. The failures are on different lanes in each run, which is the flake signature. 280/287 jobs passed on 70543.

The new vm.test.ts suite and all 98 test/js/node/test/{parallel,sequential}/test-vm-*.js tests are green. Ready for review.