Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
4 changes: 4 additions & 0 deletions src/jsc/VM.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ impl VM {
crate::cpp::JSC__VM__clearHasTerminationRequest(self)
}

pub fn has_termination_request(&self) -> bool {
crate::cpp::JSC__VM__hasTerminationRequest(self)
}

#[track_caller]
pub fn throw_error(&self, global_object: &JSGlobalObject, value: JSValue) -> JsError {
crate::validation_scope!(scope, global_object);
Expand Down
6 changes: 3 additions & 3 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1074,7 +1074,7 @@ impl VirtualMachine {
}

pub fn handled_promise(&self, global_object: &JSGlobalObject, promise: JSValue) -> bool {
if self.is_shutting_down() {
if self.script_execution_status() != crate::ScriptExecutionStatus::Running {
return true;
}
Bun__emitHandledPromiseEvent(global_object, promise)
Expand Down Expand Up @@ -1348,7 +1348,7 @@ impl VirtualMachine {
err: JSValue,
is_rejection: bool,
) -> bool {
if self.is_shutting_down() {
if self.script_execution_status() != crate::ScriptExecutionStatus::Running {
return true;
}

Expand Down Expand Up @@ -3287,7 +3287,7 @@ impl VirtualMachine {
) {
use bun_options_types::schema::api::UnhandledRejections as Mode;

if self.is_shutting_down() {
if self.script_execution_status() != crate::ScriptExecutionStatus::Running {
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
40 changes: 26 additions & 14 deletions src/jsc/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,11 @@ impl WebWorker {
}
}

// terminate() may have landed during entrySettled / the status block above; skip dispatchOnline/tick().
if self.has_requested_terminate() && !self.exit_called.load(Ordering::Relaxed) {
return self.shutdown();
}

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 Expand Up @@ -1459,7 +1464,25 @@ impl WebWorker {
if vm_log.msgs.is_empty() {
return;
}
// Keyed on the JSC request so same-thread self-signals that only set the atomic (configure_defines) still dispatch.
if self.has_requested_terminate() && vm.jsc_vm().has_termination_request() {
return;
}
let global = vm.global();
let report_thrown = |e: JsError| {
if self.has_requested_terminate() && global.vm().has_termination_request() {
return;
}
Comment thread
robobun marked this conversation as resolved.
// take_exception on a JsError always yields an Exception cell; None is unreachable.
let exc = global
.take_exception(e)
.as_exception(global.vm().as_mut_ptr())
.expect("takeException returned non-Exception");
let _ = jsc::js_global_object::report_uncaught_exception(
global,
jsc::Exception::opaque_ref(exc),
);
};
let result: jsc::JsResult<(JSValue, BunString)> = (|| {
let err = vm_log.to_js(global, "Error in worker")?;
let str = err.to_bun_string(global)?;
Expand All @@ -1468,27 +1491,16 @@ impl WebWorker {
let (err, str) = match result {
Ok(pair) => pair,
Err(JsError::OutOfMemory) => bun_core::out_of_memory(),
Err(JsError::Thrown | JsError::Terminated) => panic!("unhandled exception"),
Err(JsError::Terminated) => return,
Err(e @ JsError::Thrown) => return report_thrown(e),
};
let mut str = bun_core::OwnedString::new(str);
let dispatch = jsc::host_fn::from_js_host_call_generic(global, || {
// `cpp_worker` is the opaque C++-owned handle; `str` reffed for the call.
WebWorker__dispatchError(global, self.cpp_worker, &mut str, err)
});
if let Err(e) = dispatch {
// `take_exception` on a `JsError` always returns an Exception
// cell; None is unreachable. Do not silently drop the error.
let exc = global
.take_exception(e)
.as_exception(global.vm().as_mut_ptr())
.expect("takeException returned non-Exception");
// `Exception` is an `opaque_ffi!` ZST handle; `opaque_ref` is the
// centralised non-null-ZST deref proof (`exc` is non-null per the
// `expect` above).
let _ = jsc::js_global_object::report_uncaught_exception(
global,
jsc::Exception::opaque_ref(exc),
);
report_thrown(e);
}
}
}
Expand Down
49 changes: 49 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,55 @@ test(
timeout,
);

// Regression: a worker whose module fails to load reports its uncaught
// MODULE_NOT_FOUND via flush_logs → report_uncaught_exception →
// Bun__handleUncaughtException. When worker.terminate() + process.exit() land
// mid-dispatch, flush_logs panicked on JsError::Terminated, and the 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. The race
// is non-deterministic (~2/14 on release-asan-cov), so loop it.
test.skipIf(!isASAN)(
"terminate() + process.exit() while workers are reporting load errors does not crash",
async () => {
// Each subprocess run spawns workers whose module load fails, then main
// terminates them and exits. The race window is between a worker's error
// dispatch and terminate_all_and_wait arming TerminationException. Two
// levels only (no middle worker) to avoid also tripping the unrelated
// nested-worker parent-VM UAF that #31951 addresses.
const code = `
const { Worker } = require("node:worker_threads");
const bad = require("node:path").join(process.cwd(), "does-not-exist-xyzzy.mjs");
const workers = [];
for (let j = 0; j < 4; j++) {
const w = new Worker(bad);
w.on("error", () => {});
workers.push(w);
}
for (const w of workers) w.terminate();
console.log("ok");
process.exit(0);
`;

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