Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 commits
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
2 changes: 1 addition & 1 deletion src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3287,7 +3287,7 @@
) {
use bun_options_types::schema::api::UnhandledRejections as Mode;

if self.is_shutting_down() {
if self.script_execution_status() != crate::ScriptExecutionStatus::Running {

Check warning on line 3290 in src/jsc/VirtualMachine.rs

View check run for this annotation

Claude / Claude Code Review

uncaught_exception's is_shutting_down() guard not widened; isBunTest fast-path bypasses the C++ guard

d260627c widened `unhandled_rejection`'s early-return here to `script_execution_status() != Running`, but the two sibling Rust dispatchers — `uncaught_exception` (VirtualMachine.rs:1351) and `handled_promise` (:1077) — were left on the narrower `is_shutting_down()`. For `handled_promise` and the non-`isBunTest` arm of `uncaught_exception` the new C++ guards cover it, but `uncaught_exception`'s `isBunTest` fast-path at :1355 fires BEFORE the FFI call: `isBunTest` is a process-global static (true
Comment thread
robobun marked this conversation as resolved.
bun_core::debug_warn!("unhandledRejection during shutdown.");
return;
}
Expand Down
11 changes: 10 additions & 1 deletion src/jsc/bindings/BunProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1212,9 +1212,14 @@ extern "C" int Bun__handleUncaughtException(JSC::JSGlobalObject* lexicalGlobalOb
if (!lexicalGlobalObject->inherits(Zig::GlobalObject::info()))
return false;
auto* globalObject = uncheckedDowncast<Zig::GlobalObject>(lexicalGlobalObject);
auto& vm = JSC::getVM(globalObject);

// Stopped == a worker with has_requested_terminate(); the process->get / emit / call walk below asserts under a terminate() race.
if (Zig::GlobalObject::scriptExecutionStatus(globalObject, globalObject) != JSC::ScriptExecutionStatus::Running) [[unlikely]]
return true;
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.

auto* process = globalObject->processObject();
auto& wrapped = process->wrapped();
auto& vm = JSC::getVM(globalObject);

// node parity (exitWithUndefinedFatalException): the internal fatal-exception
// handler is monkey-patchable as process._fatalException. If user code
Expand Down Expand Up @@ -1346,6 +1351,8 @@ extern "C" int Bun__handleUnhandledRejection(JSC::JSGlobalObject* lexicalGlobalO
if (!lexicalGlobalObject->inherits(Zig::GlobalObject::info()))
return false;
auto* globalObject = uncheckedDowncast<Zig::GlobalObject>(lexicalGlobalObject);
if (Zig::GlobalObject::scriptExecutionStatus(globalObject, globalObject) != JSC::ScriptExecutionStatus::Running) [[unlikely]]
return false;
Comment thread
claude[bot] marked this conversation as resolved.
auto* process = globalObject->processObject();

auto eventType = Identifier::fromString(JSC::getVM(globalObject), "unhandledRejection"_s);
Expand All @@ -1369,6 +1376,8 @@ extern "C" bool Bun__emitHandledPromiseEvent(JSC::JSGlobalObject* lexicalGlobalO
if (!lexicalGlobalObject->inherits(Zig::GlobalObject::info()))
return false;
auto* globalObject = uncheckedDowncast<Zig::GlobalObject>(lexicalGlobalObject);
if (Zig::GlobalObject::scriptExecutionStatus(globalObject, globalObject) != JSC::ScriptExecutionStatus::Running) [[unlikely]]
return false;
auto* process = globalObject->processObject();

auto eventType = Identifier::fromString(JSC::getVM(globalObject), "rejectionHandled"_s);
Expand Down
6 changes: 6 additions & 0 deletions src/jsc/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,12 @@ impl WebWorker {
}
}

// terminate() may have landed during load_entry_point_for_web_worker; skip dispatchOnline/tick().
if self.has_requested_terminate() {
self.flush_logs(vm);
return self.shutdown();
}
Comment thread
robobun marked this conversation as resolved.
Outdated

self.flush_logs(vm);
log!("[{}] event loop start", self.execution_context_id);
// dispatchOnline fires the parent-side 'open' event and flips the C++
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 @@ -121,6 +121,66 @@ test(
timeout,
);

// Regression: a nested worker whose grandchild's module fails to load reports
// its uncaught MODULE_NOT_FOUND via flush_logs → report_uncaught_exception →
// Bun__handleUncaughtException. When worker.terminate() and process.exit()
// land mid-dispatch, that handler would still lazily create `process` and do
// `process->get("_fatalException")` on a VM already asked to terminate, which
// tripped ASSERT(object->structure() == this) in Structure::storedPrototype.
// Debug-assert-only; the race is non-deterministic (~2/14 on
// release-asan-cov), so loop it.
test.skipIf(!isASAN)(
"terminating a worker while its grandchildren are reporting load errors does not assert",
async () => {
// Each subprocess run spawns a middle worker that creates grandchildren
// whose module load fails, then main terminates the middle worker and
// exits. The race window is between the grandchild's error dispatch and
// terminate_all_and_wait arming TerminationException. The grandchild path
// is an absolute nonexistent file so MODULE_NOT_FOUND is independent of
// import.meta.url's value inside the data-URL middle worker.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
const code = `
const { Worker } = require("node:worker_threads");
const bad = require("node:path").join(process.cwd(), "does-not-exist-xyzzy.mjs");
const middleSrc = \`
const { Worker, parentPort } = require("node:worker_threads");
for (let j = 0; j < 4; j++) {
const w = new Worker(\${JSON.stringify(bad)});
w.on("error", () => {});
}
parentPort.postMessage("spawned");
setInterval(() => {}, 1000);
\`;
const middle = new Worker(new URL(
"data:text/javascript;base64," + Buffer.from(middleSrc).toString("base64"),
));
middle.on("error", e => { console.error("middle error:", e.message); process.exit(1); });
middle.on("message", () => {
middle.terminate();
console.log("ok");
process.exit(0);
});
setTimeout(() => { console.error("timeout"); process.exit(1); }, 5000);
`;

for (let i = 0; i < 20; i++) {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", code],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stderr, stdout, exitCode, signalCode: proc.signalCode }).toEqual({
stderr: "",
stdout: "ok\n",
exitCode: 0,
signalCode: null,
});
}
},
timeout * 2,
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Regression: the per-VM c-ares channel was destroyed in deinit_runtime_state
// (RuntimeState drop) AFTER JSC teardown and RareData.file_polls drop.
// ares_destroy() synchronously fires EDESTRUCTION query callbacks and socket-
Expand Down
Loading