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,6 +3582,15 @@ where
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 {
Comment thread
robobun marked this conversation as resolved.
Outdated
// SAFETY: BACKREF
let server = &*server;
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 @@
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 @@
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 };

Check warning on line 509 in src/runtime/server/ServerWebSocket.rs

View check run for this annotation

Claude / Claude Code Review

on_message/on_ping/on_pong early-return leaves the payload-conversion exception pending

nit: the `let Ok(payload) = ... else { return }` here (and in `on_ping`/`on_pong`) drops the `JsError` token without `take_exception()` + `run_error_callback`, whereas the sibling `on_close` in this file routes the identical `create_utf8_for_js` failure through the WS `error()` handler. Not blocking — the `_loop_guard` drop reports it as an uncaught exception so nothing is left pending — but matching `on_close`'s pattern would keep the error-path sequence consistent with the sibling exit site (a
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 @@
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 @@
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 @@
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 @@
});

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

Expand Down
7 changes: 7 additions & 0 deletions src/runtime/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,13 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
server_body::respond_stopped_503(bun_opaque::opaque_deref_mut(resp));
return;
};
// SAFETY: `this` is the live server backref for this request.
if unsafe { &*this }.vm().script_execution_status()
!= bun_jsc::ScriptExecutionStatus::Running
{
server_body::respond_stopped_503(bun_opaque::opaque_deref_mut(resp));
return;
}
let should_deinit_context = core::cell::Cell::new(false);
let Some(prepared) = Self::prepare_js_request_context(
this,
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
23 changes: 14 additions & 9 deletions src/runtime/socket/socket_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,14 @@
// the static list), the selected protocol string, or anything else to
// refuse the connection with a fatal no_application_protocol alert - the
// same contract as Node's ALPNCallback.
{
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
{

Check failure on line 116 in src/runtime/socket/socket_body.rs

View check run for this annotation

Claude / Claude Code Review

Remaining socket-module JS-dispatch sites still gate on is_shutting_down()

The ALPN gate is upgraded here, but its direct siblings in `src/runtime/socket/` still gate JS entry on `is_shutting_down()`: the two SNI `server_name` callbacks (`Listener.rs:1918` / `Listener.rs:2011`), `UpgradedDuplex::call_write_or_end` (`UpgradedDuplex.rs:210`), and `Handlers::{resolve_promise, reject_promise, call_error_handler}` (`Handlers.rs:218/234/280`). SNI in particular is the same BoringSSL-handshake shape as ALPN — multiple TLS handshakes in one poll sweep can fire back-to-back `se
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 @@
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 @@
}

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 @@
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,8 @@

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 +1763,8 @@
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 +2041,7 @@
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 +2117,7 @@
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