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

src/jsc/bindings/NodeVM.cpp

@@ -530,6 +530,28 @@ static void writeArrowHeaderStack(VM& vm, ErrorInstance* errorInstance, const St
     errorInstance->putDirect(vm, decoratedName, jsBoolean(true), JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::ReadOnly);
 }
 
+bool consumeTermination(JSC::VM& vm)
+{
+    vm.clearHasTerminationRequest();
+    // A parked thread services no traps, so stand down both traps this run's
+    // interrupt machinery can leave pending: the SIGINT watcher's
+    // NeedTermination, and NeedWatchdogCheck from a {timeout} watchdog that
+    // fired mid-park (left stale, it trips Watchdog::startTimer's
+    // ASSERT(hasTimeLimit()); an outer timed run re-arms via its restore).
+    vm.traps().clearTrap(JSC::VMTraps::NeedTermination);
+    vm.traps().clearTrap(JSC::VMTraps::NeedWatchdogCheck);
+    // worker.terminate() may have landed after the caller's scriptAllowed
+    // check, and the clears above must not eat its trap: fireTrap and
+    // clearTrap serialize on the trap-signaling lock, so a stop whose trap we
+    // cleared is visible to this re-check. Re-deliver it.
+    if (!Bun__VmHandle__scriptAllowed(WebCore::clientData(vm)->vmHandle)) [[unlikely]] {
+        vm.setHasTerminationRequest();
+        vm.notifyNeedTermination();
+        return false;
+    }
+    return true;
+}
+
 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,13 @@ 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.
+// Returns false when the VM began stopping (worker.terminate()) while we
+// consumed: the stop was re-delivered and the caller must propagate the
+// termination instead of reporting ERR_SCRIPT_EXECUTION_*.
+[[nodiscard]] bool 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

@@ -107,14 +107,28 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b
                 return {};
             }
             if (vm.hasTerminationRequest() || vm.hasPendingTerminationException()) {
+                // Not armed by this evaluation and not flagged for it: an
+                // outer vm frame's interrupt. Propagate the termination.
+                if (!getSigintReceived() && timeout == 0 && !breakOnSigint) {
+                    if (!vm.hasPendingTerminationException())
+                        vm.throwTerminationException();
+                    return {};
+                }
                 vm.drainMicrotasksForGlobalObject(nodeVmGlobalObject);
                 DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException();
-                vm.clearHasTerminationRequest();
+                if (!NodeVM::consumeTermination(vm)) {
+                    vm.throwTerminationException();
+                    return {};
+                }
                 if (getSigintReceived()) {
                     setSigintReceived(false);
                     throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s);
-                } else {
+                } else if (timeout != 0) {
                     throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s));
+                } else {
+                    // breakOnSigint was armed, but SIGINT raced the watcher
+                    // registration or teardown past the sigintReceived flag.
+                    throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s);
                 }
                 return {};
             }
@@ -253,16 +267,28 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b
         return {};
     }
     if (vm.hasTerminationRequest() || vm.hasPendingTerminationException()) {
+        // Not armed by this evaluation and not flagged for it: an outer vm
+        // frame's interrupt. Propagate the termination.
+        if (!getSigintReceived() && timeout == 0 && !breakOnSigint) {
+            if (!vm.hasPendingTerminationException())
+                vm.throwTerminationException();
+            return {};
+        }
         vm.drainMicrotasksForGlobalObject(nodeVmGlobalObject);
         DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException();
-        vm.clearHasTerminationRequest();
+        if (!NodeVM::consumeTermination(vm)) {
+            vm.throwTerminationException();
+            return {};
+        }
         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 {
-            RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE("vm.SourceTextModule evaluation terminated due neither to SIGINT nor to timeout");
+            // breakOnSigint was armed, but SIGINT raced the watcher
+            // registration or teardown past the sigintReceived flag.
+            throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s);
         }
     } else {
         setSigintReceived(false);

src/jsc/bindings/NodeVMScript.cpp

@@ -307,28 +307,37 @@ void NodeVMScript::destroy(JSCell* cell)
     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
         // 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;
+        // Not armed by this run and not flagged for it: the interrupt belongs
+        // to an outer vm frame (nested runs share the VM), whose own
+        // checkForTermination reports it. Leave the request pending.
+        if (!script->getSigintReceived() && !timeout && !breakOnSigint)
+            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 (!consumeTermination(vm))
+            return false;
         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 {
-            RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE("vm.Script terminated due neither to SIGINT nor to timeout");
+            // breakOnSigint was armed, but SIGINT raced the watcher
+            // registration or teardown, so the paired sigintReceived flag
+            // was not set. It is still an interrupt.
+            throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s);
         }
         return true;
     }
@@ -414,7 +423,7 @@ static JSC::EncodedJSValue runInContext(NodeVMGlobalObject* globalObject, NodeVM
         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 +490,7 @@ JSC_DEFINE_HOST_FUNCTION(scriptRunInThisContext, (JSGlobalObject * globalObject,
         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

@@ -1,6 +1,8 @@
 #include "NodeVM.h"
 #include "SigintWatcher.h"
 
+#include <JavaScriptCore/WaiterListManager.h>
+
 #if OS(WINDOWS)
 #include <windows.h>
 #endif
@@ -207,7 +209,18 @@ 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();
+        // fireTrap only notifies the sync waiter on the first thread-stop
+        // request; if another async trap (a {timeout} watchdog fire) already
+        // requested one, the parked waiter would not be woken. POSIX hides
+        // this behind the trap SignalSender's retry loop, but that is
+        // compiled out on Windows and under usePollingTraps.
+        vm.syncWaiter()->condition().notifyOne();
     }
 
     return true;