From 1b0733661eb1bbd3deebe90043086c6b53f04c76 Mon Sep 17 00:00:00 2001 From: Kai Tamkun Date: Thu, 8 May 2025 19:38:49 -0700 Subject: [PATCH] Support timeouts in SourceTextModule evaluation --- .../bindings/NodeVMSourceTextModule.cpp | 34 ++++++++++++------ src/bun.js/bindings/NodeVMSourceTextModule.h | 2 ++ src/vm/SigintWatcher.cpp | 35 +++++++++++++++++-- src/vm/SigintWatcher.h | 23 ++++++++++-- 4 files changed, 78 insertions(+), 16 deletions(-) diff --git a/src/bun.js/bindings/NodeVMSourceTextModule.cpp b/src/bun.js/bindings/NodeVMSourceTextModule.cpp index 585a121669..610cba73ca 100644 --- a/src/bun.js/bindings/NodeVMSourceTextModule.cpp +++ b/src/bun.js/bindings/NodeVMSourceTextModule.cpp @@ -5,6 +5,7 @@ #include "JSModuleRecord.h" #include "ModuleAnalyzer.h" #include "Parser.h" +#include "Watchdog.h" #include "../vm/SigintWatcher.h" @@ -250,15 +251,17 @@ JSValue NodeVMSourceTextModule::evaluate(JSGlobalObject* globalObject, uint32_t result = record->evaluate(globalObject, jsUndefined(), jsNumber(static_cast(JSGenerator::ResumeMode::NormalMode))); }; - if (timeout != 0 && breakOnSigint) { - // TODO(@heimskr): timeout support - auto holder = SigintWatcher::hold(nodeVmGlobalObject); - run(); - } else if (timeout != 0) { - // TODO(@heimskr): timeout support - run(); - } else if (breakOnSigint) { - auto holder = SigintWatcher::hold(nodeVmGlobalObject); + m_terminatedWithSigint = false; + + if (timeout != 0) { + JSC::JSLockHolder locker(vm); + JSC::Watchdog& dog = vm.ensureWatchdog(); + dog.enteredVM(); + dog.setTimeLimit(WTF::Seconds::fromMilliseconds(timeout)); + } + + if (breakOnSigint) { + auto holder = SigintWatcher::hold(nodeVmGlobalObject, this); run(); } else { run(); @@ -268,15 +271,26 @@ JSValue NodeVMSourceTextModule::evaluate(JSGlobalObject* globalObject, uint32_t scope.clearException(); vm.clearHasTerminationRequest(); status(Status::Errored); - throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); + if (m_terminatedWithSigint) { + m_terminatedWithSigint = false; + throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s); + } else { + throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s)); + } return {}; } + m_terminatedWithSigint = false; RETURN_IF_EXCEPTION(scope, (status(Status::Errored), JSValue {})); status(Status::Evaluated); return result; } +void NodeVMSourceTextModule::sigintReceived() +{ + m_terminatedWithSigint = true; +} + JSObject* NodeVMSourceTextModule::createPrototype(VM& vm, JSGlobalObject* globalObject) { return NodeVMModulePrototype::create(vm, NodeVMModulePrototype::createStructure(vm, globalObject, globalObject->objectPrototype())); diff --git a/src/bun.js/bindings/NodeVMSourceTextModule.h b/src/bun.js/bindings/NodeVMSourceTextModule.h index b1059346b9..e3d8b35397 100644 --- a/src/bun.js/bindings/NodeVMSourceTextModule.h +++ b/src/bun.js/bindings/NodeVMSourceTextModule.h @@ -33,6 +33,7 @@ public: JSValue createModuleRecord(JSGlobalObject* globalObject); JSValue link(JSGlobalObject* globalObject, JSArray* specifiers, JSArray* moduleNatives); JSValue evaluate(JSGlobalObject* globalObject, uint32_t timeout, bool breakOnSigint); + void sigintReceived(); DECLARE_EXPORT_INFO; DECLARE_VISIT_CHILDREN; @@ -40,6 +41,7 @@ public: private: WriteBarrier m_moduleRecord; SourceCode m_sourceCode; + bool m_terminatedWithSigint = false; NodeVMSourceTextModule(JSC::VM& vm, JSC::Structure* structure, WTF::String identifier, JSValue context, SourceCode sourceCode) : Base(vm, structure, WTFMove(identifier), context) diff --git a/src/vm/SigintWatcher.cpp b/src/vm/SigintWatcher.cpp index 0e7adf48d0..2b6d8cd769 100644 --- a/src/vm/SigintWatcher.cpp +++ b/src/vm/SigintWatcher.cpp @@ -1,4 +1,5 @@ #include "NodeVM.h" +#include "NodeVMSourceTextModule.h" #include "SigintWatcher.h" extern "C" void Bun__onPosixSignal(int signalNumber); @@ -96,8 +97,7 @@ void SigintWatcher::registerGlobalObject(NodeVMGlobalObject* globalObject) } std::unique_lock lock(m_globalObjectsMutex); - - m_globalObjects.append(globalObject); + m_globalObjects.appendIfNotContains(globalObject); } void SigintWatcher::unregisterGlobalObject(NodeVMGlobalObject* globalObject) @@ -109,7 +109,6 @@ void SigintWatcher::unregisterGlobalObject(NodeVMGlobalObject* globalObject) std::unique_lock lock(m_globalObjectsMutex); auto iter = std::find(m_globalObjects.begin(), m_globalObjects.end(), globalObject); - if (iter == m_globalObjects.end()) { return; } @@ -118,6 +117,29 @@ void SigintWatcher::unregisterGlobalObject(NodeVMGlobalObject* globalObject) m_globalObjects.removeLast(); } +void SigintWatcher::registerModule(NodeVMSourceTextModule* module) +{ + if (module == nullptr) { + return; + } + + std::unique_lock lock(m_modulesMutex); + m_modules.appendIfNotContains(module); +} + +void SigintWatcher::unregisterModule(NodeVMSourceTextModule* module) +{ + std::unique_lock lock(m_modulesMutex); + + auto iter = std::find(m_modules.begin(), m_modules.end(), module); + if (iter == m_modules.end()) { + return; + } + + std::swap(*iter, m_modules.last()); + m_modules.removeLast(); +} + void SigintWatcher::ref() { if (m_refCount++ == 0) { @@ -140,6 +162,13 @@ SigintWatcher& SigintWatcher::get() bool SigintWatcher::signalAll() { + { + std::unique_lock lock(m_modulesMutex); + for (NodeVMSourceTextModule* module : m_modules) { + module->sigintReceived(); + } + } + std::unique_lock lock(m_globalObjectsMutex); if (m_globalObjects.isEmpty()) { diff --git a/src/vm/SigintWatcher.h b/src/vm/SigintWatcher.h index 2444859fc5..6bbb798187 100644 --- a/src/vm/SigintWatcher.h +++ b/src/vm/SigintWatcher.h @@ -11,6 +11,7 @@ namespace Bun { class NodeVMGlobalObject; +class NodeVMSourceTextModule; class SigintWatcher { public: @@ -22,6 +23,8 @@ public: void signalReceived(); void registerGlobalObject(NodeVMGlobalObject* globalObject); void unregisterGlobalObject(NodeVMGlobalObject* globalObject); + void registerModule(NodeVMSourceTextModule* module); + void unregisterModule(NodeVMSourceTextModule* module); /** Installs the signal handler if it's not already installed and increments the ref count. */ void ref(); /** Decrements the ref count and uninstalls the signal handler if the ref count reaches 0. */ @@ -31,17 +34,26 @@ public: class GlobalObjectHolder { public: - GlobalObjectHolder(NodeVMGlobalObject* globalObject) + GlobalObjectHolder(NodeVMGlobalObject* globalObject, NodeVMSourceTextModule* module) : m_globalObject(globalObject) + , m_module(module) { if (m_globalObject) { get().ref(); get().registerGlobalObject(globalObject); } + + if (m_module) { + get().registerModule(m_module); + } } ~GlobalObjectHolder() { + if (m_module) { + get().unregisterModule(m_module); + } + if (m_globalObject) { get().unregisterGlobalObject(m_globalObject); get().deref(); @@ -51,6 +63,7 @@ public: GlobalObjectHolder(const GlobalObjectHolder&) = delete; GlobalObjectHolder(GlobalObjectHolder&& other) : m_globalObject(std::exchange(other.m_globalObject, nullptr)) + , m_module(std::exchange(other.m_module, nullptr)) { } @@ -58,16 +71,18 @@ public: GlobalObjectHolder& operator=(GlobalObjectHolder&& other) { m_globalObject = std::exchange(other.m_globalObject, nullptr); + m_module = std::exchange(other.m_module, nullptr); return *this; } private: NodeVMGlobalObject* m_globalObject = nullptr; + NodeVMSourceTextModule* m_module = nullptr; }; - static GlobalObjectHolder hold(NodeVMGlobalObject* globalObject) + static GlobalObjectHolder hold(NodeVMGlobalObject* globalObject, NodeVMSourceTextModule* module) { - return { globalObject }; + return { globalObject, module }; } private: @@ -76,7 +91,9 @@ private: std::atomic_flag m_waiting = false; Semaphore m_semaphore; std::mutex m_globalObjectsMutex; + std::mutex m_modulesMutex; WTF::Vector m_globalObjects; + WTF::Vector m_modules; uint32_t m_refCount = 0; bool signalAll();