Skip to content

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

Open
robobun wants to merge 4 commits into
mainfrom
claude/farm/1771cc13/vm-timeout-atomics-wait
Open

node:vm: enforce timeout as a wall-clock deadline that interrupts Atomics.wait#33764
robobun wants to merge 4 commits into
mainfrom
claude/farm/1771cc13/vm-timeout-atomics-wait

review: trim comments to 3 lines, guard drainMicrotasksForGlobalObjec…

000b5e4
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 8, 2026 in 26m 57s

Code review found 2 potential issues

Found 1 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 1
Severity File:Line Issue
🟣 Pre-existing src/jsc/bindings/vm/TimeoutWatchdog.cpp:61-69 breakOnSigint does not interrupt Atomics.wait (same bug class, sibling SigintWatcher)

Annotations

Check notice on line 69 in src/jsc/bindings/vm/TimeoutWatchdog.cpp

See this annotation in the file changed.

@claude claude / Claude Code Review

breakOnSigint does not interrupt Atomics.wait (same bug class, sibling SigintWatcher)

Pre-existing / follow-up: `SigintWatcher::signalAll()` (src/jsc/bindings/vm/SigintWatcher.cpp:194-214) has the identical Atomics.wait blind spot this PR fixes for `timeout` — it only calls `vm.notifyNeedTermination()`, never `setHasTerminationRequest()` or re-wakes `syncWaiter()`, so `{ breakOnSigint: true }` without a `timeout` still won't interrupt a guest parked in `Atomics.wait`. Not a blocker (SigintWatcher.cpp is untouched here and the fix isn't a pure copy — it's a process-global singleto