node:vm: make breakOnSigint interrupt a script parked in Atomics.wait

src/jsc/bindings/NodeVM.cpp

@@ -530,6 +530,12 @@
     errorInstance->putDirect(vm, decoratedName, jsBoolean(true), JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::ReadOnly);
 }
 
+void consumeTermination(JSC::VM& vm)
+{
+    vm.clearHasTerminationRequest();
+    vm.traps().clearTrap(JSC::VMTraps::NeedTermination);
+}
+
 bool handleException(JSGlobalObject* globalObject, VM& vm, NakedPtr<JSC::Exception> exception, ThrowScope& throwScope)
 {
     if (auto* errorInstance = dynamicDowncast<ErrorInstance>(exception->value())) {

src/jsc/bindings/NodeVM.h

@@ -30,6 +30,10 @@ bool extractCachedData(JSValue cachedDataValue, WTF::Vector<uint8_t>& outCachedD
 String stringifyAnonymousFunction(JSGlobalObject* globalObject, const ArgList& args, ThrowScope& scope, int* outOffset);
 JSC::EncodedJSValue createCachedData(JSGlobalObject* globalObject, const JSC::SourceCode& source);
 bool handleException(JSGlobalObject* globalObject, VM& vm, NakedPtr<JSC::Exception> exception, ThrowScope& throwScope);
+// Clear a termination node:vm consumed (breakOnSigint/timeout). The request
+// flag alone is insufficient: a termination that interrupted Atomics.wait
+// never ran handleTraps, and the pending trap would re-terminate the VM.
+void consumeTermination(JSC::VM& vm);
 // `url` must be caller-resolved: `new Script` falls back to evalmachine.<anonymous>
 // when no filename was provided; compileFunction has no such default.
 void decorateParseErrorStack(JSGlobalObject* globalObject, VM& vm, JSObject* error, StringView sourceString, const String& url, const JSC::ParserError& parseError, OrdinalNumber lineOffset);

src/jsc/bindings/NodeVMModule.cpp

@@ -109,12 +109,16 @@ 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();
+                NodeVM::consumeTermination(vm);
                 if (getSigintReceived()) {
                     setSigintReceived(false);
                     throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s);
-                } else {
+                } else if (timeout != 0 || !breakOnSigint) {
                     throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s));
+                } else {
+                    // SIGINT raced the watcher registration or teardown, setting
+                    // the request without the paired sigintReceived flag.
+                    throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s);
                 }
                 return {};
             }
@@ -255,12 +259,16 @@ 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();
+        NodeVM::consumeTermination(vm);
         if (getSigintReceived()) {
             setSigintReceived(false);
             throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s);
         } else if (timeout != 0) {
             throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s));
+        } else if (breakOnSigint) {
+            // SIGINT raced the watcher registration or teardown, setting the
+            // request without the paired sigintReceived flag.
+            throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s);
         } else {
             RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE("vm.SourceTextModule evaluation terminated due neither to SIGINT nor to timeout");
         }

src/jsc/bindings/NodeVMScript.cpp

@@ -307,7 +307,7 @@
     static_cast<NodeVMScript*>(cell)->NodeVMScript::~NodeVMScript();
 }
 
-static bool checkForTermination(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::ThrowScope& scope, NodeVMScript* script, std::optional<double> timeout)
+static bool checkForTermination(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::ThrowScope& scope, NodeVMScript* script, std::optional<double> timeout, bool breakOnSigint)
 {
     if (vm.hasTerminationRequest()) {
         // The whole VM is being stopped (worker terminate()/exit): that
@@ -321,14 +321,19 @@
         // the ERR_SCRIPT_EXECUTION_* error below replaces it.
         if (vm.hasPendingTerminationException())
             DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException();
-        vm.clearHasTerminationRequest();
+        consumeTermination(vm);
         if (script->getSigintReceived()) {
             script->setSigintReceived(false);
             throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s);
         } else if (timeout) {
             throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, *timeout, "ms"_s));
+        } else if (breakOnSigint) {
+            // SIGINT raced the watcher registration or teardown: the watcher
+            // observed the global but not the script receiver, so the request
+            // was set without the paired sigintReceived flag.
+            throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s);
         } else {
             RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE("vm.Script terminated due neither to SIGINT nor to timeout");
         }
         return true;
     }
@@ -414,7 +419,7 @@
         vm.watchdog()->setTimeLimit(WTF::Seconds::fromMilliseconds(*oldLimit));
     }
 
-    if (checkForTermination(vm, globalObject, scope, script, newLimit)) {
+    if (checkForTermination(vm, globalObject, scope, script, newLimit, options.breakOnSigint)) {
         return {};
     }
 
@@ -481,7 +486,7 @@
         vm.watchdog()->setTimeLimit(WTF::Seconds::fromMilliseconds(*oldLimit));
     }
 
