Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 26 additions & 33 deletions src/jsc/bindings/NodeVMModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
#include "JavaScriptCore/Exception.h"
#include "JavaScriptCore/JSModuleRecord.h"
#include "JavaScriptCore/JSPromise.h"
#include "JavaScriptCore/Watchdog.h"

#include "../vm/SigintWatcher.h"
#include "../vm/TimeoutWatchdog.h"

namespace Bun {

Expand Down Expand Up @@ -50,8 +50,6 @@ JSArray* NodeVMModuleRequest::toJS(JSGlobalObject* globalObject) const
return array;
}

void setupWatchdog(VM& vm, double timeout, double* oldTimeout, double* newTimeout);

void NodeVMModule::reconcileEvaluationState(JSC::VM& vm)
{
if (m_status != Status::Evaluating)
Expand Down Expand Up @@ -93,21 +91,21 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b
NodeVMGlobalObject* nodeVmGlobalObject = NodeVM::getGlobalObjectFromContext(globalObject, m_context.get(), false);
RETURN_IF_EXCEPTION(scope, {});
if (nodeVmGlobalObject && nodeVmGlobalObject->hasOwnMicrotaskQueue()) {
std::optional<double> oldLimit;
if (timeout != 0)
setupWatchdog(vm, timeout, &oldLimit.emplace(), nullptr);
nodeVmGlobalObject->drainOwnMicrotasks();
if (timeout != 0)
vm.watchdog()->setTimeLimit(WTF::Seconds::fromMilliseconds(*oldLimit));
bool didTimeOut = false;
{
TimeoutWatchdog watchdog(vm, timeout != 0 ? std::optional<int64_t>(timeout) : std::nullopt);
nodeVmGlobalObject->drainOwnMicrotasks();
watchdog.disarm();
didTimeOut = watchdog.didFire();
}
// The drain may legitimately leave the termination exception
// pending (watchdog fired mid-checkpoint); observe it so the
// exception-check validator is satisfied before the TOP scope
// below, then convert it to ERR_SCRIPT_EXECUTION_*.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
std::ignore = scope.exception();
if (vm.hasTerminationRequest() || vm.hasPendingTerminationException()) {
if (getSigintReceived() || didTimeOut) {
vm.drainMicrotasksForGlobalObject(nodeVmGlobalObject);
DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException();
vm.clearHasTerminationRequest();
TimeoutWatchdog::clearTerminationState(vm);
if (getSigintReceived()) {
setSigintReceived(false);
throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s);
Expand Down Expand Up @@ -220,41 +218,36 @@ JSValue NodeVMModule::evaluate(JSGlobalObject* globalObject, uint32_t timeout, b

setSigintReceived(false);

std::optional<double> oldLimit, newLimit;
bool didTimeOut = false;
{
TimeoutWatchdog watchdog(vm, timeout != 0 ? std::optional<int64_t>(timeout) : std::nullopt);

if (timeout != 0) {
setupWatchdog(vm, timeout, &oldLimit.emplace(), &newLimit.emplace());
}

if (breakOnSigint) {
auto holder = SigintWatcher::hold(nodeVmGlobalObject, this);
run();
drainAfterEvaluate();
} else {
run();
drainAfterEvaluate();
}
if (breakOnSigint) {
auto holder = SigintWatcher::hold(nodeVmGlobalObject, this);
run();
drainAfterEvaluate();
} else {
run();
drainAfterEvaluate();
}

if (timeout != 0) {
vm.watchdog()->setTimeLimit(WTF::Seconds::fromMilliseconds(*oldLimit));
watchdog.disarm();
didTimeOut = watchdog.didFire();
}

// Evaluation (or the afterEvaluate drain) may leave an exception pending
// — a regular one is rethrown by VM_RETURN_IF_EXCEPTION below, a
// termination one is converted to ERR_SCRIPT_EXECUTION_* here. Observe it
// so the exception-check validator is satisfied before the TOP scope.
std::ignore = scope.exception();
if (vm.hasTerminationRequest() || vm.hasPendingTerminationException()) {
if (getSigintReceived() || didTimeOut) {
vm.drainMicrotasksForGlobalObject(nodeVmGlobalObject);
DECLARE_TOP_EXCEPTION_SCOPE(vm).clearException();
vm.clearHasTerminationRequest();
TimeoutWatchdog::clearTerminationState(vm);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
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");
throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, timeout, "ms"_s));
}
} else {
setSigintReceived(false);
Expand Down
88 changes: 26 additions & 62 deletions src/jsc/bindings/NodeVMScript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include "NodeVMScriptFetcher.h"
#include "../vm/SigintWatcher.h"
#include "../vm/TimeoutWatchdog.h"

#include <bit>

Expand Down Expand Up @@ -281,51 +282,26 @@ 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<int64_t> timeout, bool didTimeOut)
{
if (vm.hasTerminationRequest()) {
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 (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");
}
return true;
}

return false;
}

void setupWatchdog(VM& vm, double timeout, double* oldTimeout, double* newTimeout)
{
JSC::JSLockHolder locker(vm);
JSC::Watchdog& dog = vm.ensureWatchdog();
dog.enteredVM();

Seconds oldLimit = dog.getTimeLimit();

if (oldTimeout) {
*oldTimeout = oldLimit.milliseconds();
}

if (oldLimit.isInfinity() || timeout < oldLimit.milliseconds()) {
dog.setTimeLimit(WTF::Seconds::fromMilliseconds(timeout));
bool sigint = script->getSigintReceived();
if (!sigint && !didTimeOut) {
// A termination that this scope did not initiate (an enclosing
// node:vm timeout, or worker termination) propagates unchanged so the
// outer scope can translate it with its own timeout value.
return false;
}

vm.drainMicrotasksForGlobalObject(globalObject);
TimeoutWatchdog::clearTerminationState(vm);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
if (sigint) {
script->setSigintReceived(false);
throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_INTERRUPTED, "Script execution was interrupted by `SIGINT`"_s);
} else {
timeout = oldLimit.milliseconds();
}

if (newTimeout) {
*newTimeout = timeout;
ASSERT(timeout);
throwError(globalObject, scope, ErrorCode::ERR_SCRIPT_EXECUTION_TIMEOUT, makeString("Script execution timed out after "_s, *timeout, "ms"_s));
}
return true;
}

static JSC::EncodedJSValue runInContext(NodeVMGlobalObject* globalObject, NodeVMScript* script, JSObject* contextifiedObject, JSValue optionsArg, bool allowStringInPlaceOfOptions = false)
Expand Down Expand Up @@ -354,12 +330,6 @@ static JSC::EncodedJSValue runInContext(NodeVMGlobalObject* globalObject, NodeVM
result = JSC::evaluate(globalObject, script->source(), globalObject, exception);
};

std::optional<double> oldLimit, newLimit;

if (options.timeout) {
setupWatchdog(vm, *options.timeout, &oldLimit.emplace(), &newLimit.emplace());
}

script->setSigintReceived(false);

// Node performs the afterEvaluate microtask checkpoint inside the
Expand All @@ -370,6 +340,8 @@ static JSC::EncodedJSValue runInContext(NodeVMGlobalObject* globalObject, NodeVM
globalObject->drainOwnMicrotasks();
};

TimeoutWatchdog watchdog(vm, options.timeout);

if (options.breakOnSigint) {
auto holder = SigintWatcher::hold(globalObject, script);
run();
Expand All @@ -379,11 +351,9 @@ static JSC::EncodedJSValue runInContext(NodeVMGlobalObject* globalObject, NodeVM
drainAfterEvaluate();
}

if (options.timeout) {
vm.watchdog()->setTimeLimit(WTF::Seconds::fromMilliseconds(*oldLimit));
}
watchdog.disarm();

if (checkForTermination(vm, globalObject, scope, script, newLimit)) {
if (checkForTermination(vm, globalObject, scope, script, options.timeout, watchdog.didFire())) {
return {};
}

Expand Down Expand Up @@ -428,14 +398,10 @@ JSC_DEFINE_HOST_FUNCTION(scriptRunInThisContext, (JSGlobalObject * globalObject,
result = JSC::evaluate(globalObject, script->source(), globalObject, exception);
};

std::optional<double> oldLimit, newLimit;

if (options.timeout) {
setupWatchdog(vm, *options.timeout, &oldLimit.emplace(), &newLimit.emplace());
}

script->setSigintReceived(false);

TimeoutWatchdog watchdog(vm, options.timeout);

if (options.breakOnSigint) {
auto holder = SigintWatcher::hold(globalObject, script);
vm.ensureTerminationException();
Expand All @@ -444,11 +410,9 @@ JSC_DEFINE_HOST_FUNCTION(scriptRunInThisContext, (JSGlobalObject * globalObject,
run();
}

if (options.timeout) {
vm.watchdog()->setTimeLimit(WTF::Seconds::fromMilliseconds(*oldLimit));
}
watchdog.disarm();

if (checkForTermination(vm, globalObject, scope, script, newLimit)) {
if (checkForTermination(vm, globalObject, scope, script, options.timeout, watchdog.didFire())) {
return {};
}

Expand Down
91 changes: 91 additions & 0 deletions src/jsc/bindings/vm/TimeoutWatchdog.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
#include "TimeoutWatchdog.h"

#include <JavaScriptCore/VM.h>
#include <JavaScriptCore/VMTraps.h>
#include <JavaScriptCore/WaiterListManager.h>
#include <JavaScriptCore/ExceptionScope.h>

namespace Bun {

TimeoutWatchdog::TimeoutWatchdog(JSC::VM& vm, std::optional<int64_t> timeoutMs)
: m_vm(vm)
{
if (!timeoutMs)
return;

// The worker thread requests termination via throwTerminationException()
// (reached from VMTraps::handleTraps / Atomics.wait Terminated). That
// path asserts the lazily-allocated termination exception exists, so
// create it now on the mutator thread.
vm.ensureTerminationException();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

auto deadline = MonotonicTime::now() + Seconds::fromMilliseconds(static_cast<double>(*timeoutMs));
m_thread = WTF::Thread::create("node:vm timeout"_s, [this, deadline] {
Locker locker { m_lock };
while (!m_disarmed) {
if (m_cond.waitUntil(m_lock, deadline))
continue;
if (m_disarmed)
return;
fire();
// Keep nudging the sync waiter until the mutator disarms us:
// the first notify can be lost if it lands between the waiter
// evaluating its loop predicate and parking (we do not hold the
// waiter's list lock).
while (!m_disarmed) {
if (m_cond.waitUntil(m_lock, MonotonicTime::now() + 1_ms))
continue;
m_vm.syncWaiter()->condition().notifyOne();
}

Check warning on line 39 in src/jsc/bindings/vm/TimeoutWatchdog.cpp

View check run for this annotation

Claude / Claude Code Review

Nudge loop only wakes syncWaiter; a nested clearTerminationState() can neuter an already-fired outer watchdog

Nit: after `fire()`, this nudge loop only re-notifies the sync waiter — it never re-asserts `setHasTerminationRequest()` / `notifyNeedTermination()`. If a nested inner `node:vm` scope's own watchdog fires and its `clearTerminationState()` runs after this outer watchdog has already fired, the inner clear wipes the outer's `hasTerminationRequest` flag and `NeedTermination` trap; the outer script can then catch the inner's `ERR_SCRIPT_EXECUTION_TIMEOUT` and enter a CPU-bound loop that this notify (
return;
}
});
}
Comment thread
robobun marked this conversation as resolved.

TimeoutWatchdog::~TimeoutWatchdog()
{
disarm();
}

void TimeoutWatchdog::disarm()
{
if (!m_thread)
return;
{
Locker locker { m_lock };
m_disarmed = true;
}
m_cond.notifyOne();
m_thread->waitForCompletion();
m_thread = nullptr;
}

void TimeoutWatchdog::fire()
{
m_fired.store(true, std::memory_order_release);
// Mark the VM as terminating so WaiterListManager::waitForSync's loop
// predicate (!vm.hasTerminationRequest()) falls through and a blocked
// Atomics.wait returns Terminated. m_hasTerminationRequest is a plain
// bool but the notify below is a full fence and the woken waiter
// reacquires its list lock (acquire), so the write is observable.
m_vm.setHasTerminationRequest();
// Raise NeedTermination so running JS exits at the next back-edge. This
// path also reaches requestThreadStopIfNeeded which notifies the sync
// waiter, but that notify is skipped when a thread-stop is already
// pending, so deliver one unconditionally here as well.
m_vm.notifyNeedTermination();
Comment thread
robobun marked this conversation as resolved.
m_vm.syncWaiter()->condition().notifyOne();
}

void TimeoutWatchdog::clearTerminationState(JSC::VM& vm)
{
if (vm.hasPendingTerminationException()) {
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
scope.clearException();
}
vm.clearHasTerminationRequest();
vm.traps().clearTrap(JSC::VMTraps::NeedTermination);
vm.traps().clearTrap(JSC::VMTraps::NeedWatchdogCheck);
}

} // namespace Bun
41 changes: 41 additions & 0 deletions src/jsc/bindings/vm/TimeoutWatchdog.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#pragma once

#include "root.h"

#include <wtf/Condition.h>
#include <wtf/Lock.h>
#include <wtf/Threading.h>

namespace Bun {

// Wall-clock watchdog for node:vm's `timeout` option. On fire it requests VM
// termination and wakes a blocked Atomics.wait so the guest cannot sit out the
// deadline in a futex. The destructor joins the worker thread, so the watchdog
// is always armed on the stack bracketing JSC::evaluate.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
class TimeoutWatchdog {
WTF_MAKE_NONCOPYABLE(TimeoutWatchdog);

public:
TimeoutWatchdog(JSC::VM& vm, std::optional<int64_t> timeoutMs);
~TimeoutWatchdog();

void disarm();
bool didFire() const { return m_fired.load(std::memory_order_acquire); }

// Clears the VM's termination state (request flag + trap bits + pending
// exception) that this watchdog installed when it fired. Call from the
// mutator thread after evaluate returns.
static void clearTerminationState(JSC::VM&);

private:
void fire();

JSC::VM& m_vm;
WTF::Lock m_lock;
WTF::Condition m_cond;
std::atomic<bool> m_fired { false };
bool m_disarmed WTF_GUARDED_BY_LOCK(m_lock) { false };
RefPtr<WTF::Thread> m_thread;
};

} // namespace Bun
Loading
Loading