Skip to content
Open
9 changes: 9 additions & 0 deletions src/runtime/server/RequestContext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3582,7 +3582,16 @@
status: u16,
) {
jsc::mark_binding!();
if let Some(server) = self.server {
// The "error" may be the worker's TerminationException raised
// inside on_request's fetch-handler call; entering the user
// error() handler with it pending trips assertNoException().
Comment thread
robobun marked this conversation as resolved.
// SAFETY: BACKREF
if (*server).vm().script_execution_status() != bun_jsc::ScriptExecutionStatus::Running {
return;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
if let Some(server) = self.server {

Check warning on line 3594 in src/runtime/server/RequestContext.rs

View check run for this annotation

Claude / Claude Code Review

Redundant consecutive `if let Some(server) = self.server` blocks

nit: this opens a fresh `if let Some(server) = self.server { ... }` block immediately before the identical existing one at line 3594, with no intervening mutation of `self.server`. The gate could sit at the top of the existing block instead of duplicating the option match. Cosmetic only.
Comment thread
robobun marked this conversation as resolved.
Outdated
// SAFETY: BACKREF
let server = &*server;
let on_error = server.config().on_error;
Expand Down
43 changes: 25 additions & 18 deletions src/runtime/server/ServerWebSocket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ impl ServerWebSocket {
let global_object = handler.global_object();
let on_open_handler = handler.on_open;
let on_error = handler.on_error;
if vm.is_shutting_down() {
if vm.script_execution_status() != bun_jsc::ScriptExecutionStatus::Running {
bun_output::scoped_log!(WebSocketServer, "onOpen called after script execution");
ws.close();
return;
Expand Down Expand Up @@ -493,27 +493,26 @@ impl ServerWebSocket {
let global_object = self.handler().global_object();
// This is the start of a task.
let vm = self.handler().vm();
if vm.is_shutting_down() {
if vm.script_execution_status() != bun_jsc::ScriptExecutionStatus::Running {
bun_output::scoped_log!(WebSocketServer, "onMessage called after script execution");
ws.close();
return;
}

let _loop_guard = vm.enter_event_loop_scope();

let payload = match opcode {
Opcode::Text => jsc::bun_string_jsc::create_utf8_for_js(global_object, message),
Opcode::Binary => self.binary_to_js(global_object, message),
_ => unreachable!(),
};
let Ok(payload) = payload else { return };
Comment thread
robobun marked this conversation as resolved.
let arguments = [
self.this_value
.get()
.try_get()
.unwrap_or(JSValue::UNDEFINED),
match opcode {
Opcode::Text => jsc::bun_string_jsc::create_utf8_for_js(global_object, message)
.unwrap_or(JSValue::ZERO), // TODO: properly propagate exception upwards
Opcode::Binary => self
.binary_to_js(global_object, message)
.unwrap_or(JSValue::ZERO), // TODO: properly propagate exception upwards
_ => unreachable!(),
},
payload,
];

let mut corker = Corker {
Expand Down Expand Up @@ -562,7 +561,9 @@ impl ServerWebSocket {
bun_output::scoped_log!(WebSocketServer, "onDrain");
let handler = self.handler();
let vm = handler.vm();
if self.is_closed() || vm.is_shutting_down() {
if self.is_closed()
|| vm.script_execution_status() != bun_jsc::ScriptExecutionStatus::Running
{
return;
}

Expand Down Expand Up @@ -610,21 +611,25 @@ impl ServerWebSocket {
let cb = handler.on_ping;
let on_error = handler.on_error;
let vm = handler.vm();
if cb.is_empty_or_undefined_or_null() || vm.is_shutting_down() {
if cb.is_empty_or_undefined_or_null()
|| vm.script_execution_status() != bun_jsc::ScriptExecutionStatus::Running
{
return;
}
let global_this = handler.global_object();

// This is the start of a task.
let _loop_guard = vm.enter_event_loop_scope();

let Ok(payload) = self.binary_to_js(global_this, data) else {
return;
};
let args = [
self.this_value
.get()
.try_get()
.unwrap_or(JSValue::UNDEFINED),
self.binary_to_js(global_this, data)
.unwrap_or(JSValue::ZERO), // TODO: properly propagate exception upwards
payload,
];
if let Err(e) = cb.call(global_this, JSValue::UNDEFINED, &args) {
let err = global_this.take_exception(e);
Expand All @@ -646,20 +651,22 @@ impl ServerWebSocket {
let global_this = handler.global_object();
let vm = handler.vm();

if vm.is_shutting_down() {
if vm.script_execution_status() != bun_jsc::ScriptExecutionStatus::Running {
return;
}

// This is the start of a task.
let _loop_guard = vm.enter_event_loop_scope();

let Ok(payload) = self.binary_to_js(global_this, data) else {
return;
};
let args = [
self.this_value
.get()
.try_get()
.unwrap_or(JSValue::UNDEFINED),
self.binary_to_js(global_this, data)
.unwrap_or(JSValue::ZERO), // TODO: properly propagate exception upwards
payload,
];
if let Err(e) = cb.call(global_this, JSValue::UNDEFINED, &args) {
let err = global_this.take_exception(e);
Expand Down Expand Up @@ -723,7 +730,7 @@ impl ServerWebSocket {
});

let vm = handler.vm();
if vm.is_shutting_down() {
if vm.script_execution_status() != bun_jsc::ScriptExecutionStatus::Running {
return;
}

Expand Down
8 changes: 8 additions & 0 deletions src/runtime/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,14 @@
// S008: `Response<SSL>` is a ZST opaque — safe `*mut → &mut` deref.
let resp_ref = bun_opaque::opaque_deref_mut(resp);

// A worker terminate() raised inside a previous request's handler in
// the same poll sweep leaves the TerminationException pending; every
// on_request-family entry funnels here before its first JS call.
Comment thread
robobun marked this conversation as resolved.
Outdated
if server.vm().script_execution_status() != bun_jsc::ScriptExecutionStatus::Running {
server_body::respond_stopped_503(resp_ref);
return None;
}

Check failure on line 681 in src/runtime/server/mod.rs

View check run for this annotation

Claude / Claude Code Review

Three on_request-family entries bypass the new terminate gate (node:http, WS-upgrade id==0 fetch fallthrough, on_saved_request Saved arm)

The comment here asserts "every on_request-family entry funnels here before its first JS call", but three sibling request-entry paths still enter JS without a `script_execution_status()` gate (only the `js_value_for_dispatch()` finalized-wrapper check): `on_node_http_request_with_upgrade_ctx` (mod.rs:1217→1286), the `on_web_socket_upgrade` `id==0` fetch fallthrough (server_body.rs:3441→3497), and the `SavedRequestUnion::Saved` arm of `on_saved_request` (mod.rs:923→951). Each is a multi-dispatch-
Comment thread
claude[bot] marked this conversation as resolved.
Outdated

// We need to register the handler immediately since uSockets will not buffer.
//
// We first validate the self-reported request body length so that
Expand Down
6 changes: 6 additions & 0 deletions src/runtime/server/server_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3077,6 +3077,12 @@ where
) -> Option<PreparedRequestFor<'_, Ctx>> {
jsc::mark_binding!();

// Same worker-terminate gate as `NewServer::prepare_js_request_context`.
if self.vm().script_execution_status() != bun_jsc::ScriptExecutionStatus::Running {
respond_stopped_503(resp);
return None;
}

// We need to register the handler immediately since uSockets will not buffer.
//
// We first validate the self-reported request body length so that
Expand Down
7 changes: 7 additions & 0 deletions src/runtime/shell/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1199,6 +1199,13 @@ impl Interpreter {
let global_this = self
.global_this_ref()
.expect("global_this set on Js event-loop path");
if global_this.bun_vm().script_execution_status()
!= bun_jsc::ScriptExecutionStatus::Running
{
self.keep_alive.with_mut(|k| k.disable());
self.deref_root_shell_and_io_if_needed(true);
return Yield::done();
}
let buffered_stdout = self.get_buffered_stdout(global_this);
let buffered_stderr = self.get_buffered_stderr(global_this);
self.keep_alive.with_mut(|k| k.disable());
Expand Down
24 changes: 15 additions & 9 deletions src/runtime/socket/socket_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,11 @@ extern "C" fn select_alpn_callback(
{
let handlers = this.get_handlers();
let callback = handlers.on_alpn_callback();
if !callback.is_empty() && !handlers.vm.is_shutting_down() && !in_.is_null() && inlen > 0 {
if !callback.is_empty()
&& handlers.vm.script_execution_status() == jsc::ScriptExecutionStatus::Running
&& !in_.is_null()
&& inlen > 0
{
Comment thread
robobun marked this conversation as resolved.
let scope = handlers.enter();
let global = handlers.global_object;
let this_value = this.get_this_value(&global);
Expand Down Expand Up @@ -838,7 +842,7 @@ impl<const SSL: bool> NewSocket<SSL> {
log!("handleError");
let handlers = self.get_handlers();
let vm = handlers.vm;
if vm.is_shutting_down() {
if vm.script_execution_status() != jsc::ScriptExecutionStatus::Running {
return;
}
// the handlers must be kept alive for the duration of the function call
Expand Down Expand Up @@ -879,7 +883,7 @@ impl<const SSL: bool> NewSocket<SSL> {
}

let vm = handlers.vm;
if vm.is_shutting_down() {
if vm.script_execution_status() != jsc::ScriptExecutionStatus::Running {
return;
}
// Hold the socket alive for the rest of the dispatch: `internal_flush`
Expand Down Expand Up @@ -971,7 +975,7 @@ impl<const SSL: bool> NewSocket<SSL> {
if callback.is_empty() || this.flags.get().contains(Flags::FINALIZING) {
return;
}
if handlers.vm.is_shutting_down() {
if handlers.vm.script_execution_status() != jsc::ScriptExecutionStatus::Running {
return;
}

Expand Down Expand Up @@ -1615,7 +1619,9 @@ impl<const SSL: bool> NewSocket<SSL> {

let callback = handlers.on_end();
let vm = handlers.vm;
if callback.is_empty() || vm.is_shutting_down() {
if callback.is_empty()
|| vm.script_execution_status() != jsc::ScriptExecutionStatus::Running
{
this.poll_ref.with_mut(|p| p.unref(js_loop_ctx()));

// If you don't handle TCP fin, we assume you're done.
Expand Down Expand Up @@ -1758,8 +1764,8 @@ impl<const SSL: bool> NewSocket<SSL> {
let mut is_open = false;

if handlers.vm.script_execution_status() != jsc::ScriptExecutionStatus::Running {
// `on_close` is single-JS-entry, so the native close it routes
// through here just takes the trap and returns.
// `on_close` skips its JS dispatch under the same gate, so the
// native close is still safe here.
Comment thread
robobun marked this conversation as resolved.
if reject_unauthorized {
this.reject_unauthorized_connection();
}
Expand Down Expand Up @@ -2036,7 +2042,7 @@ impl<const SSL: bool> NewSocket<SSL> {
return Ok(());
}

if vm.is_shutting_down() {
if vm.script_execution_status() != jsc::ScriptExecutionStatus::Running {
drop(cleanup);
return Ok(());
}
Expand Down Expand Up @@ -2112,7 +2118,7 @@ impl<const SSL: bool> NewSocket<SSL> {
if callback.is_empty() || this.flags.get().contains(Flags::FINALIZING) {
return;
}
if handlers.vm.is_shutting_down() {
if handlers.vm.script_execution_status() != jsc::ScriptExecutionStatus::Running {
return;
}

Expand Down
67 changes: 67 additions & 0 deletions test/js/web/workers/worker-terminate-lifetime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -455,3 +455,70 @@ test.skipIf(!isDebug)(
},
120_000,
);

// Regression: the remaining NewSocket dispatch entry points (on_end,
// on_writable, on_data, on_timeout, on_close, handle_error, ALPN) still gated
// on is_shutting_down(), so a worker terminate() raised inside one socket's
// callback left the TerminationException pending for the next socket's
// dispatch in the same uSockets tick. This shape fires on plain TCP via
// on_end: the worker accepts NCLI connections, the parent FINs them all in
// one burst (so multiple on_end dispatches land in one poll sweep), then
// terminates the worker mid-sweep.
test.skipIf(!isDebug)(
"terminate() while a worker's Bun.listen end handler is firing does not trip assertNoException()",
async () => {
const ROUNDS = 15;
const NCLI = 60;
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const { Worker } = require("node:worker_threads");
const net = require("node:net");
const src =
"const { parentPort, workerData: d } = require('node:worker_threads');" +
"let opens = 0;" +
"const srv = Bun.listen({ hostname: '127.0.0.1', port: 0, allowHalfOpen: true, socket: {" +
" open(s) { if (++opens === d.ncli) parentPort.postMessage('ready'); }," +
" data() {}, end(s) { s.end(); }, close() {}, error() {} } });" +
"parentPort.postMessage(srv.port);";
for (let r = 0; r < ${ROUNDS}; r++) {
const w = new Worker(src, { eval: true, workerData: { ncli: ${NCLI} } });
const msgs = [];
const port = await new Promise((res, rej) => {
w.once("error", rej);
w.on("message", (m) => { msgs.push(m); if (msgs.length === 1) res(m); });
});
const clients = [];
for (let i = 0; i < ${NCLI}; i++) {
const c = net.connect(port, "127.0.0.1");
c.on("error", () => {}); c.on("data", () => {});
clients.push(c);
}
await new Promise((res, rej) => {
w.once("error", rej);
if (msgs.includes("ready")) return res();
w.on("message", (m) => { if (m === "ready") res(); });
});
w.on("error", () => {});
await Bun.sleep(r % 4);
for (const c of clients) c.end();
await w.terminate();
for (const c of clients) c.destroy();
}
console.log("PASS");
`,
],
env: { ...bunEnv, BUN_JSC_validateExceptionChecks: "1" },
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
expect(stdout).toBe("PASS\n");
expect(exitCode).toBe(0);
},
120_000,
);
Loading