-    if (checkForTermination(vm, globalObject, scope, script, newLimit)) {
+    if (checkForTermination(vm, globalObject, scope, script, newLimit, options.breakOnSigint)) {
         return {};
     }
 

src/jsc/bindings/vm/SigintWatcher.cpp

@@ -207,7 +207,12 @@ bool SigintWatcher::signalAll()
     }
 
     for (JSGlobalObject* globalObject : m_globalObjects) {
-        globalObject->vm().notifyNeedTermination();
+        JSC::VM& vm = globalObject->vm();
+        // Atomics.wait's park loop (WaiterListManager::waitForSync) only exits
+        // on hasTerminationRequest(); the NeedTermination trap wakes the waiter
+        // but is serviced only at safepoints, which a parked thread never reaches.
+        vm.setHasTerminationRequest();
+        vm.notifyNeedTermination();
     }
 
     return true;

test/js/node/vm/vm.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from "bun:test";
-import { bunEnv, bunExe, normalizeBunSnapshot } from "harness";
+import { bunEnv, bunExe, isWindows, normalizeBunSnapshot } from "harness";
 import {
   compileFunction,
   constants,
@@ -1484,3 +1484,92 @@ test("node:vm Object.defineProperty on the context global when the sandbox is an
   expect(stdout.trim()).toBe(JSON.stringify({ result: 1, sandboxArray: 1 }));
   expect(exitCode).toBe(0);
 });
+
+// breakOnSigint must interrupt a guest parked in Atomics.wait. The SIGINT
+// watcher thread fires the NeedTermination trap, which wakes the VM's sync
+// waiter, but JSC's park loop re-parks unless vm.hasTerminationRequest() is
+// set, and no safepoint inside the futex wait ever sets it. The watcher now
+// sets the request itself, and the vm entry points clear the unserviced trap
+// afterward so the surviving thread is not re-terminated at its next trap
+// check.
+describe.skipIf(isWindows)("breakOnSigint interrupts Atomics.wait", () => {
+  // Atomics.notify returning 1 proves the main thread is parked; the short
+  // wait lets it re-enter the park before SIGINT is sent.
+  const workerSource = `
+    const { workerData } = require("node:worker_threads");
+    const ia = new Int32Array(workerData);
+    while (Atomics.notify(ia, 0, 1) === 0) {}
+    Atomics.wait(ia, 1, 0, 100);
+    process.kill(process.pid, "SIGINT");
+  `;
+
+  // The loop and call after catching are trap checks: they would rethrow the
+  // termination if the NeedTermination trap were left pending.
+  const proveStillAlive = `
+    let n = 0;
+    for (let i = 0; i < 100000; i++) n++;
+    (function alive() { console.log("alive", n === 100000); })();
+  `;
+
+  async function run(fixture: string) {
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", fixture],
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    return await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  }
+
+  test.concurrent(
+    "script",
+    async () => {
+      const fixture = `
+      const vm = require("node:vm");
+      const { Worker } = require("node:worker_threads");
+      const sab = new SharedArrayBuffer(8);
+      globalThis.ia = new Int32Array(sab);
+      new Worker(${JSON.stringify(workerSource)}, { eval: true, workerData: sab }).unref();
+      try {
+        vm.runInThisContext("for(;;) Atomics.wait(ia, 0, 0);", { breakOnSigint: true });
+        console.log("returned");
+      } catch (e) {
+        console.log("caught", e.code);
+      }
+      ${proveStillAlive}
+    `;
+      const [stdout, stderr, exitCode] = await run(fixture);
+      expect(stdout).toBe("caught ERR_SCRIPT_EXECUTION_INTERRUPTED\nalive true\n");
+      expect(stderr).toBe("");
+      expect(exitCode).toBe(0);
+    },
+    20_000,
+  );
+
+  test.concurrent(
+    "source text module",
+    async () => {
+      const fixture = `
+      const vm = require("node:vm");
+      const { Worker } = require("node:worker_threads");
+      const sab = new SharedArrayBuffer(8);
+      const context = vm.createContext({ ia: new Int32Array(sab) });
+      new Worker(${JSON.stringify(workerSource)}, { eval: true, workerData: sab }).unref();
+      const mod = new vm.SourceTextModule("for(;;) Atomics.wait(ia, 0, 0);", { context });
+      await mod.link(() => { throw new Error("unexpected import"); });
+      try {
+        await mod.evaluate({ breakOnSigint: true });
+        console.log("returned");
+      } catch (e) {
+        console.log("caught", e.code);
+      }
+      ${proveStillAlive}
+    `;
+      const [stdout, stderr, exitCode] = await run(fixture);
+      expect(stdout).toBe("caught ERR_SCRIPT_EXECUTION_INTERRUPTED\nalive true\n");
+      expect(stderr).toBe("");
+      expect(exitCode).toBe(0);
+    },
+    20_000,
+  );
+});