Skip to content
Closed
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
5 changes: 5 additions & 0 deletions src/jsc/bindings/BunObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <JavaScriptCore/JSPromise.h>
#include <JavaScriptCore/JSBase.h>
#include <JavaScriptCore/BuiltinNames.h>
#include <JavaScriptCore/DeferTermination.h>
#include "ScriptExecutionContext.h"
#include "WebCoreJSClientData.h"
#include <JavaScriptCore/JSFunction.h>
Expand Down Expand Up @@ -356,9 +357,13 @@

return fetchFn;
}

static JSValue constructBunShell(VM& vm, JSObject* bunObject)
{
// Lazy property builder: exceptions must not propagate into
// reifyStaticProperty, which performs no exception check. A
// TerminationException cannot be cleared, so defer it across the call.
JSC::DeferTerminationForAWhile deferTermination(vm);

Check failure on line 366 in src/jsc/bindings/BunObject.cpp

View check run for this annotation

Claude / Claude Code Review

Bun.sql/Bun.SQL/Bun.postgres lazy builders also enter JS and need DeferTerminationForAWhile

`Bun.sql`, `Bun.postgres` (`defaultBunSQLObject`) and `Bun.SQL` (`constructBunSQLObject`) also enter JS via `requireId()` → `generateModule()` → `JSC::profiledCall` on first touch, and both use `DECLARE_THROW_SCOPE` + `RETURN_IF_EXCEPTION(scope, {})` — the same shape as `constructBunShell` pre-fix. The PR description says the remaining `PropertyCallback` builders "only allocate", but these three don't; they need the same `DeferTerminationForAWhile` guard (and ideally a slot in the `lazyPropertie
Comment thread
claude[bot] marked this conversation as resolved.
auto* globalObject = uncheckedDowncast<Zig::GlobalObject>(bunObject->globalObject());
JSFunction* createParsedShellScript = JSFunction::create(vm, bunObject->globalObject(), 2, "createParsedShellScript"_s, BunObject_callback_createParsedShellScript, ImplementationVisibility::Private, NoIntrinsic);
JSFunction* createShellInterpreterFunction = JSFunction::create(vm, bunObject->globalObject(), 1, "createShellInterpreter"_s, BunObject_callback_createShellInterpreter, ImplementationVisibility::Private, NoIntrinsic);
Expand Down
21 changes: 18 additions & 3 deletions src/jsc/bindings/BunProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "ErrorCode+List.h"
#include "JavaScriptCore/ArgList.h"
#include "JavaScriptCore/CallData.h"
#include "JavaScriptCore/DeferTermination.h"
#include "JavaScriptCore/TopExceptionScope.h"
#include "JavaScriptCore/JSCJSValue.h"
#include "JavaScriptCore/JSCast.h"
Expand Down Expand Up @@ -2652,6 +2653,10 @@ extern "C" void Bun__ForceFileSinkToBeSynchronousForProcessObjectStdio(JSC::JSGl
static JSValue constructStdioWriteStream(JSC::JSGlobalObject* globalObject, JSC::JSObject* processObject, int fd)
{
auto& vm = JSC::getVM(globalObject);
// Lazy property builder: exceptions must not propagate into
// reifyStaticProperty, which performs no exception check. A
// TerminationException cannot be cleared, so defer it across the call.
JSC::DeferTerminationForAWhile deferTermination(vm);
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);

JSC::JSFunction* getStdioWriteStream = JSC::JSFunction::create(vm, globalObject, processObjectInternalsGetStdioWriteStreamCodeGenerator(vm), globalObject);
Expand Down Expand Up @@ -2715,6 +2720,10 @@ static JSValue constructStderr(VM& vm, JSObject* processObject)
static JSValue constructStdin(VM& vm, JSObject* processObject)
{
auto* globalObject = processObject->globalObject();
// Lazy property builder: exceptions must not propagate into
// reifyStaticProperty, which performs no exception check. A
// TerminationException cannot be cleared, so defer it across the call.
JSC::DeferTerminationForAWhile deferTermination(vm);
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
JSC::JSFunction* getStdinStream = JSC::JSFunction::create(vm, globalObject, processObjectInternalsGetStdinStreamCodeGenerator(vm), globalObject);
JSC::MarkedArgumentBuffer args;
Expand Down Expand Up @@ -2783,7 +2792,9 @@ static JSValue constructProcessChannel(VM& vm, JSObject* processObject)
if (Bun__GlobalObject__hasIPC(globalObject)) {
auto& vm = JSC::getVM(globalObject);
// Lazy property builder: exceptions must not propagate into
// reifyStaticProperty, which performs no exception check.
// reifyStaticProperty, which performs no exception check. A
// TerminationException cannot be cleared, so defer it across the call.
JSC::DeferTerminationForAWhile deferTermination(vm);
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);

JSC::JSFunction* getControl = JSC::JSFunction::create(vm, globalObject, processObjectInternalsGetChannelCodeGenerator(vm), globalObject);
Expand Down Expand Up @@ -3979,7 +3990,9 @@ extern "C" void Bun__Process__queueNextTick2(GlobalObject* globalObject, Encoded
static JSValue constructMainModuleProperty(VM& vm, JSObject* processObject)
{
// Lazy property builder: exceptions must not propagate into
// reifyStaticProperty, which performs no exception check.
// reifyStaticProperty, which performs no exception check. A
// TerminationException cannot be cleared, so defer it across the gets.
JSC::DeferTerminationForAWhile deferTermination(vm);
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
auto* globalObject = defaultGlobalObject(processObject->globalObject());
auto* bun = globalObject->bunObject();
Expand Down Expand Up @@ -4019,7 +4032,9 @@ JSValue Process::constructNextTickFn(JSC::VM& vm, Zig::GlobalObject* globalObjec
args.append(JSC::JSFunction::create(vm, globalObject, 1, String(), jsFunctionReportUncaughtException, ImplementationVisibility::Private));

// Lazy property builder: exceptions must not propagate into
// reifyStaticProperty, which performs no exception check.
// reifyStaticProperty, which performs no exception check. A
// TerminationException cannot be cleared, so defer it across the call.
JSC::DeferTerminationForAWhile deferTermination(vm);
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
JSValue nextTickFunction = JSC::profiledCall(globalObject, ProfilingReason::API, initializer, JSC::getCallData(initializer), globalObject->globalThis(), args);
if (auto* exception = scope.exception()) [[unlikely]] {
Expand Down
60 changes: 60 additions & 0 deletions test/js/web/workers/worker-terminate-lifetime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,66 @@ test(
timeout,
);

// Regression: `process.nextTick`, `process.mainModule`, `process.stdin` and
// `Bun.$` are lazily built by JSC's reifyStaticProperty, which performs no
// exception check. A terminate() that landed while a builder was entering JS
// left a TerminationException pending (tryClearException() cannot clear one),
// and the caller's `EXCEPTION_ASSERT(!scope.exception() || !hasSlot)` aborted.
//
// Each worker blocks in Bun.sleepSync so terminate() is requested while the
// thread sits in native code with no JS safepoint ahead of the property read;
// the builder then runs with the termination trap armed. A blocking call is
// the point of the test, not a wait for a condition.
const lazyProperties = ["process.nextTick", "process.mainModule", "process.stdin", "Bun.$"];
const blockMs = slow ? 600 : 200;

test(
"terminate() while a lazy property builder is entering JS does not abort",
async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const properties = ${JSON.stringify(lazyProperties)};
await Promise.all(
properties.map(property => {
const w = new Worker(
"data:text/javascript," +
encodeURIComponent(
'postMessage("go");' +
// Blocks the worker thread in native code; terminate() arms the
// termination trap while we are parked here.
"Bun.sleepSync(${blockMs});" +
// First touch of the lazy property: its builder enters JS and is
// terminated mid-call.
property + ";",
),
);
const closed = new Promise(resolve => w.addEventListener("close", resolve, { once: true }));
w.addEventListener("message", () => w.terminate(), { once: true });
return closed;
}),
);
console.log("terminated " + properties.length);
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout, stderr, exitCode, signalCode: proc.signalCode }).toEqual({
stdout: `terminated ${lazyProperties.length}\n`,
stderr: "",
exitCode: 0,
signalCode: null,
});
},
timeout,
);

// Regression: WebWorker__dispatchExit deref'd the C++ Worker on the worker
// thread; if that was the last ref, ~Worker → ~EventTarget ran there and
// EventListenerMap::releaseAssertOrSetThreadUID tripped because the listener
Expand Down
Loading