Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 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
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 @@
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 @@
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;

Check warning on line 1355 in src/jsc/bindings/BunProcess.cpp

View check run for this annotation

Claude / Claude Code Review

Bun__handleUnhandledRejection guard returns false, routing with-listener case to unguarded ->get() paths

This guard returns `false`, but `Bun__handleUncaughtException`'s guard at :1219 returns `true` with the stated rationale "stops the Rust caller from going on to run `on_unhandled_rejection` (more JS on a terminating VM)". For a worker that HAS a `process.on('unhandledRejection', …)` listener, pre-PR this function returned `true` (via `wrapped.emit()`) and `VirtualMachine::unhandled_rejection` short-circuited; returning `false` now falls through to the unguarded `Bun__promises__emitUnhandledRejec
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 @@
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