From 386b212464e4f27d5eadbf03e7d82c36dd8db461 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:15:11 -0700 Subject: [PATCH 01/45] serve: defer js_value downgrade until no dispatch can fire Per-route handlers are stored as WriteBarriers reachable only via the Server JS wrapper. stop() previously downgraded js_value immediately, so a late keep-alive request after stop+drop+GC would hit js_value_assert_alive() with a Finalized ref and panic. Downgrade inside the deinit_if_we_can idle predicate instead so the wrapper stays rooted until pending_requests/listener/active websockets are all clear. The websocket close path does not yet call deinit_if_we_can; the next commit threads an AnyServer backref through Handler so the last close can trigger it. --- src/runtime/server/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 9a5e5f90b6f4..7127b19a3c5b 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -1583,9 +1583,6 @@ impl NewServer { } pub fn stop(&mut self, abrupt: bool) { - if self.js_value.is_not_empty() { - self.js_value.downgrade(); - } if self.config.allow_hot && !self.config.id.is_empty() { // `hot_map()` is reached via the thread-local VM singleton (raw ptr // deref) and does not borrow `self`, so it cannot overlap with the @@ -1664,6 +1661,9 @@ impl NewServer { ); } if self.pending_requests == 0 && !self.has_listener() && !self.has_active_web_sockets() { + // Wrapper-rooted handlers need the wrapper to outlive every + // dispatch; downgrade only once nothing can call them. + self.js_value.downgrade(); if let Some(ws) = self.config.websocket.as_mut() { ws.handler.app = None; } From 65c76a2dfc7e7879739903a964915a7def30ce9b Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:17:49 -0700 Subject: [PATCH 02/45] serve: run deinit_if_we_can when the last websocket closes The previous commit moved the JsRef downgrade into deinit_if_we_can's idle predicate, but nothing calls that when the last websocket closes after a graceful stop. Thread an AnyServer backref through Handler so on_close can trigger it; move the live-socket count onto NewServer (where reload's context swap can no longer reset it). on_close also copies the close handler to a stack local before sig.signal() so a GC between the test and the call cannot collect it. --- src/runtime/server/ServerWebSocket.rs | 38 ++++++++++++---- src/runtime/server/WebSocketServerContext.rs | 24 +++------- src/runtime/server/mod.rs | 47 ++++++++++++++++++-- 3 files changed, 79 insertions(+), 30 deletions(-) diff --git a/src/runtime/server/ServerWebSocket.rs b/src/runtime/server/ServerWebSocket.rs index 5fb0c0e297b4..5056568580af 100644 --- a/src/runtime/server/ServerWebSocket.rs +++ b/src/runtime/server/ServerWebSocket.rs @@ -388,8 +388,12 @@ impl ServerWebSocket { let handler = self.handler(); let vm = handler.vm(); - // The handler is shared (&), so mutate via the interior-mutability helper. - handler.active_connections_saturating_add(1); + // Live-socket accounting lives on the server (`Cell`), reached + // through the type-erased backref so the shared `&Handler` suffices. + let server = handler.server; + if let Some(server) = server { + server.on_websocket_opened(); + } let global_object = handler.global_object(); let on_open_handler = handler.on_open; if vm.is_shutting_down() { @@ -426,17 +430,26 @@ impl ServerWebSocket { if let Some(err_value) = result.to_error() { bun_output::scoped_log!(WebSocketServer, "onOpen exception"); + let mut closed_here = false; if !self.flags.get().closed() { self.update_flags(|f| f.set_closed(true)); // we un-gracefully close the connection if there was an exception // we don't want any event handlers to fire after this for anything other than error() // https://github.com/oven-sh/bun/issues/1480 + // (`close()` re-enters `on_close`, which skips its own + // accounting because the closed flag is already set.) self.websocket().close(); - handler.active_connections_saturating_sub(1); + closed_here = true; this_value.unprotect(); } handler.run_error_callback(vm, global_object, err_value); + if closed_here { + if let Some(server) = server { + // May run the idle pass; no `&Handler` borrow is live here. + server.on_websocket_closed(); + } + } } } @@ -633,11 +646,18 @@ impl ServerWebSocket { bun_output::scoped_log!(WebSocketServer, "onClose"); // TODO: Can this called inside finalize? let handler = self.handler(); + // Copy the erased server handle out now: the guard below runs after + // every `handler` borrow has expired, and `on_websocket_closed` may + // form `&mut NewServer` (which owns the handler storage) to run the + // idle pass when this was the last live socket. + let server = handler.server; let was_closed = self.is_closed(); self.update_flags(|f| f.set_closed(true)); scopeguard::defer! { if !was_closed { - handler.active_connections_saturating_sub(1); + if let Some(server) = server { + server.on_websocket_closed(); + } } } let signal = self.signal.take(); @@ -673,7 +693,10 @@ impl ServerWebSocket { return; } - if !handler.on_close.is_empty_or_undefined_or_null() { + // Copy to a stack local before `sig.signal()` re-enters JS: a GC + // between the test and the `.call(...)` could otherwise collect it. + let on_close_handler = handler.on_close; + if !on_close_handler.is_empty_or_undefined_or_null() { let global_object = handler.global_object(); let _loop_guard = vm.enter_event_loop_scope(); @@ -702,9 +725,8 @@ impl ServerWebSocket { }; let call_args = [cached_this, JSValue::js_number(code as f64), message_js]; - if let Err(e) = handler - .on_close - .call(global_object, JSValue::UNDEFINED, &call_args) + if let Err(e) = + on_close_handler.call(global_object, JSValue::UNDEFINED, &call_args) { let err = global_object.take_exception(e); bun_output::scoped_log!(WebSocketServer, "onClose error {}", was_not_empty); diff --git a/src/runtime/server/WebSocketServerContext.rs b/src/runtime/server/WebSocketServerContext.rs index 2548f670af85..c643d5ce6c86 100644 --- a/src/runtime/server/WebSocketServerContext.rs +++ b/src/runtime/server/WebSocketServerContext.rs @@ -34,10 +34,11 @@ pub struct Handler { // LIFETIMES.tsv = STATIC (vm) / JSC_BORROW (global_object) — both outlive the handler. pub vm: bun_ptr::BackRef, pub global_object: bun_ptr::BackRef, - /// Mutated through `&Handler` (the field is owned by - /// `ServerConfig.websocket` and only ever touched on the JS thread), so - /// it's a `Cell`. - pub active_connections: core::cell::Cell, + /// Type-erased backref to the owning `NewServer`, set alongside `app` + /// in `set_routes` (so it is in place before any socket can upgrade and + /// refreshed whenever a reload installs a new context). `ServerWebSocket` + /// open/close events route the live-socket accounting through it. + pub server: Option, /// used by publish() pub flags: HandlerFlags, @@ -68,19 +69,6 @@ impl Handler { self.vm.get() } - #[inline] - pub fn active_connections_saturating_add(&self, n: usize) { - self.active_connections - .set(self.active_connections.get().saturating_add(n)); - } - - /// See `active_connections_saturating_add`. - #[inline] - pub fn active_connections_saturating_sub(&self, n: usize) { - self.active_connections - .set(self.active_connections.get().saturating_sub(n)); - } - pub fn run_error_callback( &self, vm: &VirtualMachine, @@ -119,7 +107,7 @@ impl Handler { app: None, vm: bun_ptr::BackRef::new(VirtualMachine::get()), global_object: bun_ptr::BackRef::new(global_object), - active_connections: core::cell::Cell::new(0), + server: None, flags: HandlerFlags::empty(), }; diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 7127b19a3c5b..e5f29454a00c 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -265,6 +265,11 @@ pub struct NewServer { pub base_url_string_for_joining: Box<[u8]>, pub config: ServerConfig, pub pending_requests: usize, + /// Live `ServerWebSocket` count. Lives on the server (not the websocket + /// context) so a reload's context swap cannot reset it, and sits in a + /// `Cell` because the open/close accounting arrives through shared + /// `AnyServer` handles on the JS thread. + pub active_websocket_count: core::cell::Cell, pub request_pool: *mut request_context::RequestContextStackAllocator, /// Null until the H3 listen path runs (`HAS_H3 && config.http3`); never /// allocated when `!SSL`. Kept as a raw nullable pointer rather than a @@ -1462,10 +1467,19 @@ impl NewServer { } pub fn active_sockets_count(&self) -> u32 { - self.config - .websocket - .as_ref() - .map_or(0, |ws| ws.handler.active_connections.get() as u32) + self.active_websocket_count.get() + } + + pub(crate) fn note_websocket_opened(&self) { + self.active_websocket_count + .set(self.active_websocket_count.get().saturating_add(1)); + } + + /// Returns true when this close drained the last live websocket. + pub(crate) fn note_websocket_closed(&self) -> bool { + let remaining = self.active_websocket_count.get().saturating_sub(1); + self.active_websocket_count.set(remaining); + remaining == 0 } pub fn has_active_web_sockets(&self) -> bool { @@ -1905,6 +1919,7 @@ impl NewServer { h3_alt_svc: Box::<[u8]>::default(), js_value: jsc::JsRef::empty(), pending_requests: 0, + active_websocket_count: core::cell::Cell::new(0), request_pool: >::request_pool(), // Plain HTTP servers never allocate the ~816 KB H3 pool; defer to // the H3-listen path (`listen()` below) so HTTPS servers that @@ -2047,6 +2062,7 @@ impl NewServer { websocket.global_object = bun_ptr::BackRef::new(bun_opaque::opaque_deref(self.global_this)); websocket.handler.app = Some(std::ptr::from_mut(app).cast::()); + websocket.handler.server = Some(any_server); websocket .handler .flags @@ -3453,6 +3469,29 @@ impl AnyServer { any_server_dispatch!(self, |s| s.inspector_server_id) } + pub(crate) fn on_websocket_opened(&self) { + any_server_dispatch!(self, |s| s.note_websocket_opened()); + } + + /// Decrement the live-socket count and, when the last socket drained on + /// an already-stopped server, run the idle pass so the `JsRef` downgrade + /// (and deferred deinit) that was held back by the open sockets fires. + /// + /// Skipped while still listening (the idle pass would no-op) and while + /// TERMINATED: the abrupt-stop path drains every socket synchronously + /// from inside `stop_listening` (which holds `&mut self`), and `stop()` + /// runs `deinit_if_we_can` itself right after. + pub(crate) fn on_websocket_closed(&self) { + let drained = any_server_dispatch!(self, |s| { + s.note_websocket_closed() + && !s.has_listener() + && !s.flags.contains(ServerFlags::TERMINATED) + }); + if drained { + any_server_dispatch_mut!(self, |s| s.deinit_if_we_can()); + } + } + pub fn set_inspector_server_id(&mut self, id: jsc::DebuggerId) { any_server_dispatch_mut!(self, |s| { s.inspector_server_id = id; From 0ed8dfdab108bec1a733100ff250da9cafcb1d95 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:22:27 -0700 Subject: [PATCH 03/45] serve: answer 503 instead of panicking when a late request races wrapper finalize Idle keep-alive sockets are not counted in pending_requests, so the wrapper can downgrade and be collected while one such socket can still deliver another request. js_value_assert_alive() panics on Finalized; the dispatch entry points now check first and close the connection with 503 instead. --- src/runtime/server/mod.rs | 28 ++++++++++++++++++++++++++++ src/runtime/server/server_body.rs | 22 ++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index e5f29454a00c..db43a22208f8 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -570,6 +570,16 @@ impl NewServer { self.js_value.try_get().expect("js_value alive") } + /// Returns the wrapper, or None if it has been finalized (server fully + /// stopped and the JS reference dropped). Dispatch trampolines should + /// answer 503+close on None. + pub fn js_value_for_dispatch(&self) -> Option { + match &self.js_value { + jsc::JsRef::Finalized => None, + _ => self.js_value.try_get(), + } + } + /// Per-monomorphization static. /// Rust statics cannot be const-generic; routed through a /// `&'static AtomicBool` so the four (SSL,DEBUG) instantiations share one @@ -1031,6 +1041,16 @@ impl NewServer { req: &mut uws_sys::Request, resp: *mut uws_sys::NewAppResponse, ) { + // Idle keep-alive sockets aren't counted in pending_requests, so the + // wrapper can have been collected before this fires. Refuse and close + // rather than panicking in js_value_assert_alive() below. + // SAFETY: `this` is the live server backref for this request. + if unsafe { &*this }.js_value_for_dispatch().is_none() { + let resp_ref = bun_opaque::opaque_deref_mut(resp); + resp_ref.write_status(b"503 Service Unavailable"); + resp_ref.end_without_body(true); + return; + } let should_deinit_context = core::cell::Cell::new(false); let Some(prepared) = Self::prepare_js_request_context( this, @@ -1082,6 +1102,14 @@ impl NewServer { let server = user_route.server.cast_mut(); let index = user_route.id; + // SAFETY: `server` is the live backref stored in `user_route`. + if unsafe { &*server }.js_value_for_dispatch().is_none() { + let resp_ref = bun_opaque::opaque_deref_mut(resp); + resp_ref.write_status(b"503 Service Unavailable"); + resp_ref.end_without_body(true); + return; + } + let should_deinit_context = core::cell::Cell::new(false); let Some(mut prepared) = Self::prepare_js_request_context( server, diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 6182510d5cd2..f67a5e3970c8 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2806,6 +2806,12 @@ where let server = unsafe { &mut *server_ptr }; let index = user_route.id; + if server.js_value_for_dispatch().is_none() { + RespLike::write_status(resp, b"503 Service Unavailable"); + RespLike::end_without_body(resp, true); + return; + } + let should_deinit_context = core::cell::Cell::new(false); let Some(mut prepared) = server.prepare_js_request_context_for::( req, @@ -2890,6 +2896,11 @@ where req: &mut Ctx::Req, resp: &mut Ctx::Resp, ) { + if self.js_value_for_dispatch().is_none() { + RespLike::write_status(resp, b"503 Service Unavailable"); + RespLike::end_without_body(resp, true); + return; + } let self_ptr: *mut Self = self; let should_deinit_context = core::cell::Cell::new(false); let Some(prepared) = self.prepare_js_request_context_for::( @@ -3189,6 +3200,12 @@ where let server_ptr = server_ref.as_ptr(); let index = this.id; + if server_ref.js_value_for_dispatch().is_none() { + resp.write_status(b"503 Service Unavailable"); + resp.end_without_body(true); + return; + } + let should_deinit_context = core::cell::Cell::new(false); // SAFETY: `server_ptr` is the live heap server registered for this route; // `req`/`resp` are the live uWS handles passed to the route handler. @@ -3265,6 +3282,11 @@ where // (mod.rs `app.ws("/*", self_ptr, 0, ..)`); live for the request's // duration. let this = unsafe { &mut *self_ptr }; + if this.js_value_for_dispatch().is_none() { + resp.write_status(b"503 Service Unavailable"); + resp.end_without_body(true); + return; + } if this.config.on_node_http_request.is_some() { // NOTE: receiver is `*mut Self` (mod.rs) — the callee re-enters // JS, so a long-lived `&mut self` here would alias on callback. From 9b3903d2f794837dbde1877c67c9b9492f6d7436 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:24:18 -0700 Subject: [PATCH 04/45] test: late keep-alive request to a route after stop+GC does not crash --- test/js/bun/http/bun-server.test.ts | 98 +++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index 26b8d80e2bd2..3937df78b39a 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -613,6 +613,104 @@ test("should be able to await server.stop(true) with keep alive", async () => { expect(async () => await fetch(server.url)).toThrow(); }); +test("late keep-alive request to a route after stop()+GC does not crash", async () => { + // Per-route handlers live in ServerRouteList, which is reachable from JS only + // through the Server wrapper. After a graceful stop() the user may drop the + // wrapper, and a subsequent GC may finalize it before an idle keep-alive + // connection sends one more request. Run in a subprocess so the previous + // js_value_assert_alive() panic on that path surfaces as a non-zero exit + // instead of taking down the test runner. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { fullGC } = require("bun:jsc"); + + let received = ""; + let waiter = Promise.withResolvers(); + const nextResponse = () => { + // Wait until a complete HTTP/1.1 response (headers + 2-byte body or + // an empty body for 503) has arrived, then consume it. + return (async () => { + while (true) { + const headerEnd = received.indexOf("\\r\\n\\r\\n"); + if (headerEnd !== -1) { + const head = received.slice(0, headerEnd); + const m = /content-length: (\\d+)/i.exec(head); + const bodyLen = m ? Number(m[1]) : 0; + const total = headerEnd + 4 + bodyLen; + if (received.length >= total) { + const status = head.split("\\r\\n")[0]; + received = received.slice(total); + return status; + } + } + await waiter.promise; + waiter = Promise.withResolvers(); + } + })(); + }; + + const port = await (async () => { + const server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + routes: { "/r": () => new Response("ok") }, + }); + const port = server.port; + + globalThis.sock = await Bun.connect({ + hostname: "127.0.0.1", + port, + socket: { + data(socket, data) { + received += data.toString("latin1"); + waiter.resolve(); + }, + close() { waiter.resolve(); }, + error() { waiter.resolve(); }, + }, + }); + + sock.write("GET /r HTTP/1.1\\r\\nHost: x\\r\\nConnection: keep-alive\\r\\n\\r\\n"); + const first = await nextResponse(); + if (!first.includes("200")) throw new Error("first request failed: " + first); + + // Graceful stop: existing connections stay open, listener closes. + server.stop(); + return port; + })(); + // The only \`server\` binding is now out of scope. + + for (let i = 0; i < 10; i++) { + Bun.gc(true); + fullGC(); + await Bun.sleep(10); + } + + // Reuse the same keep-alive socket. Previously this would dispatch into + // a finalized route list and panic; now it must answer (200 if the + // wrapper happened to survive, 503 if it was collected). + sock.write("GET /r HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\n\\r\\n"); + const second = await nextResponse(); + console.log(second); + + sock.end(); + process.exit(0); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toMatch(/HTTP\/1\.1 (200|503)/); + expect(exitCode).toBe(0); +}); + test("should be able to async upgrade using custom protocol", async () => { const { promise, resolve } = Promise.withResolvers<{ code: number; reason: string } | boolean>(); using server = Bun.serve({ From 9e8171a857ef82ce5f93234676422a64f27d9cc3 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:37:51 -0700 Subject: [PATCH 05/45] serve: close idle keep-alive sockets in graceful stop_listening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JsRef::Weak holds a raw JSValue, not a JSC::Weak: try_get() can return the address of a dead-but-unswept cell, so the Finalized→503 check alone leaves a window where dispatch reads an unrooted handler shadow. Closing idle connections at stop() removes the late-request source; in-flight requests are not idle and drain normally. The on_open error-path websocket-close accounting already runs after run_error_callback as of 65c76a2dfc, so the live-socket count stays nonzero across that read; no further change needed there. --- src/runtime/server/mod.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index db43a22208f8..bd55d6942463 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -1614,6 +1614,15 @@ impl NewServer { if !abrupt { // S012: `app::ListenSocket` is a ZST opaque — safe deref. bun_opaque::opaque_deref_mut(listener).close(); + // Shut idle keep-alive sockets so a late request can't dispatch + // after the JS wrapper drops its strong root. `JsRef::Weak` holds + // a raw JSValue (not a JSC::Weak), so `try_get` can hand back a + // dead-but-unswept cell and the Finalized→503 guard alone leaves a + // window. In-flight requests aren't idle and drain normally. + if let Some(app) = self.app { + // S012: `NewApp` is a ZST opaque — safe `*mut → &mut` deref. + bun_opaque::opaque_deref_mut(app).close_idle_connections(); + } } else if !self.flags.contains(ServerFlags::TERMINATED) { if let Some(ws) = self.config.websocket.as_mut() { ws.handler.app = None; From 41464801e283f031c297ae7e4db9c38c15deb0d1 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:38:20 -0700 Subject: [PATCH 06/45] test: server wrapper survives GC while a websocket is connected after stop --- test/js/bun/http/bun-server.test.ts | 88 +++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index 3937df78b39a..69751d75c2f5 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -711,6 +711,94 @@ test("late keep-alive request to a route after stop()+GC does not crash", async expect(exitCode).toBe(0); }); +test("server wrapper survives GC while a websocket is connected after stop()", async () => { + // The previous test exercises the one-tick HTTP keep-alive race; this one + // covers the steadier websocket case. After a graceful stop() with a live + // websocket, the user may drop their `server` binding. The native struct + // stays alive (active_websockets > 0), but stop() previously downgraded + // js_value immediately, so GC could finalize the JS wrapper — and with it + // m_routeList — while the connection was still in use. With the downgrade + // deferred into deinit_if_we_can's idle predicate, the wrapper must outlive + // the websocket and become collectable only after the last close. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { fullGC, heapStats } = require("bun:jsc"); + + const serverCount = () => { + const c = heapStats().objectTypeCounts; + return (c.DebugHTTPServer ?? 0) + (c.HTTPServer ?? 0); + }; + + const baseline = serverCount(); + + const ws = await (async () => { + const server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + routes: { "/r": () => new Response("ok") }, + fetch(req, server) { + if (server.upgrade(req)) return; + return new Response("nope", { status: 404 }); + }, + websocket: { open() {}, message() {}, close() {} }, + }); + + const opened = Promise.withResolvers(); + const ws = new WebSocket("ws://127.0.0.1:" + server.port); + ws.onopen = () => opened.resolve(); + ws.onerror = e => opened.reject(e); + await opened.promise; + + // Graceful stop: listener closes, the live websocket stays open. + server.stop(); + return ws; + })(); + // The only \`server\` binding is now out of scope; only the live + // websocket keeps the native side around. + + for (let i = 0; i < 10; i++) { + Bun.gc(true); + fullGC(); + await Bun.sleep(10); + } + const afterStopGC = serverCount(); + + const closed = Promise.withResolvers(); + ws.onclose = () => closed.resolve(); + ws.close(); + await closed.promise; + + for (let i = 0; i < 10; i++) { + Bun.gc(true); + fullGC(); + await Bun.sleep(10); + } + const afterCloseGC = serverCount(); + + console.log(JSON.stringify({ baseline, afterStopGC, afterCloseGC })); + process.exit(0); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const { baseline, afterStopGC, afterCloseGC } = JSON.parse(stdout.trim()); + // js_value stays Strong while a websocket is connected → GC must not + // collect the wrapper. + expect(afterStopGC).toBeGreaterThan(baseline); + // Last websocket closing triggers deinit_if_we_can → downgrade → wrapper + // becomes collectable again (no leak). + expect(afterCloseGC).toBeLessThanOrEqual(baseline); + expect(exitCode).toBe(0); +}); + test("should be able to async upgrade using custom protocol", async () => { const { promise, resolve } = Promise.withResolvers<{ code: number; reason: string } | boolean>(); using server = Bun.serve({ From 0929f8b2e5cb482eef4f643abe0b2a8d4325466d Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:57:32 -0700 Subject: [PATCH 07/45] serve: gate dispatch on js_value being strong, not just non-finalized JsRef::Weak holds a raw JSValue: try_get() on Weak returns the address even when the cell is dead-but-unswept. Gating on Strong means trampolines refuse the moment the server goes idle (downgrade) rather than only after the wrapper destructor has run. --- src/runtime/server/mod.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index bd55d6942463..764663cb8d9b 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -570,13 +570,14 @@ impl NewServer { self.js_value.try_get().expect("js_value alive") } - /// Returns the wrapper, or None if it has been finalized (server fully - /// stopped and the JS reference dropped). Dispatch trampolines should - /// answer 503+close on None. + /// Returns the wrapper while it is strongly rooted, or None once the + /// server has gone idle and downgraded. Dispatch trampolines should + /// answer 503+close on None. Checking strong (not just Finalized) closes + /// the dead-but-unswept window where `JsRef::Weak` holds a stale address. pub fn js_value_for_dispatch(&self) -> Option { match &self.js_value { - jsc::JsRef::Finalized => None, - _ => self.js_value.try_get(), + jsc::JsRef::Strong(_) => self.js_value.try_get(), + _ => None, } } From 868f7aeec3bdbc45553f16fd49878e536c8a78a4 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 13:06:53 -0700 Subject: [PATCH 08/45] test: account for prototype in heapStats baseline; handle closed-socket second request --- test/js/bun/http/bun-server.test.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index 69751d75c2f5..f67cd68d05eb 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -628,6 +628,7 @@ test("late keep-alive request to a route after stop()+GC does not crash", async const { fullGC } = require("bun:jsc"); let received = ""; + let sockClosed = false; let waiter = Promise.withResolvers(); const nextResponse = () => { // Wait until a complete HTTP/1.1 response (headers + 2-byte body or @@ -646,6 +647,7 @@ test("late keep-alive request to a route after stop()+GC does not crash", async return status; } } + if (sockClosed) return ""; await waiter.promise; waiter = Promise.withResolvers(); } @@ -668,8 +670,8 @@ test("late keep-alive request to a route after stop()+GC does not crash", async received += data.toString("latin1"); waiter.resolve(); }, - close() { waiter.resolve(); }, - error() { waiter.resolve(); }, + close() { sockClosed = true; waiter.resolve(); }, + error() { sockClosed = true; waiter.resolve(); }, }, }); @@ -707,7 +709,11 @@ test("late keep-alive request to a route after stop()+GC does not crash", async const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); - expect(stdout).toMatch(/HTTP\/1\.1 (200|503)/); + // The second request either dispatches (wrapper survived → 200), is refused + // (wrapper downgraded → 503), or never arrives because graceful stop closed + // the idle keep-alive socket (empty). Any of these is correct; the point is + // the process didn't crash. + expect(stdout.trim()).toMatch(/^(|HTTP\/1\.1 (200|503).*)$/s); expect(exitCode).toBe(0); }); @@ -791,11 +797,12 @@ test("server wrapper survives GC while a websocket is connected after stop()", a expect(stderr).toBe(""); const { baseline, afterStopGC, afterCloseGC } = JSON.parse(stdout.trim()); // js_value stays Strong while a websocket is connected → GC must not - // collect the wrapper. - expect(afterStopGC).toBeGreaterThan(baseline); + // collect the wrapper. objectTypeCounts includes the prototype (1) once the + // first server has been created, so the live instance shows as baseline+2. + expect(afterStopGC).toBeGreaterThan(baseline + 1); // Last websocket closing triggers deinit_if_we_can → downgrade → wrapper // becomes collectable again (no leak). - expect(afterCloseGC).toBeLessThanOrEqual(baseline); + expect(afterCloseGC).toBeLessThanOrEqual(baseline + 1); expect(exitCode).toBe(0); }); From 4c08ec88047e53983633fa9bcd42617b437fc9db Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:25:05 -0700 Subject: [PATCH 09/45] serve: add WriteBarrier slots for handler callbacks on the wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per-route handlers already use this pattern (ServerRouteList held in m_routeList). Extend to fetch/error/nodeHTTPRequest/clientError/ws handlers and the all-closed promise so the native↔JS cycle becomes all-JS-heap once the corresponding Strong/gcProtect roots are dropped in a follow-up. The unused JSServerWebSocket m_socket slot is repurposed to hold the server wrapper so a connected websocket keeps it (and its handler slots) reachable. No behavior change yet — the slots are declared but not written. --- src/runtime/server/mod.rs | 39 +++++++++++++++++++++++++--- src/runtime/server/server.classes.ts | 12 +++++++-- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 764663cb8d9b..821cd7360aee 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -2919,19 +2919,50 @@ use server_body::{Bun__ServerRouteList__callRoute, Bun__ServerRouteList__create} /// `${T}Prototype__routeList{Get,Set}CachedValue` (generate-classes.ts). mod route_list_cached { pub(super) mod http { - bun_jsc::codegen_cached_accessors!("HTTPServer"; routeList); + bun_jsc::codegen_cached_accessors!( + "HTTPServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsHandlers, allClosedPromise + ); } pub(super) mod https { - bun_jsc::codegen_cached_accessors!("HTTPSServer"; routeList); + bun_jsc::codegen_cached_accessors!( + "HTTPSServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsHandlers, allClosedPromise + ); } pub(super) mod debug_http { - bun_jsc::codegen_cached_accessors!("DebugHTTPServer"; routeList); + bun_jsc::codegen_cached_accessors!( + "DebugHTTPServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsHandlers, allClosedPromise + ); } pub(super) mod debug_https { - bun_jsc::codegen_cached_accessors!("DebugHTTPSServer"; routeList); + bun_jsc::codegen_cached_accessors!( + "DebugHTTPSServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsHandlers, allClosedPromise + ); } } +macro_rules! cached_value_dispatch { + ($get_fn:ident, $set_fn:ident, $get_cached:ident, $set_cached:ident) => { + #[allow(dead_code)] + pub fn $get_fn(server_js: JSValue) -> Option { + match (SSL, DEBUG) { + (false, false) => route_list_cached::http::$get_cached(server_js), + (true, false) => route_list_cached::https::$get_cached(server_js), + (false, true) => route_list_cached::debug_http::$get_cached(server_js), + (true, true) => route_list_cached::debug_https::$get_cached(server_js), + } + } + #[allow(dead_code)] + pub fn $set_fn(server_js: JSValue, global: &JSGlobalObject, v: JSValue) { + match (SSL, DEBUG) { + (false, false) => route_list_cached::http::$set_cached(server_js, global, v), + (true, false) => route_list_cached::https::$set_cached(server_js, global, v), + (false, true) => route_list_cached::debug_http::$set_cached(server_js, global, v), + (true, true) => route_list_cached::debug_https::$set_cached(server_js, global, v), + } + } + }; +} + // ─── extern "C" trampolines ────────────────────────────────────────────────── // Monomorphized on the const-generic server params; // the bodies downcast `user_data` and forward into the typed method. diff --git a/src/runtime/server/server.classes.ts b/src/runtime/server/server.classes.ts index 08cc34caf0c4..bd0a26dc39a5 100644 --- a/src/runtime/server/server.classes.ts +++ b/src/runtime/server/server.classes.ts @@ -90,7 +90,15 @@ function generate(name) { finalize: true, construct: true, noConstructor: true, - values: ["routeList"], + values: [ + "routeList", + "onRequest", + "onError", + "onNodeHTTPRequest", + "onClientError", + "wsHandlers", + "allClosedPromise", + ], }); } export default [ @@ -335,7 +343,7 @@ export default [ finalize: true, construct: true, klass: {}, - values: ["socket"], + values: ["server"], }), define({ From b8498fd5ce4224e238295e3603b676a04175b851 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:29:32 -0700 Subject: [PATCH 10/45] serve: write fetch/error/nodeHTTP handler values into wrapper slots The slots are populated alongside the existing routeList write after ptr_to_js. ServerConfig still holds Strong handles; the slot writes are additive until the Strong fields are dropped in a follow-up. wsHandlers, allClosedPromise and onClientError are written from their respective set sites in later commits. --- src/runtime/api/BunObject.rs | 15 ++++++++++++++ src/runtime/server/mod.rs | 39 ++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 36ff7be17128..a364d66eb9ec 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1648,6 +1648,21 @@ pub(crate) fn serve(global_object: &JSGlobalObject, callframe: &CallFrame) -> Js // `server_body` until per-type codegen externs land. <$ServerType>::js_gc_route_list_set(obj, global_object, route_list_object); } + // Mirror the fetch/error/nodeHTTP handler callbacks into the + // wrapper's WriteBarrier slots so they trace from the wrapper. + // ServerConfig still holds Strong handles; the slot writes are + // additive until those are dropped in a follow-up. + // TODO: wsHandlers / allClosedPromise / onClientError are written + // from their respective set sites in later commits. + if let Some(h) = server_ref.config.on_request.as_ref() { + <$ServerType>::js_gc_on_request_set(obj, global_object, h.get()); + } + if let Some(h) = server_ref.config.on_error.as_ref() { + <$ServerType>::js_gc_on_error_set(obj, global_object, h.get()); + } + if let Some(h) = server_ref.config.on_node_http_request.as_ref() { + <$ServerType>::js_gc_on_node_http_request_set(obj, global_object, h.get()); + } server_ref.js_value.set_strong(obj, global_object); if global_object.bun_vm().test_isolation_enabled { diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 821cd7360aee..b1c37c5a046b 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -2963,6 +2963,45 @@ macro_rules! cached_value_dispatch { }; } +impl NewServer { + cached_value_dispatch!( + js_gc_on_request_get, + js_gc_on_request_set, + on_request_get_cached, + on_request_set_cached + ); + cached_value_dispatch!( + js_gc_on_error_get, + js_gc_on_error_set, + on_error_get_cached, + on_error_set_cached + ); + cached_value_dispatch!( + js_gc_on_node_http_request_get, + js_gc_on_node_http_request_set, + on_node_h_t_t_p_request_get_cached, + on_node_h_t_t_p_request_set_cached + ); + cached_value_dispatch!( + js_gc_on_client_error_get, + js_gc_on_client_error_set, + on_client_error_get_cached, + on_client_error_set_cached + ); + cached_value_dispatch!( + js_gc_ws_handlers_get, + js_gc_ws_handlers_set, + ws_handlers_get_cached, + ws_handlers_set_cached + ); + cached_value_dispatch!( + js_gc_all_closed_promise_get, + js_gc_all_closed_promise_set, + all_closed_promise_get_cached, + all_closed_promise_set_cached + ); +} + // ─── extern "C" trampolines ────────────────────────────────────────────────── // Monomorphized on the const-generic server params; // the bodies downcast `user_data` and forward into the typed method. From efd9e96948faabf0c64eb30ff224008a5e6d027d Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:30:16 -0700 Subject: [PATCH 11/45] serve: use individual ws handler slots, not a wsHandlers object with_async_context_if_needed wraps each callback in a fresh function; storing the original websocket: {...} object would not root the wrappers (the H2FrameParser bug a5af485354 fixed). Match rdr-impl. --- src/runtime/server/mod.rs | 8 ++++---- src/runtime/server/server.classes.ts | 8 +++++++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index b1c37c5a046b..d30c18dd0b31 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -2920,22 +2920,22 @@ use server_body::{Bun__ServerRouteList__callRoute, Bun__ServerRouteList__create} mod route_list_cached { pub(super) mod http { bun_jsc::codegen_cached_accessors!( - "HTTPServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsHandlers, allClosedPromise + "HTTPServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong, allClosedPromise ); } pub(super) mod https { bun_jsc::codegen_cached_accessors!( - "HTTPSServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsHandlers, allClosedPromise + "HTTPSServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong, allClosedPromise ); } pub(super) mod debug_http { bun_jsc::codegen_cached_accessors!( - "DebugHTTPServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsHandlers, allClosedPromise + "DebugHTTPServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong, allClosedPromise ); } pub(super) mod debug_https { bun_jsc::codegen_cached_accessors!( - "DebugHTTPSServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsHandlers, allClosedPromise + "DebugHTTPSServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong, allClosedPromise ); } } diff --git a/src/runtime/server/server.classes.ts b/src/runtime/server/server.classes.ts index bd0a26dc39a5..c319c87f279e 100644 --- a/src/runtime/server/server.classes.ts +++ b/src/runtime/server/server.classes.ts @@ -96,7 +96,13 @@ function generate(name) { "onError", "onNodeHTTPRequest", "onClientError", - "wsHandlers", + "wsOnOpen", + "wsOnMessage", + "wsOnClose", + "wsOnDrain", + "wsOnError", + "wsOnPing", + "wsOnPong", "allClosedPromise", ], }); From 58ab885bafd8c93075b9627c2b4653dfbdc4cd33 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:36:08 -0700 Subject: [PATCH 12/45] =?UTF-8?q?serve:=20trace=20JSServerWebSocket=20?= =?UTF-8?q?=E2=86=92=20server=20wrapper=20via=20m=5Fserver=20slot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unused m_socket slot is repurposed. Handler gains an AnyServer backref (same field PR2 adds for the on_close trigger) so init can reach the server wrapper. With the handler callbacks moving to wrapper slots, this edge keeps the server wrapper — and its m_ws* handler slots — reachable while any websocket is connected. --- src/runtime/server/ServerWebSocket.rs | 10 ++++++++-- src/runtime/server/WebSocketServerContext.rs | 6 ++++++ src/runtime/server/mod.rs | 7 +++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/runtime/server/ServerWebSocket.rs b/src/runtime/server/ServerWebSocket.rs index 5056568580af..1c47f9e5cd15 100644 --- a/src/runtime/server/ServerWebSocket.rs +++ b/src/runtime/server/ServerWebSocket.rs @@ -147,9 +147,9 @@ impl Flags { // (`generate-classes.ts` → `${T}__data{Get,Set}Cached`). #[allow(non_snake_case)] pub mod js { - // Emits `data_{get,set}_cached`. Getter maps `JSValue::ZERO` → `None`; + // Emits `{data,server}_{get,set}_cached`. Getter maps `JSValue::ZERO` → `None`; // setter forwards through the JSC `WriteBarrier` slot. - ::bun_jsc::codegen_cached_accessors!("ServerWebSocket"; data); + ::bun_jsc::codegen_cached_accessors!("ServerWebSocket"; data, server); } /// Maps a uWS `SendStatus` to the JS-visible number contract shared by every @@ -361,6 +361,12 @@ impl ServerWebSocket { .this_value .set(JsRef::init_strong(this_value, global_object)); js::data_set_cached(this_value, global_object, data_value); + if let Some(server) = handler.server { + let server_js = server.js_value(); + if !server_js.is_undefined() { + js::server_set_cached(this_value, global_object, server_js); + } + } this } diff --git a/src/runtime/server/WebSocketServerContext.rs b/src/runtime/server/WebSocketServerContext.rs index c643d5ce6c86..7f615f20ee38 100644 --- a/src/runtime/server/WebSocketServerContext.rs +++ b/src/runtime/server/WebSocketServerContext.rs @@ -29,6 +29,11 @@ pub struct Handler { pub on_pong: JSValue, pub app: Option<*mut c_void>, + /// Backref to the owning server. Set in `set_routes()` alongside `app`. + /// Used by `ServerWebSocket::init` to write the server JS wrapper into the + /// per-socket `m_server` traced slot, so the wrapper (and the `m_ws*` + /// handler slots it carries) stays reachable while any socket is connected. + pub server: Option, // Always set manually. // LIFETIMES.tsv = STATIC (vm) / JSC_BORROW (global_object) — both outlive the handler. @@ -105,6 +110,7 @@ impl Handler { on_ping: JSValue::ZERO, on_pong: JSValue::ZERO, app: None, + server: None, vm: bun_ptr::BackRef::new(VirtualMachine::get()), global_object: bun_ptr::BackRef::new(global_object), server: None, diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index d30c18dd0b31..f4b478ca1743 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -3565,6 +3565,13 @@ impl AnyServer { any_server_dispatch!(self, |s| &s.config) } + /// The server's JS wrapper object, or `UNDEFINED` if the `JsRef` has been + /// downgraded and the wrapper collected (post-`stop()` with no user refs). + #[inline] + pub fn js_value(&self) -> JSValue { + any_server_dispatch!(self, |s| s.js_value.try_get().unwrap_or(JSValue::UNDEFINED)) + } + pub fn h3_alt_svc(&self) -> Option<&[u8]> { match self.tag { AnyServerTag::HTTPSServer => self.as_https().h3_alt_svc(), From 71dac52b080746ebed97594cadec52f286089e38 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:38:41 -0700 Subject: [PATCH 13/45] serve: drop ServerConfig handler Strongs; read shadows, root via wrapper The wrapper slot is now the sole GC root for fetch/error/nodeHTTP handlers. ServerConfig holds the raw JSValue as a shadow for hot-path dispatch reads; reload writes both the shadow and the slot. --- src/runtime/api/BunObject.rs | 18 ++++---- src/runtime/server/RequestContext.rs | 6 +-- src/runtime/server/ServerConfig.rs | 32 ++++++------- src/runtime/server/mod.rs | 20 +++------ src/runtime/server/server_body.rs | 67 ++++++++++++++-------------- 5 files changed, 65 insertions(+), 78 deletions(-) diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index a364d66eb9ec..2f20509ab5c6 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1649,19 +1649,19 @@ pub(crate) fn serve(global_object: &JSGlobalObject, callframe: &CallFrame) -> Js <$ServerType>::js_gc_route_list_set(obj, global_object, route_list_object); } // Mirror the fetch/error/nodeHTTP handler callbacks into the - // wrapper's WriteBarrier slots so they trace from the wrapper. - // ServerConfig still holds Strong handles; the slot writes are - // additive until those are dropped in a follow-up. + // wrapper's WriteBarrier slots — the wrapper is the sole GC root + // for these; `ServerConfig` only holds a raw `JSValue` shadow for + // hot-path dispatch reads. // TODO: wsHandlers / allClosedPromise / onClientError are written // from their respective set sites in later commits. - if let Some(h) = server_ref.config.on_request.as_ref() { - <$ServerType>::js_gc_on_request_set(obj, global_object, h.get()); + if !server_ref.config.on_request.is_empty() { + <$ServerType>::js_gc_on_request_set(obj, global_object, server_ref.config.on_request); } - if let Some(h) = server_ref.config.on_error.as_ref() { - <$ServerType>::js_gc_on_error_set(obj, global_object, h.get()); + if !server_ref.config.on_error.is_empty() { + <$ServerType>::js_gc_on_error_set(obj, global_object, server_ref.config.on_error); } - if let Some(h) = server_ref.config.on_node_http_request.as_ref() { - <$ServerType>::js_gc_on_node_http_request_set(obj, global_object, h.get()); + if !server_ref.config.on_node_http_request.is_empty() { + <$ServerType>::js_gc_on_node_http_request_set(obj, global_object, server_ref.config.on_node_http_request); } server_ref.js_value.set_strong(obj, global_object); diff --git a/src/runtime/server/RequestContext.rs b/src/runtime/server/RequestContext.rs index bbaeac780aa9..a0aef20b5eaf 100644 --- a/src/runtime/server/RequestContext.rs +++ b/src/runtime/server/RequestContext.rs @@ -3188,12 +3188,10 @@ where if let Some(server) = self.server { // SAFETY: BACKREF let server = &*server; - if let Some(on_error) = server.config().on_error.as_ref() - && !self.flags.has_called_error_handler() - { + let on_error = server.config().on_error; + if !on_error.is_empty() && !self.flags.has_called_error_handler() { self.flags.set_has_called_error_handler(true); let result = on_error - .get() .call( server.global_this(), server.js_value().try_get().unwrap_or(JSValue::UNDEFINED), diff --git a/src/runtime/server/ServerConfig.rs b/src/runtime/server/ServerConfig.rs index 590eb66ba303..07f48ecc3213 100644 --- a/src/runtime/server/ServerConfig.rs +++ b/src/runtime/server/ServerConfig.rs @@ -45,9 +45,12 @@ pub struct ServerConfig { /// If HMR is not enabled, then this field is ignored. pub enable_chrome_devtools_automatic_workspace_folders: bool, - pub on_error: Option, - pub on_request: Option, - pub on_node_http_request: Option, + /// Raw shadow of the wrapper's `onError`/`onRequest`/`onNodeHTTPRequest` + /// WriteBarrier slots. The wrapper JSCell is the GC root; these are + /// `JSValue::ZERO` when unset and copied for hot-path dispatch reads. + pub on_error: JSValue, + pub on_request: JSValue, + pub on_node_http_request: JSValue, pub websocket: Option, @@ -81,9 +84,9 @@ impl Default for ServerConfig { development: DevelopmentOption::Development, broadcast_console_log_from_browser_to_server_for_bake: false, enable_chrome_devtools_automatic_workspace_folders: true, - on_error: None, - on_request: None, - on_node_http_request: None, + on_error: JSValue::ZERO, + on_request: JSValue::ZERO, + on_node_http_request: JSValue::ZERO, websocket: None, reuse_port: false, id: Box::default(), @@ -274,9 +277,9 @@ impl ServerConfig { .broadcast_console_log_from_browser_to_server_for_bake, enable_chrome_devtools_automatic_workspace_folders: self .enable_chrome_devtools_automatic_workspace_folders, - on_error: self.on_error.take(), - on_request: self.on_request.take(), - on_node_http_request: self.on_node_http_request.take(), + on_error: self.on_error, + on_request: self.on_request, + on_node_http_request: self.on_node_http_request, websocket: self.websocket.take(), reuse_port: self.reuse_port, id: core::mem::take(&mut self.id), @@ -1270,8 +1273,7 @@ impl ServerConfig { global.throw_invalid_arguments(format_args!("Expected error to be a function")) ); } - let on_error_snapshot = on_error.with_async_context_if_needed(global); - args.on_error = Some(Strong::create(on_error_snapshot, global)); + args.on_error = on_error.with_async_context_if_needed(global); } if global.has_exception() { return Err(JsError::Thrown); @@ -1283,8 +1285,7 @@ impl ServerConfig { "Expected onNodeHTTPRequest to be a function", ))); } - let on_request = on_request_.with_async_context_if_needed(global); - args.on_node_http_request = Some(Strong::create(on_request, global)); + args.on_node_http_request = on_request_.with_async_context_if_needed(global); } if let Some(on_request_) = arg.get_truthy(global, "fetch")? { @@ -1292,10 +1293,9 @@ impl ServerConfig { return Err(global .throw_invalid_arguments(format_args!("Expected fetch() to be a function"))); } - let on_request = on_request_.with_async_context_if_needed(global); - args.on_request = Some(Strong::create(on_request, global)); + args.on_request = on_request_.with_async_context_if_needed(global); } else if args.bake.is_none() - && args.on_node_http_request.is_none() + && args.on_node_http_request.is_empty() && ((args.static_routes.len() + args.user_routes_to_build.len()) == 0 && !opts.has_user_routes) && opts.is_fetch_required diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index f4b478ca1743..ad71efb0bf80 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -1066,12 +1066,7 @@ impl NewServer { // SAFETY: `this` is the live server backref for this request. let server = unsafe { &*this }; - let on_request = server - .config - .on_request - .as_ref() - .map(|s| s.get()) - .unwrap_or(JSValue::ZERO); + let on_request = server.config.on_request; debug_assert!(!on_request.is_empty()); let global = server.global_this(); @@ -1239,12 +1234,7 @@ impl NewServer { }, None => JSValue::UNDEFINED, }; - let callback = this_ref - .config - .on_node_http_request - .as_ref() - .map(|s| s.get()) - .unwrap_or(JSValue::ZERO); + let callback = this_ref.config.on_node_http_request; // C++ forwards `any_server` to `NodeHTTPResponse::create`, which // unpacks it via `any_server_from_packed` (bits 49..64 = variant tag); // a raw `*mut Self` would zero those bits and trip the dispatch @@ -2397,8 +2387,8 @@ impl NewServer { // --- 9. Consolidated "/*" HTTP fallback registration --- let ud = self_ptr.cast::(); - let has_node_http = self.config.on_node_http_request.is_some(); - let has_on_request = self.config.on_request.is_some(); + let has_node_http = !self.config.on_node_http_request.is_empty(); + let has_on_request = !self.config.on_request.is_empty(); if star_methods_covered_by_user == http_method::Set::all() { // User/Static/Dev has already provided a "/*" handler for ALL methods. // No further global "/*" HTTP fallback needed. @@ -2720,7 +2710,7 @@ impl NewServer { route_list_value = unsafe { &mut *this }.set_routes(); } - if this_ref.config.on_node_http_request.is_some() { + if !this_ref.config.on_node_http_request.is_empty() { // SAFETY: `this` is the live boxed server from `init()`; no other borrow is live. unsafe { &mut *this }.set_using_custom_expect_handler(true); } diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index f67a5e3970c8..628469702e5f 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -26,7 +26,7 @@ use bun_jsc::ZigStringJsc as _; use bun_jsc::uuid::UUID; use bun_jsc::{ self as jsc, ArrayBuffer, CallFrame, GlobalRef, JSGlobalObject, JSPromise, JSValue, JsError, - JsResult, Node, StringJsc as _, Strong, StrongOptional, VirtualMachine, host_fn, + JsResult, Node, StringJsc as _, StrongOptional, VirtualMachine, host_fn, }; use bun_paths as paths; use bun_ptr::RefPtr; @@ -2160,29 +2160,33 @@ where } // Only reload `on_request` / `on_error` when the new config actually - // specifies one. `Option` drops the old handle (= JSValue.unprotect()). - if new_config - .on_request - .as_ref() - .is_some_and(|s| !s.get().is_undefined()) - { - self.config.on_request = new_config.on_request.take(); + // specifies one. The shadow `JSValue` is updated alongside the + // wrapper's WriteBarrier slot (the GC root); both must agree. + let server_js = self.js_value.try_get().filter(|v| !v.is_empty()); + if !new_config.on_request.is_empty_or_undefined_or_null() { + self.config.on_request = new_config.on_request; + if let Some(server_js) = server_js { + Self::js_gc_on_request_set(server_js, global, new_config.on_request); + } } // Swap on any change, *including* clearing to `.zero` when the reload // config omits the handler, so subsequent `on_web_socket_upgrade` / - // `set_routes` stop routing through the node:http path. `take()` yields - // `None` when the new config omitted it; assignment drops the old Strong. - if self.config.on_node_http_request.as_ref().map(Strong::get) - != new_config.on_node_http_request.as_ref().map(Strong::get) - { - self.config.on_node_http_request = new_config.on_node_http_request.take(); + // `set_routes` stop routing through the node:http path. + if self.config.on_node_http_request != new_config.on_node_http_request { + self.config.on_node_http_request = new_config.on_node_http_request; + if let Some(server_js) = server_js { + Self::js_gc_on_node_http_request_set( + server_js, + global, + new_config.on_node_http_request, + ); + } } - if new_config - .on_error - .as_ref() - .is_some_and(|s| !s.get().is_undefined()) - { - self.config.on_error = new_config.on_error.take(); + if !new_config.on_error.is_empty_or_undefined_or_null() { + self.config.on_error = new_config.on_error; + if let Some(server_js) = server_js { + Self::js_gc_on_error_set(server_js, global, new_config.on_error); + } } if let Some(mut ws) = new_config.websocket.take() { @@ -2300,7 +2304,7 @@ where pub fn on_fetch(&mut self, ctx: &JSGlobalObject, callframe: &CallFrame) -> JsResult { jsc::mark_binding!(); - if self.config.on_request.is_none() { + if self.config.on_request.is_empty() { return Ok( JSPromise::dangerously_create_rejected_promise_value_without_notifying_vm( ctx, @@ -2439,9 +2443,9 @@ where // local going out of scope does not also drop it (double-free / UAF). let request: *mut Request = bun_core::heap::into_raw(existing_request); - debug_assert!(self.config.on_request.is_some()); // confirmed above + debug_assert!(!self.config.on_request.is_empty()); // confirmed above let global_this = self.global(); - let on_request = self.config.on_request.as_ref().unwrap().get(); + let on_request = self.config.on_request; // SAFETY: `request` was just allocated via `heap::alloc`; ownership // transfers to the JS wrapper inside `to_js`. let request_value = unsafe { (*request).to_js(&global_this) }; @@ -2710,7 +2714,7 @@ where if !Self::HAS_H3 { unreachable!(); } - if self.config.on_request.is_none() { + if self.config.on_request.is_empty() { return Self::on_h3_404(self, req, resp); } self.on_request_for::>(req, resp); @@ -2917,16 +2921,11 @@ where // (`config.on_request`, `global_this`, `js_value`) are disjoint from // the request/ctx allocations it references. Reborrow to satisfy NLL. let this = unsafe { &mut *self_ptr }; - debug_assert!(this.config.on_request.is_some()); + debug_assert!(!this.config.on_request.is_empty()); let global = this.global_this(); let js_value = this.js_value_assert_alive(); - let on_request_fn = this - .config - .on_request - .as_ref() - .map(|s| s.get()) - .unwrap_or(JSValue::UNDEFINED); + let on_request_fn = this.config.on_request; let response_value = match on_request_fn.call(global, js_value, &[prepared.js_request, js_value]) { Ok(v) => v, @@ -3287,13 +3286,13 @@ where resp.end_without_body(true); return; } - if this.config.on_node_http_request.is_some() { + if !this.config.on_node_http_request.is_empty() { // NOTE: receiver is `*mut Self` (mod.rs) — the callee re-enters // JS, so a long-lived `&mut self` here would alias on callback. Self::on_node_http_request_with_upgrade_ctx(self_ptr, req, resp, upgrade_ctx); return; } - if this.config.on_request.is_none() { + if this.config.on_request.is_empty() { // require fetch method to be set otherwise we dont know what route to call // this should be the fallback in case no route is provided to upgrade resp.write_status(b"403 Forbidden"); @@ -3354,7 +3353,7 @@ where let request_value = args[0]; request_value.ensure_still_alive(); - let response_value = match this.config.on_request.as_ref().unwrap().get().call( + let response_value = match this.config.on_request.call( &global, this.js_value_assert_alive(), &args, From d6440ae87f0ecff1519ef1054ebeeb4fa95ed485 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:48:45 -0700 Subject: [PATCH 14/45] serve: write ws handler shadows into wrapper slots; drop gcProtect --- src/runtime/api/BunObject.rs | 10 ++-- src/runtime/server/WebSocketServerContext.rs | 33 ---------- src/runtime/server/mod.rs | 63 ++++++++++++++++++-- src/runtime/server/server_body.rs | 12 ++-- 4 files changed, 68 insertions(+), 50 deletions(-) diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 2f20509ab5c6..e5a2ae323396 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1648,12 +1648,9 @@ pub(crate) fn serve(global_object: &JSGlobalObject, callframe: &CallFrame) -> Js // `server_body` until per-type codegen externs land. <$ServerType>::js_gc_route_list_set(obj, global_object, route_list_object); } - // Mirror the fetch/error/nodeHTTP handler callbacks into the - // wrapper's WriteBarrier slots — the wrapper is the sole GC root - // for these; `ServerConfig` only holds a raw `JSValue` shadow for - // hot-path dispatch reads. - // TODO: wsHandlers / allClosedPromise / onClientError are written - // from their respective set sites in later commits. + // Mirror the handler callbacks into the wrapper's WriteBarrier + // slots — the wrapper is the sole GC root for these; `ServerConfig` + // / `Handler` only hold raw `JSValue` shadows for hot-path dispatch. if !server_ref.config.on_request.is_empty() { <$ServerType>::js_gc_on_request_set(obj, global_object, server_ref.config.on_request); } @@ -1663,6 +1660,7 @@ pub(crate) fn serve(global_object: &JSGlobalObject, callframe: &CallFrame) -> Js if !server_ref.config.on_node_http_request.is_empty() { <$ServerType>::js_gc_on_node_http_request_set(obj, global_object, server_ref.config.on_node_http_request); } + server_ref.write_ws_handler_slots(obj, global_object); server_ref.js_value.set_strong(obj, global_object); if global_object.bun_vm().test_isolation_enabled { diff --git a/src/runtime/server/WebSocketServerContext.rs b/src/runtime/server/WebSocketServerContext.rs index 7f615f20ee38..02007ac5c82f 100644 --- a/src/runtime/server/WebSocketServerContext.rs +++ b/src/runtime/server/WebSocketServerContext.rs @@ -155,30 +155,6 @@ impl Handler { "WebSocketServerContext expects a message handler" ))) } - - pub fn protect(&self) { - self.on_open.protect(); - self.on_message.protect(); - self.on_close.protect(); - self.on_drain.protect(); - self.on_error.protect(); - self.on_ping.protect(); - self.on_pong.protect(); - } - - pub fn unprotect(&self) { - if self.vm.is_shutting_down() { - return; - } - - self.on_open.unprotect(); - self.on_message.unprotect(); - self.on_close.unprotect(); - self.on_drain.unprotect(); - self.on_error.unprotect(); - self.on_ping.unprotect(); - self.on_pong.unprotect(); - } } impl WebSocketServerContext { @@ -195,14 +171,6 @@ impl WebSocketServerContext { ..Default::default() } } - - pub fn protect(&self) { - self.handler.protect(); - } - - pub fn unprotect(&self) { - self.handler.unprotect(); - } } bun_core::comptime_string_map! { @@ -422,6 +390,5 @@ pub(crate) fn on_create( } } - server.protect(); Ok(server) } diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index ad71efb0bf80..0aef122528b8 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -2979,10 +2979,46 @@ impl NewServer { on_client_error_set_cached ); cached_value_dispatch!( - js_gc_ws_handlers_get, - js_gc_ws_handlers_set, - ws_handlers_get_cached, - ws_handlers_set_cached + js_gc_ws_on_open_get, + js_gc_ws_on_open_set, + ws_on_open_get_cached, + ws_on_open_set_cached + ); + cached_value_dispatch!( + js_gc_ws_on_message_get, + js_gc_ws_on_message_set, + ws_on_message_get_cached, + ws_on_message_set_cached + ); + cached_value_dispatch!( + js_gc_ws_on_close_get, + js_gc_ws_on_close_set, + ws_on_close_get_cached, + ws_on_close_set_cached + ); + cached_value_dispatch!( + js_gc_ws_on_drain_get, + js_gc_ws_on_drain_set, + ws_on_drain_get_cached, + ws_on_drain_set_cached + ); + cached_value_dispatch!( + js_gc_ws_on_error_get, + js_gc_ws_on_error_set, + ws_on_error_get_cached, + ws_on_error_set_cached + ); + cached_value_dispatch!( + js_gc_ws_on_ping_get, + js_gc_ws_on_ping_set, + ws_on_ping_get_cached, + ws_on_ping_set_cached + ); + cached_value_dispatch!( + js_gc_ws_on_pong_get, + js_gc_ws_on_pong_set, + ws_on_pong_get_cached, + ws_on_pong_set_cached ); cached_value_dispatch!( js_gc_all_closed_promise_get, @@ -2990,6 +3026,25 @@ impl NewServer { all_closed_promise_get_cached, all_closed_promise_set_cached ); + + /// Mirror all 7 `Handler.on_*` shadows into the wrapper's `m_wsOn*` + /// WriteBarrier slots. Writes all slots unconditionally — `JSValue::ZERO` + /// clears, so a reload that omits a callback drops the previous root (G6). + /// Called after `ptr_to_js` in `serve()` and after the websocket-context + /// swap in `on_reload_from_zig`; dispatch keeps reading the shadow. + pub fn write_ws_handler_slots(&self, server_js: JSValue, global: &JSGlobalObject) { + let Some(ws) = self.config.websocket.as_ref() else { + return; + }; + let h = &ws.handler; + Self::js_gc_ws_on_open_set(server_js, global, h.on_open); + Self::js_gc_ws_on_message_set(server_js, global, h.on_message); + Self::js_gc_ws_on_close_set(server_js, global, h.on_close); + Self::js_gc_ws_on_drain_set(server_js, global, h.on_drain); + Self::js_gc_ws_on_error_set(server_js, global, h.on_error); + Self::js_gc_ws_on_ping_set(server_js, global, h.on_ping); + Self::js_gc_ws_on_pong_set(server_js, global, h.on_pong); + } } // ─── extern "C" trampolines ────────────────────────────────────────────────── diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 628469702e5f..1a127d7479cd 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2194,16 +2194,14 @@ where .flags .set(super::web_socket_server_context::HandlerFlags::SSL, SSL); if !ws.handler.on_message.is_empty() || !ws.handler.on_open.is_empty() { - if let Some(old_ws) = self.config.websocket.as_ref() { - old_ws.unprotect(); - } ws.global_object = bun_ptr::BackRef::new(global); self.config.websocket = Some(ws); - } else { - // Not adopting it: release the protections taken in - // `WebSocketServerContext::on_create` so the handlers don't leak. - ws.unprotect(); + if let Some(server_js) = server_js { + self.write_ws_handler_slots(server_js, global); + } } + // Not adopting it: nothing to release — handler callbacks are + // unrooted shadows until written into the wrapper's `m_wsOn*` slots. } // These get re-applied when we set the static routes again. From 44f84149e254d3a572705353e71521cc9522a4b7 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:49:25 -0700 Subject: [PATCH 15/45] serve: store on_clienterror in wrapper slot, drop StrongOptional --- src/runtime/server/mod.rs | 10 ++++++---- src/runtime/server/server_body.rs | 9 +++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 0aef122528b8..6f48dc39ac57 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -296,7 +296,9 @@ pub struct NewServer { /// times due to SNI, so we have to store them. pub user_routes: Vec>, - pub on_clienterror: jsc::StrongOptional, + /// Raw shadow of the wrapper's `m_onClientError` WriteBarrier slot. + /// `JSValue::ZERO` when unset; written by `server_set_on_client_error_`. + pub on_clienterror: JSValue, pub inspector_server_id: jsc::DebuggerId, } @@ -310,7 +312,7 @@ pub struct UserRoute { impl Drop for NewServer { fn drop(&mut self) { // The remaining owned fields (config, base_url, h3_alt_svc, dev_server, - // user_routes, all_closed_promise, on_clienterror) drop automatically. + // user_routes, all_closed_promise) drop automatically. if let Some(p) = self.plugins.take() { // SAFETY: `plugins` carries the `heap::alloc` provenance from // `ServePlugins::init`; this releases the server's counted ref. @@ -1895,7 +1897,7 @@ impl NewServer { } // owned-field cleanup (all_closed_promise / user_routes / - // config / on_clienterror / h3_alt_svc / dev_server / plugins) is + // config / h3_alt_svc / dev_server / plugins) is // handled by the heap::take drop below — see `impl Drop for NewServer`. if Self::HAS_H3 { if let Some(h3a) = this_ref.h3_app.take() { @@ -1962,7 +1964,7 @@ impl NewServer { flags: ServerFlags::default(), plugins: None, user_routes: Vec::new(), - on_clienterror: jsc::StrongOptional::empty(), + on_clienterror: JSValue::ZERO, inspector_server_id: jsc::DebuggerId::init(0), })); diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 1a127d7479cd..e5d696b7010e 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -3504,9 +3504,10 @@ where error_code: u8, raw_packet: &[u8], ) { - let Some(callback) = self.on_clienterror.get() else { + let callback = self.on_clienterror; + if callback.is_empty() { return; - }; + } { let is_ssl = SSL; let global = self.global(); @@ -3651,8 +3652,8 @@ pub(super) fn server_set_on_client_error_( // SAFETY: as_ returned a non-null *mut to a live server. let this = unsafe { &mut *this }; if let Some(app) = this.app { - this.on_clienterror.deinit(); - this.on_clienterror = StrongOptional::create(callback, global); + this.on_clienterror = callback; + <$T>::js_gc_on_client_error_set(server, global, callback); // uws_sys::App::on_client_error takes the raw C-ABI handler shape; // wrap our typed callback in an extern "C" thunk that slices raw_packet. extern "C" fn thunk( From ce6266873c6c64e1bdfb4ac24c532f6647cfc0db Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:50:41 -0700 Subject: [PATCH 16/45] serve: store all_closed_promise in wrapper slot, drop JSPromiseStrong field --- src/runtime/server/mod.rs | 32 ++++++++++++++++--------------- src/runtime/server/server_body.rs | 15 +++++++++------ 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 6f48dc39ac57..084b41606a91 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -275,7 +275,11 @@ pub struct NewServer { /// allocated when `!SSL`. Kept as a raw nullable pointer rather than a /// conditional field so the struct stays uniform across monomorphizations. pub h3_request_pool: *mut request_context::RequestContextStackAllocator, - pub all_closed_promise: jsc::JSPromiseStrong, + /// Raw shadow of the wrapper's `m_allClosedPromise` WriteBarrier slot. + /// `JSValue::ZERO` until `server.stop()` lazily creates the promise. The + /// shadow lets `deinit_if_we_can` resolve it after the wrapper is gone + /// (the user's `await` keeps the promise itself alive in that window). + pub all_closed_promise: JSValue, pub listen_callback: jsc::AnyTask::AnyTask, // allocator field dropped — global mimalloc per §Allocators @@ -312,7 +316,7 @@ pub struct UserRoute { impl Drop for NewServer { fn drop(&mut self) { // The remaining owned fields (config, base_url, h3_alt_svc, dev_server, - // user_routes, all_closed_promise) drop automatically. + // user_routes) drop automatically. if let Some(p) = self.plugins.take() { // SAFETY: `plugins` carries the `heap::alloc` provenance from // `ServePlugins::init`; this releases the server's counted ref. @@ -1660,10 +1664,10 @@ impl NewServer { }, self.flags .contains(ServerFlags::HAS_HANDLED_ALL_CLOSED_PROMISE), - if self.all_closed_promise.has_value() { - "has" - } else { + if self.all_closed_promise.is_empty() { "no" + } else { + "has" }, matches!(self.js_value, jsc::JsRef::Finalized), ); @@ -1674,7 +1678,7 @@ impl NewServer { && !self .flags .contains(ServerFlags::HAS_HANDLED_ALL_CLOSED_PROMISE) - && self.all_closed_promise.has_value() + && !self.all_closed_promise.is_empty() // `ServerAllConnectionsClosedTask::run_from_js_thread` early-returns // (without resolving the promise) when the VM is shutting down — // see the `if !vm.is_shutting_down()` gate there. Skip the @@ -1694,11 +1698,9 @@ impl NewServer { ServerAllConnectionsClosedTask::schedule( ServerAllConnectionsClosedTask { global_object: self.global_this, - // Duplicate the Strong handle so that we can hold two independent strong references to it. - promise: jsc::JSPromiseStrong::from_value( - self.all_closed_promise.value(), - global, - ), + // Task-local Strong: short-lived (one event-loop hop), and + // the only root once the wrapper's slot is gone. + promise: jsc::JSPromiseStrong::from_value(self.all_closed_promise, global), tracker: jsc::AsyncTaskTracker::init(vm_ref), }, vm_ref, @@ -1896,9 +1898,9 @@ impl NewServer { } } - // owned-field cleanup (all_closed_promise / user_routes / - // config / h3_alt_svc / dev_server / plugins) is - // handled by the heap::take drop below — see `impl Drop for NewServer`. + // owned-field cleanup (user_routes / config / h3_alt_svc / dev_server / + // plugins) is handled by the heap::take drop below — see `impl Drop for + // NewServer`. if Self::HAS_H3 { if let Some(h3a) = this_ref.h3_app.take() { // SAFETY: live H3::App handle owned by this server. @@ -1955,7 +1957,7 @@ impl NewServer { // the H3-listen path (`listen()` below) so HTTPS servers that // don't enable `config.http3` don't pay either. h3_request_pool: core::ptr::null_mut(), - all_closed_promise: jsc::JSPromiseStrong::default(), + all_closed_promise: JSValue::ZERO, listen_callback: jsc::AnyTask::AnyTask { ctx: None, callback: |_| Ok(()), diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index e5d696b7010e..6a746a00254c 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2697,12 +2697,15 @@ where if !self.has_listener() && self.pending_requests == 0 { return JSPromise::resolved_promise(global, JSValue::UNDEFINED).to_js(); } - let prom = &mut self.all_closed_promise; - if prom.has_value() { - return prom.value(); - } - *prom = jsc::JSPromiseStrong::init(global); - prom.value() + if !self.all_closed_promise.is_empty() { + return self.all_closed_promise; + } + let prom = JSPromise::create(global).to_js(); + self.all_closed_promise = prom; + // Only reachable via `server.stop()` (a JS-called method) — wrapper is + // alive. The slot is the GC root; the field above is the dispatch shadow. + Self::js_gc_all_closed_promise_set(self.js_value_assert_alive(), global, prom); + prom } // `notify_inspector_server_stopped` lives in the unbounded impl block From 0c230465049492d91c8b79014a6291159fd0bbf0 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:52:20 -0700 Subject: [PATCH 17/45] serve: wrap handlers with async context at slot-write, not at parse --- src/runtime/api/BunObject.rs | 16 ++++++-- src/runtime/server/ServerConfig.rs | 9 +++-- src/runtime/server/WebSocketServerContext.rs | 7 ++-- src/runtime/server/mod.rs | 42 +++++++++++++------- src/runtime/server/server_body.rs | 27 ++++++++----- 5 files changed, 67 insertions(+), 34 deletions(-) diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index e5a2ae323396..de907c2651ee 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1651,14 +1651,24 @@ pub(crate) fn serve(global_object: &JSGlobalObject, callframe: &CallFrame) -> Js // Mirror the handler callbacks into the wrapper's WriteBarrier // slots — the wrapper is the sole GC root for these; `ServerConfig` // / `Handler` only hold raw `JSValue` shadows for hot-path dispatch. + // The async-context wrap is applied here (not in `from_js`) so the + // freshly-allocated wrapper fn is rooted by the slot immediately; + // the unwrapped fn is held live by the user's options object on the + // `serve()` stack across `init`/`listen` until this point. if !server_ref.config.on_request.is_empty() { - <$ServerType>::js_gc_on_request_set(obj, global_object, server_ref.config.on_request); + let wrapped = server_ref.config.on_request.with_async_context_if_needed(global_object); + <$ServerType>::js_gc_on_request_set(obj, global_object, wrapped); + server_ref.config.on_request = wrapped; } if !server_ref.config.on_error.is_empty() { - <$ServerType>::js_gc_on_error_set(obj, global_object, server_ref.config.on_error); + let wrapped = server_ref.config.on_error.with_async_context_if_needed(global_object); + <$ServerType>::js_gc_on_error_set(obj, global_object, wrapped); + server_ref.config.on_error = wrapped; } if !server_ref.config.on_node_http_request.is_empty() { - <$ServerType>::js_gc_on_node_http_request_set(obj, global_object, server_ref.config.on_node_http_request); + let wrapped = server_ref.config.on_node_http_request.with_async_context_if_needed(global_object); + <$ServerType>::js_gc_on_node_http_request_set(obj, global_object, wrapped); + server_ref.config.on_node_http_request = wrapped; } server_ref.write_ws_handler_slots(obj, global_object); server_ref.js_value.set_strong(obj, global_object); diff --git a/src/runtime/server/ServerConfig.rs b/src/runtime/server/ServerConfig.rs index 07f48ecc3213..5564096b8020 100644 --- a/src/runtime/server/ServerConfig.rs +++ b/src/runtime/server/ServerConfig.rs @@ -1273,7 +1273,10 @@ impl ServerConfig { global.throw_invalid_arguments(format_args!("Expected error to be a function")) ); } - args.on_error = on_error.with_async_context_if_needed(global); + // Raw value — async-context wrapping is deferred to the slot-write + // site (`serve_with!` / `on_reload_from_zig`) so the wrapped fn is + // rooted by the wrapper's WriteBarrier slot the moment it exists. + args.on_error = on_error; } if global.has_exception() { return Err(JsError::Thrown); @@ -1285,7 +1288,7 @@ impl ServerConfig { "Expected onNodeHTTPRequest to be a function", ))); } - args.on_node_http_request = on_request_.with_async_context_if_needed(global); + args.on_node_http_request = on_request_; } if let Some(on_request_) = arg.get_truthy(global, "fetch")? { @@ -1293,7 +1296,7 @@ impl ServerConfig { return Err(global .throw_invalid_arguments(format_args!("Expected fetch() to be a function"))); } - args.on_request = on_request_.with_async_context_if_needed(global); + args.on_request = on_request_; } else if args.bake.is_none() && args.on_node_http_request.is_empty() && ((args.static_routes.len() + args.user_routes_to_build.len()) == 0 diff --git a/src/runtime/server/WebSocketServerContext.rs b/src/runtime/server/WebSocketServerContext.rs index 02007ac5c82f..f3fc3744509b 100644 --- a/src/runtime/server/WebSocketServerContext.rs +++ b/src/runtime/server/WebSocketServerContext.rs @@ -137,9 +137,10 @@ impl Handler { key ))); } - let cb = value.with_async_context_if_needed(global_object); - *field = cb; - cb.ensure_still_alive(); + // Raw value — async-context wrapping is deferred to + // `NewServer::write_ws_handler_slots` so the wrapped fn is + // rooted by the wrapper's WriteBarrier slot immediately. + *field = value; if i > 0 { // anything other than "error" is considered valid. valid = true; diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 084b41606a91..cf622afc13df 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -3032,22 +3032,36 @@ impl NewServer { ); /// Mirror all 7 `Handler.on_*` shadows into the wrapper's `m_wsOn*` - /// WriteBarrier slots. Writes all slots unconditionally — `JSValue::ZERO` - /// clears, so a reload that omits a callback drops the previous root (G6). - /// Called after `ptr_to_js` in `serve()` and after the websocket-context - /// swap in `on_reload_from_zig`; dispatch keeps reading the shadow. - pub fn write_ws_handler_slots(&self, server_js: JSValue, global: &JSGlobalObject) { - let Some(ws) = self.config.websocket.as_ref() else { + /// WriteBarrier slots, applying the async-context wrap (deferred from + /// `Handler::from_js` so the wrapped fn is rooted the moment it exists). + /// Writes all slots unconditionally — `JSValue::ZERO` clears, so a reload + /// that omits a callback drops the previous root (G6). Called after + /// `ptr_to_js` in `serve()` and after the websocket-context swap in + /// `on_reload_from_zig`; dispatch keeps reading the shadow. + pub fn write_ws_handler_slots(&mut self, server_js: JSValue, global: &JSGlobalObject) { + let Some(ws) = self.config.websocket.as_mut() else { return; }; - let h = &ws.handler; - Self::js_gc_ws_on_open_set(server_js, global, h.on_open); - Self::js_gc_ws_on_message_set(server_js, global, h.on_message); - Self::js_gc_ws_on_close_set(server_js, global, h.on_close); - Self::js_gc_ws_on_drain_set(server_js, global, h.on_drain); - Self::js_gc_ws_on_error_set(server_js, global, h.on_error); - Self::js_gc_ws_on_ping_set(server_js, global, h.on_ping); - Self::js_gc_ws_on_pong_set(server_js, global, h.on_pong); + let h = &mut ws.handler; + macro_rules! wrap_slot { + ($field:ident, $set:ident) => {{ + let raw = h.$field; + if raw.is_empty() { + Self::$set(server_js, global, JSValue::ZERO); + } else { + let wrapped = raw.with_async_context_if_needed(global); + Self::$set(server_js, global, wrapped); + h.$field = wrapped; + } + }}; + } + wrap_slot!(on_open, js_gc_ws_on_open_set); + wrap_slot!(on_message, js_gc_ws_on_message_set); + wrap_slot!(on_close, js_gc_ws_on_close_set); + wrap_slot!(on_drain, js_gc_ws_on_drain_set); + wrap_slot!(on_error, js_gc_ws_on_error_set); + wrap_slot!(on_ping, js_gc_ws_on_ping_set); + wrap_slot!(on_pong, js_gc_ws_on_pong_set); } } diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 6a746a00254c..ae1c31122167 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2161,32 +2161,37 @@ where // Only reload `on_request` / `on_error` when the new config actually // specifies one. The shadow `JSValue` is updated alongside the - // wrapper's WriteBarrier slot (the GC root); both must agree. + // wrapper's WriteBarrier slot (the GC root); both must agree. The + // async-context wrap is applied here (deferred from `from_js`) so the + // wrapped fn is rooted by the slot the moment it exists. let server_js = self.js_value.try_get().filter(|v| !v.is_empty()); if !new_config.on_request.is_empty_or_undefined_or_null() { - self.config.on_request = new_config.on_request; + let wrapped = new_config.on_request.with_async_context_if_needed(global); if let Some(server_js) = server_js { - Self::js_gc_on_request_set(server_js, global, new_config.on_request); + Self::js_gc_on_request_set(server_js, global, wrapped); } + self.config.on_request = wrapped; } // Swap on any change, *including* clearing to `.zero` when the reload // config omits the handler, so subsequent `on_web_socket_upgrade` / // `set_routes` stop routing through the node:http path. if self.config.on_node_http_request != new_config.on_node_http_request { - self.config.on_node_http_request = new_config.on_node_http_request; + let wrapped = if new_config.on_node_http_request.is_empty() { + JSValue::ZERO + } else { + new_config.on_node_http_request.with_async_context_if_needed(global) + }; if let Some(server_js) = server_js { - Self::js_gc_on_node_http_request_set( - server_js, - global, - new_config.on_node_http_request, - ); + Self::js_gc_on_node_http_request_set(server_js, global, wrapped); } + self.config.on_node_http_request = wrapped; } if !new_config.on_error.is_empty_or_undefined_or_null() { - self.config.on_error = new_config.on_error; + let wrapped = new_config.on_error.with_async_context_if_needed(global); if let Some(server_js) = server_js { - Self::js_gc_on_error_set(server_js, global, new_config.on_error); + Self::js_gc_on_error_set(server_js, global, wrapped); } + self.config.on_error = wrapped; } if let Some(mut ws) = new_config.websocket.take() { From f91ef8b54c40187d05563ff60d91a5f2dcd8ba85 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 12:54:54 -0700 Subject: [PATCH 18/45] serve: tests for the GC-traced handler redesign heapStats wrapper-count assertions for the cycle (handler closing over server is collected; control case unchanged), websocket-connected liveness, reload swap, and the AsyncLocalStorage init-window collectContinuously stress. --- test/bake/deinitialization.test.ts | 4 +- test/bake/fixtures/deinitialization/test.ts | 56 ++- test/js/bun/http/bun-server.test.ts | 459 +++++++++++++++++++- 3 files changed, 514 insertions(+), 5 deletions(-) diff --git a/test/bake/deinitialization.test.ts b/test/bake/deinitialization.test.ts index 3a6f6772903f..9622fa7973bf 100644 --- a/test/bake/deinitialization.test.ts +++ b/test/bake/deinitialization.test.ts @@ -10,4 +10,6 @@ test("dev server deinitializes itself", () => { }); expect(result.signalCode).toBeUndefined(); expect(result.exitCode).toBe(0); -}); + // The child runs a whole `bun test` suite (nine GC-heavy cases plus leak + // reporting at exit), which takes longer than the 5s default under ASAN. +}, 60_000); diff --git a/test/bake/fixtures/deinitialization/test.ts b/test/bake/fixtures/deinitialization/test.ts index b6b41533a15c..f83f9822bce6 100644 --- a/test/bake/fixtures/deinitialization/test.ts +++ b/test/bake/fixtures/deinitialization/test.ts @@ -1,7 +1,7 @@ import { getDevServerDeinitCount } from "bun:internal-for-testing"; import html from "./index.html"; -import { expect, test } from "bun:test"; -import { fullGC } from "bun:jsc"; +import { afterAll, beforeAll, expect, test } from "bun:test"; +import { fullGC, heapStats } from "bun:jsc"; expect(process.cwd()).toBe(import.meta.dir); @@ -71,7 +71,15 @@ async function run({ closeActiveConnections = false, sendAnyRequests = true, web expect(fetch(server.url.origin, { keepalive: false })).rejects.toThrow("Unable to connect"); } - await main(); + try { + await main(); + } finally { + // The closure assigned to `globalThis.callback` inside `main()` captures + // `server`; left in place it roots the JS Server wrapper through every GC + // below, so the wrapper never finalizes and the native NewServer box (and + // everything its config owns) is still live at process exit. + globalThis.callback = undefined; + } if (closeActiveConnections) { await promise; @@ -109,6 +117,48 @@ const cases = [ { closeActiveConnections: true, sendAnyRequests: false, websocket: 8 }, ]; +function liveServerWrappers() { + const c = heapStats().objectTypeCounts; + return (c.HTTPServer ?? 0) + (c.DebugHTTPServer ?? 0) + (c.HTTPSServer ?? 0) + (c.DebugHTTPSServer ?? 0); +} + +async function drainServerWrappers(target: number) { + for (let i = 0; i < 10 && liveServerWrappers() > target; i++) { + Bun.gc(true); + fullGC(); + await new Promise(resolve => setTimeout(resolve, 100)); + } +} + +// `objectTypeCounts` includes the (lazily created) prototype object once the +// first server has been constructed. Create-and-stop one trivial server here +// so the prototype is materialized but the instance is freed; the afterAll +// check then asserts every dev-server case returns to this baseline (i.e. zero +// live wrapper instances and the native boxes were actually freed). Captured +// in beforeAll so the baseline exists even when a name filter skips the +// baseline test. +let serverWrapperBaseline = 0; +beforeAll(async () => { + await (async () => { + const server = Bun.serve({ port: 0, fetch: () => new Response("ok") }); + server.stop(true); + })(); + await drainServerWrappers(1); + serverWrapperBaseline = liveServerWrappers(); +}); + +test("baseline: stopped server wrapper collects", () => { + expect(serverWrapperBaseline).toBeLessThanOrEqual(1); +}); + +afterAll(async () => { + // Drain any deferred deinit task scheduled during the final case's GC, then + // assert every JS Server wrapper has actually been collected — i.e. the + // native NewServer boxes are freed, not just the embedded dev servers. + await drainServerWrappers(serverWrapperBaseline); + expect(liveServerWrappers()).toBe(serverWrapperBaseline); +}); + for (const { closeActiveConnections, sendAnyRequests, websocket } of cases) { test( "flags: " + diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index f67cd68d05eb..faf7930bcf17 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -1,6 +1,15 @@ import type { Server, ServerWebSocket, Socket } from "bun"; import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, bunRun, isWindows, rejectUnauthorizedScope, tempDirWithFiles, tls } from "harness"; +import { + bunEnv, + bunExe, + bunRun, + isWindows, + normalizeBunSnapshot, + rejectUnauthorizedScope, + tempDirWithFiles, + tls, +} from "harness"; import path from "path"; describe.concurrent("Server", () => { @@ -1661,3 +1670,451 @@ test("HEAD request for a Response with an S3 file body reports the object size a expect(stdout.trim()).toBe("s3-head-ok"); expect(exitCode).toBe(0); }); + +// Handler callbacks (fetch/error/websocket.*) are stored on the JS wrapper and +// traced by the GC rather than independently rooted. These tests lock in that +// reload/stop transitions never leave a window where a handler is collected +// while a dispatch path can still reach it. +describe("handler liveness across reload/stop", () => { + test("server.reload({ fetch }) swaps the handler for the next request", async () => { + using server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch() { + return new Response("first"); + }, + }); + + expect(await (await fetch(server.url)).text()).toBe("first"); + + server.reload({ + fetch() { + return new Response("second"); + }, + }); + // Drop any last reference the test frame holds to the old handler, then + // collect. The new handler must be the one the wrapper traces now. + Bun.gc(true); + + expect(await (await fetch(server.url)).text()).toBe("second"); + + // A second reload back-to-back must also take effect (catches a stale + // cached read of the previous slot value). + server.reload({ + fetch() { + return new Response("third"); + }, + }); + Bun.gc(true); + expect(await (await fetch(server.url)).text()).toBe("third"); + }); + + test("in-flight request completes with its handler after stop() + GC", async () => { + const received = Promise.withResolvers(); + const release = Promise.withResolvers(); + let handlerRan = 0; + + const server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + async fetch() { + handlerRan++; + received.resolve(); + await release.promise; + return new Response("in-flight-ok", { headers: { Connection: "close" } }); + }, + }); + + const responsePromise = fetch(server.url); + await received.promise; + + // stop() drops the listener while the request is mid-handler. The wrapper + // must remain live (pending_requests > 0) so the handler the request was + // dispatched into is still reachable. + const stopped = server.stop(); + Bun.gc(true); + + release.resolve(); + const body = await (await responsePromise).text(); + await stopped; + + expect(body).toBe("in-flight-ok"); + expect(handlerRan).toBe(1); + }); + + test("websocket close handler fires when stop() closes an open connection", async () => { + const opened = Promise.withResolvers(); + const serverClose = Promise.withResolvers<{ code: number; reason: string }>(); + const clientClose = Promise.withResolvers(); + + const server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch(req, server) { + if (server.upgrade(req)) return; + return new Response(null, { status: 404 }); + }, + websocket: { + open() { + opened.resolve(); + }, + message() {}, + close(_ws, code, reason) { + serverClose.resolve({ code, reason }); + }, + }, + }); + + const ws = new WebSocket(`ws://127.0.0.1:${server.port}/`); + ws.onclose = () => clientClose.resolve(); + await opened.promise; + + // Connection is open; force-stop the server. The wrapper must stay live + // long enough for the close callback (read off the wrapper) to fire. + Bun.gc(true); + const stopped = server.stop(true); + Bun.gc(true); + + const { code } = await serverClose.promise; + await clientClose.promise; + await stopped; + + // The invariant is that the close handler ran at all (it's read off the + // wrapper after stop()); the exact close code is uws's choice. + expect(typeof code).toBe("number"); + expect(code).toBeGreaterThanOrEqual(1000); + }); + + test("server.fetch() still dispatches to the handler after stop()", async () => { + const server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch(req) { + return new Response("via server.fetch: " + new URL(req.url).pathname); + }, + }); + + const url = `http://${server.hostname}:${server.port}/after-stop`; + + server.stop(); + Bun.gc(true); + + // No listener, but the JS wrapper is still on our stack — server.fetch() + // reads the handler off the wrapper, so it must still resolve. + const response = await server.fetch(url); + expect(await response.text()).toBe("via server.fetch: /after-stop"); + expect(response.status).toBe(200); + }); +}); + +// The native↔JS cycle: a handler that closes over `server` used to be +// uncollectable because ServerConfig held it as a Strong root. With handlers +// stored as WriteBarrier slots on the wrapper, the cycle is all-JS-heap and +// GC collects it once nothing else references the wrapper. +describe("handler GC tracing (heapStats wrapper-count)", () => { + test("server with handler closing over itself is collected after stop()", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* js */ ` + const { heapStats, fullGC } = require("bun:jsc"); + const live = () => heapStats().objectTypeCounts.DebugHTTPServer ?? 0; + + const before = live(); + await (async () => { + const server = Bun.serve({ + port: 0, + development: true, + // Closes over server — the cycle. + fetch: () => new Response("port " + server.port), + error: e => { server.stop(); return new Response(String(e)); }, + }); + const r = await fetch(server.url, { keepalive: false }); + if (!(await r.text()).startsWith("port ")) throw new Error("dispatch broke"); + server.stop(true); + })(); + // No live reference to server or its handlers from here. + for (let i = 0; i < 20 && live() > before; i++) { + Bun.gc(true); + fullGC(); + await new Promise(r => setTimeout(r, 50)); + } + console.log(JSON.stringify({ before, after: live() })); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + const { before, after } = JSON.parse(stdout.trim()); + // objectTypeCounts includes the prototype once the first server is created + // (1), so the post-stop count is before+1 when the instance was collected. + // On main this fails: the cycle keeps the instance alive (after = before+2). + expect(after).toBe(before + 1); + expect(exitCode).toBe(0); + }); + + // Control: a handler that does NOT close over server is collected on main + // today. This pins that the redesign doesn't regress the non-cycle case. + test("server with handler NOT closing over itself is collected (control)", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* js */ ` + const { heapStats, fullGC } = require("bun:jsc"); + const live = () => heapStats().objectTypeCounts.DebugHTTPServer ?? 0; + const before = live(); + await (async () => { + const server = Bun.serve({ + port: 0, development: true, + fetch: () => new Response("ok"), + }); + await fetch(server.url, { keepalive: false }); + server.stop(true); + })(); + for (let i = 0; i < 20 && live() > before + 1; i++) { + Bun.gc(true); fullGC(); + await new Promise(r => setTimeout(r, 50)); + } + console.log(JSON.stringify({ before, after: live() })); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const { before, after } = JSON.parse(stdout.trim()); + expect(after).toBe(before + 1); + expect(exitCode).toBe(0); + }); + + // JSServerWebSocket holds a traced reference to the JSServer wrapper, so the + // server (and its ws handlers) stay alive while any websocket is connected, + // and become collectable once the last one closes. + test("server stays alive while a websocket is connected, then collects after close", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* js */ ` + const { heapStats, fullGC } = require("bun:jsc"); + const liveServer = () => heapStats().objectTypeCounts.DebugHTTPServer ?? 0; + + async function gcUntilCountAtMost(max) { + for (let i = 0; i < 50; i++) { + Bun.gc(true); + fullGC(); + if (liveServer() <= max) return liveServer(); + await Bun.sleep(10); + } + return liveServer(); + } + + const before = liveServer(); + const opened = Promise.withResolvers(); + const clientOpen = Promise.withResolvers(); + const echoed = Promise.withResolvers(); + const closed = Promise.withResolvers(); + + // Scope server so the only post-stop root is the connected websocket. + // Hold a WeakRef so we can stop(true) after the ws closes without + // adding a GC root ourselves. Assign client directly to the outer var + // rather than returning it — returning keeps the async frame's scope + // (which contains server) alive via the resolved-value chain in JSC. + let client, serverWeak; + await (async () => { + const server = Bun.serve({ + port: 0, + development: true, + fetch(req, s) { if (s.upgrade(req)) return; return new Response("ok"); }, + websocket: { + open() { opened.resolve(); }, + // Closes over server — the cycle through wsHandlers. + message(ws, m) { ws.send(server.port + ":" + m); }, + }, + }); + serverWeak = new WeakRef(server); + client = new WebSocket(server.url.href.replace("http", "ws")); + client.onopen = () => clientOpen.resolve(); + client.onmessage = e => echoed.resolve(e.data); + client.onclose = () => closed.resolve(); + await opened.promise; // server-side ws created (roots wrapper) + await clientOpen.promise; // client ready to send (avoid InvalidStateError) + server.stop(); // graceful — listener gone, ws stays + })(); + + // server out of scope. Wrapper is rooted only via: + // ServerWebSocket(this_value strong) → JSServerWebSocket → m_server → JSServer + // GC must NOT collect while the ws is open. + Bun.gc(true); fullGC(); + const whileConnected = liveServer(); + + // Dispatch through the cycle-captured handler (proves it's alive). + client.send("hi"); + const echo = await echoed.promise; + + client.close(); + await closed.promise; + client = null; + // Graceful stop with an open ws does not auto-finalize when the ws + // closes; force-finish via the WeakRef so the native side drops its + // event-loop ref. The WeakRef adds no root — if tracing failed above, + // deref() would already be undefined. + serverWeak.deref()?.stop(true); + const afterClose = await gcUntilCountAtMost(before + 1); + + console.log(JSON.stringify({ before, whileConnected, echo, afterClose })); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + const { before, whileConnected, echo, afterClose } = JSON.parse(stdout.trim()); + // objectTypeCounts includes the prototype (+1); the instance must also be + // present to prove the ws traced root kept it alive across GC. + expect(whileConnected).toBeGreaterThan(before + 1); + expect(echo).toMatch(/^\d+:hi$/); // handler dispatched (server.port captured) + expect(afterClose).toBeLessThanOrEqual(before + 1); // instance collected, prototype may remain + expect(exitCode).toBe(0); + }); + + // Reload swaps handlers via WriteBarrier .set() — old handlers become + // unreachable once nothing else holds them. + test("reload() releases the old handlers for collection", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* js */ ` + const { heapStats, fullGC } = require("bun:jsc"); + // objectTypeCounts only tracks JSC cell types, not user-defined JS + // classes, so use AsyncFunction as the observable: the OLD handler is + // async (counted), the NEW handler is a plain function (not counted). + const liveAsync = () => heapStats().objectTypeCounts.AsyncFunction ?? 0; + + const baseline = liveAsync(); + const server = Bun.serve({ + port: 0, + fetch: async () => new Response("old"), + }); + const beforeReload = liveAsync(); + server.reload({ fetch: () => new Response("new") }); + for (let i = 0; i < 20 && liveAsync() > baseline; i++) { + Bun.gc(true); + fullGC(); + await new Promise(r => setTimeout(r, 50)); + } + console.log(JSON.stringify({ baseline, beforeReload, afterReload: liveAsync() })); + server.stop(true); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + const { baseline, beforeReload, afterReload } = JSON.parse(stdout.trim()); + expect(beforeReload).toBeGreaterThan(baseline); // sanity: the async handler was counted + expect(afterReload).toBeLessThan(beforeReload); // old handler released after reload + expect(exitCode).toBe(0); + }); + + // Stress test under aggressive GC — catches missing write barriers. + test("serve+ws+reload survives BUN_JSC_collectContinuously=1", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* js */ ` + const server = Bun.serve({ + port: 0, + fetch: (req, s) => s.upgrade(req) ? undefined : new Response("ok"), + websocket: { open() {}, message(ws, m) { ws.send(m); } }, + }); + for (let i = 0; i < 10; i++) { + const ws = new WebSocket(server.url.href.replace("http", "ws")); + await new Promise(r => { ws.onopen = r; }); + ws.send("hi"); + await new Promise(r => { ws.onmessage = r; }); + ws.close(); + await new Promise(r => { ws.onclose = r; }); + server.reload({ + fetch: (req, s) => s.upgrade(req) ? undefined : new Response("ok " + i), + websocket: { open() {}, message(ws, m) { ws.send(m + i); } }, + }); + } + server.stop(true); + console.log("survived"); + `, + ], + env: { ...bunEnv, BUN_JSC_collectContinuously: "1", BUN_JSC_useConcurrentGC: "0" }, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(normalizeBunSnapshot(stdout)).toBe("survived"); + expect(exitCode).toBe(0); + }, 30_000); + + // with_async_context_if_needed wraps each handler in a fresh AsyncContextFrame + // that is NOT a property of the user's options arg. Stored as a raw JSValue in + // heap-boxed ServerConfig, it must stay rooted across init→listen→ptr_to_js→ + // slot-set (which includes vm.perform_gc()). + test("handlers wrapped via AsyncLocalStorage survive Bun.serve init under collectContinuously", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* js */ ` + const { AsyncLocalStorage } = require("node:async_hooks"); + const als = new AsyncLocalStorage(); + // Serve inside als.run so with_async_context_if_needed wraps every handler. + const server = await als.run({ ctx: 1 }, async () => { + return Bun.serve({ + port: 0, development: true, + fetch: () => new Response(String(als.getStore()?.ctx)), + error: () => new Response("err"), + websocket: { + open() {}, message(ws, m) { ws.send(m); }, close() {}, + }, + }); + }); + const r = await fetch(server.url, { keepalive: false }); + const body = await r.text(); + server.stop(true); + // The handler's ALS context wrapper survived init→ptr_to_js (would crash + // under collectContinuously if the AsyncContextFrame were collected). + console.log(JSON.stringify({ body, ok: body === "1" })); + `, + ], + env: { ...bunEnv, BUN_JSC_collectContinuously: "1", BUN_JSC_useConcurrentGC: "0" }, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + const { body, ok } = JSON.parse(stdout.trim()); + expect({ body, ok }).toEqual({ body: "1", ok: true }); + expect(exitCode).toBe(0); + }, 30_000); +}); From 40c06c177ea2313af6a2e08214403ff7ce6c6ae6 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 13:08:28 -0700 Subject: [PATCH 19/45] serve: DevServer on_request check uses !is_empty() not is_some() --- src/runtime/bake/DevServer.rs | 4 ++-- src/runtime/server/mod.rs | 38 +++++++++++++++++-------------- src/runtime/server/server_body.rs | 12 ++++++---- 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/runtime/bake/DevServer.rs b/src/runtime/bake/DevServer.rs index f697101b5d75..472463a6d703 100644 --- a/src/runtime/bake/DevServer.rs +++ b/src/runtime/bake/DevServer.rs @@ -5239,13 +5239,13 @@ fn on_request(dev: &mut DevServer, req: &mut Request, mut resp: AnyResponse) { return; } - if dev + if !dev .server .as_ref() .expect("infallible: server bound") .config() .on_request - .is_some() + .is_empty() { dev.server .as_mut() diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index cf622afc13df..92661d4ad831 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -275,11 +275,13 @@ pub struct NewServer { /// allocated when `!SSL`. Kept as a raw nullable pointer rather than a /// conditional field so the struct stays uniform across monomorphizations. pub h3_request_pool: *mut request_context::RequestContextStackAllocator, - /// Raw shadow of the wrapper's `m_allClosedPromise` WriteBarrier slot. - /// `JSValue::ZERO` until `server.stop()` lazily creates the promise. The - /// shadow lets `deinit_if_we_can` resolve it after the wrapper is gone - /// (the user's `await` keeps the promise itself alive in that window). - pub all_closed_promise: JSValue, + /// Authoritative GC root for the `server.stop()` promise. Lazily filled by + /// `get_all_closed_promise`; read in `deinit_if_we_can` (which can run + /// after the wrapper is collected, so the wrapper's `m_allClosedPromise` + /// slot alone is not sufficient — this Strong is what keeps the cell live + /// across that window). The slot is still written as a traced edge so the + /// cycle through user `.then(cb)` is collectable. + pub all_closed_promise: jsc::JSPromiseStrong, pub listen_callback: jsc::AnyTask::AnyTask, // allocator field dropped — global mimalloc per §Allocators @@ -316,7 +318,7 @@ pub struct UserRoute { impl Drop for NewServer { fn drop(&mut self) { // The remaining owned fields (config, base_url, h3_alt_svc, dev_server, - // user_routes) drop automatically. + // user_routes, all_closed_promise) drop automatically. if let Some(p) = self.plugins.take() { // SAFETY: `plugins` carries the `heap::alloc` provenance from // `ServePlugins::init`; this releases the server's counted ref. @@ -1664,10 +1666,10 @@ impl NewServer { }, self.flags .contains(ServerFlags::HAS_HANDLED_ALL_CLOSED_PROMISE), - if self.all_closed_promise.is_empty() { - "no" - } else { + if self.all_closed_promise.has_value() { "has" + } else { + "no" }, matches!(self.js_value, jsc::JsRef::Finalized), ); @@ -1678,7 +1680,7 @@ impl NewServer { && !self .flags .contains(ServerFlags::HAS_HANDLED_ALL_CLOSED_PROMISE) - && !self.all_closed_promise.is_empty() + && self.all_closed_promise.has_value() // `ServerAllConnectionsClosedTask::run_from_js_thread` early-returns // (without resolving the promise) when the VM is shutting down — // see the `if !vm.is_shutting_down()` gate there. Skip the @@ -1698,9 +1700,11 @@ impl NewServer { ServerAllConnectionsClosedTask::schedule( ServerAllConnectionsClosedTask { global_object: self.global_this, - // Task-local Strong: short-lived (one event-loop hop), and - // the only root once the wrapper's slot is gone. - promise: jsc::JSPromiseStrong::from_value(self.all_closed_promise, global), + // Duplicate the Strong handle so that we can hold two independent strong references to it. + promise: jsc::JSPromiseStrong::from_value( + self.all_closed_promise.value(), + global, + ), tracker: jsc::AsyncTaskTracker::init(vm_ref), }, vm_ref, @@ -1898,9 +1902,9 @@ impl NewServer { } } - // owned-field cleanup (user_routes / config / h3_alt_svc / dev_server / - // plugins) is handled by the heap::take drop below — see `impl Drop for - // NewServer`. + // owned-field cleanup (all_closed_promise / user_routes / config / + // h3_alt_svc / dev_server / plugins) is handled by the heap::take drop + // below — see `impl Drop for NewServer`. if Self::HAS_H3 { if let Some(h3a) = this_ref.h3_app.take() { // SAFETY: live H3::App handle owned by this server. @@ -1957,7 +1961,7 @@ impl NewServer { // the H3-listen path (`listen()` below) so HTTPS servers that // don't enable `config.http3` don't pay either. h3_request_pool: core::ptr::null_mut(), - all_closed_promise: JSValue::ZERO, + all_closed_promise: jsc::JSPromiseStrong::default(), listen_callback: jsc::AnyTask::AnyTask { ctx: None, callback: |_| Ok(()), diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index ae1c31122167..8c85dd129c75 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2702,13 +2702,15 @@ where if !self.has_listener() && self.pending_requests == 0 { return JSPromise::resolved_promise(global, JSValue::UNDEFINED).to_js(); } - if !self.all_closed_promise.is_empty() { - return self.all_closed_promise; + if self.all_closed_promise.has_value() { + return self.all_closed_promise.value(); } - let prom = JSPromise::create(global).to_js(); - self.all_closed_promise = prom; + self.all_closed_promise = jsc::JSPromiseStrong::init(global); + let prom = self.all_closed_promise.value(); // Only reachable via `server.stop()` (a JS-called method) — wrapper is - // alive. The slot is the GC root; the field above is the dispatch shadow. + // alive. Mirror into the slot as a traced edge so the cycle through + // user `.then(cb)` is collectable; the Strong field above is the + // authoritative root that survives wrapper collection. Self::js_gc_all_closed_promise_set(self.js_value_assert_alive(), global, prom); prom } From ab7020eb4a73547d4c4e01c74ab88b1254804c4f Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 13:45:23 -0700 Subject: [PATCH 20/45] serve: dedup Handler.server field after rebase onto ws-liveness branch Both branches added the same `server: Option` field at different struct positions; the rebase auto-merge kept both. Keep one, merge the doc comment to cover both uses (m_server slot tracing and live-socket accounting). --- src/runtime/server/WebSocketServerContext.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/runtime/server/WebSocketServerContext.rs b/src/runtime/server/WebSocketServerContext.rs index f3fc3744509b..68edea0d13f5 100644 --- a/src/runtime/server/WebSocketServerContext.rs +++ b/src/runtime/server/WebSocketServerContext.rs @@ -29,21 +29,20 @@ pub struct Handler { pub on_pong: JSValue, pub app: Option<*mut c_void>, - /// Backref to the owning server. Set in `set_routes()` alongside `app`. - /// Used by `ServerWebSocket::init` to write the server JS wrapper into the - /// per-socket `m_server` traced slot, so the wrapper (and the `m_ws*` - /// handler slots it carries) stays reachable while any socket is connected. + /// Type-erased backref to the owning `NewServer`, set alongside `app` + /// in `set_routes` (so it is in place before any socket can upgrade and + /// refreshed whenever a reload installs a new context). + /// `ServerWebSocket::init` reads it to write the server JS wrapper into the + /// per-socket `m_server` traced slot (keeping the wrapper, and the `m_ws*` + /// handler slots it carries, reachable while any socket is connected), and + /// `ServerWebSocket` open/close events route the live-socket accounting + /// through it. pub server: Option, // Always set manually. // LIFETIMES.tsv = STATIC (vm) / JSC_BORROW (global_object) — both outlive the handler. pub vm: bun_ptr::BackRef, pub global_object: bun_ptr::BackRef, - /// Type-erased backref to the owning `NewServer`, set alongside `app` - /// in `set_routes` (so it is in place before any socket can upgrade and - /// refreshed whenever a reload installs a new context). `ServerWebSocket` - /// open/close events route the live-socket accounting through it. - pub server: Option, /// used by publish() pub flags: HandlerFlags, @@ -113,7 +112,6 @@ impl Handler { server: None, vm: bun_ptr::BackRef::new(VirtualMachine::get()), global_object: bun_ptr::BackRef::new(global_object), - server: None, flags: HandlerFlags::empty(), }; From 7984ef9828e4fd678e689e287272fc2b6677fbee Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 11:54:48 -0700 Subject: [PATCH 21/45] bake: deref html_bundle in DevServer Drop get_or_put_route_bundle takes an intrusive ref when it creates a RouteBundle, but the raw html_bundle pointer has no Drop and DevServer Drop never released it. One Route per HTML route per dev-server leaked. Zig RouteBundle.deinit:112 does the deref; the Rust port missed it. --- src/runtime/bake/DevServer.rs | 8 ++++++-- src/runtime/bake/dev_server/route_bundle.rs | 11 ++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/runtime/bake/DevServer.rs b/src/runtime/bake/DevServer.rs index 472463a6d703..5f41fee29ba0 100644 --- a/src/runtime/bake/DevServer.rs +++ b/src/runtime/bake/DevServer.rs @@ -1247,6 +1247,10 @@ impl Drop for DevServer { // SAFETY: stored ref from `init_from_any_blob`; no live borrow. unsafe { StaticRoute::deref_(cached.as_ptr()) }; } + // SAFETY: paired with the `ref_` taken in + // `get_or_put_route_bundle` when this bundle was created; the + // raw `html_bundle` field has no Drop, so release it here. + unsafe { bun_ptr::RefCount::::deref(html.html_bundle) }; } } @@ -5376,8 +5380,8 @@ impl DevServer { let file = &mut self.client_graph.bundled_files.values_mut() [incremental_graph_index.get() as usize]; file.html_route_bundle_index = Some(bundle_index); - // Bump the intrusive refcount; matched by - // `RouteBundle::deinit`'s deref of `html_bundle`. + // Bump the intrusive refcount; matched by the + // `html_bundle` deref in `DevServer`'s `Drop`. // SAFETY: `html` is a live IntrusiveRc-managed allocation. unsafe { bun_ptr::RefCount::::ref_(html) }; break 'brk route_bundle::Data::Html(route_bundle::Html { diff --git a/src/runtime/bake/dev_server/route_bundle.rs b/src/runtime/bake/dev_server/route_bundle.rs index 3736176cc045..32a31e21ea2f 100644 --- a/src/runtime/bake/dev_server/route_bundle.rs +++ b/src/runtime/bake/dev_server/route_bundle.rs @@ -164,8 +164,9 @@ impl RouteBundle { } } -// `deinit` is fully subsumed by Drop: -// - client_bundle / cached_response: Option> drop = .deref() -// - Framework: StrongOptional fields drop = .deinit() -// - Html: bundled_html_text Box<[u8]> drop = allocator.free() -// html_bundle RefPtr drop = .deref() +// Zig `RouteBundle.deinit` equivalent, split across two mechanisms: +// - Drop: `Framework` StrongOptional fields (= .deinit()) and +// `Html.bundled_html_text` Box<[u8]> (= allocator.free()). +// - `DevServer`'s `Drop` (explicit): `client_bundle`, `Html.cached_response` +// (BackRef, no Drop) and `Html.html_bundle` (raw ptr, no Drop) each hold +// an intrusive ref that is deref'd there. From c1310d11e53fbaa839f81c03753d41766b501dac Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 12 Jun 2026 20:49:50 +0000 Subject: [PATCH 22/45] [autofix.ci] apply automated fixes --- src/runtime/api/BunObject.rs | 15 ++++++++++++--- src/runtime/server/ServerWebSocket.rs | 4 +--- src/runtime/server/server_body.rs | 21 ++++++++++++--------- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index de907c2651ee..67e27e868792 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1656,17 +1656,26 @@ pub(crate) fn serve(global_object: &JSGlobalObject, callframe: &CallFrame) -> Js // the unwrapped fn is held live by the user's options object on the // `serve()` stack across `init`/`listen` until this point. if !server_ref.config.on_request.is_empty() { - let wrapped = server_ref.config.on_request.with_async_context_if_needed(global_object); + let wrapped = server_ref + .config + .on_request + .with_async_context_if_needed(global_object); <$ServerType>::js_gc_on_request_set(obj, global_object, wrapped); server_ref.config.on_request = wrapped; } if !server_ref.config.on_error.is_empty() { - let wrapped = server_ref.config.on_error.with_async_context_if_needed(global_object); + let wrapped = server_ref + .config + .on_error + .with_async_context_if_needed(global_object); <$ServerType>::js_gc_on_error_set(obj, global_object, wrapped); server_ref.config.on_error = wrapped; } if !server_ref.config.on_node_http_request.is_empty() { - let wrapped = server_ref.config.on_node_http_request.with_async_context_if_needed(global_object); + let wrapped = server_ref + .config + .on_node_http_request + .with_async_context_if_needed(global_object); <$ServerType>::js_gc_on_node_http_request_set(obj, global_object, wrapped); server_ref.config.on_node_http_request = wrapped; } diff --git a/src/runtime/server/ServerWebSocket.rs b/src/runtime/server/ServerWebSocket.rs index 1c47f9e5cd15..311813dfff15 100644 --- a/src/runtime/server/ServerWebSocket.rs +++ b/src/runtime/server/ServerWebSocket.rs @@ -731,9 +731,7 @@ impl ServerWebSocket { }; let call_args = [cached_this, JSValue::js_number(code as f64), message_js]; - if let Err(e) = - on_close_handler.call(global_object, JSValue::UNDEFINED, &call_args) - { + if let Err(e) = on_close_handler.call(global_object, JSValue::UNDEFINED, &call_args) { let err = global_object.take_exception(e); bun_output::scoped_log!(WebSocketServer, "onClose error {}", was_not_empty); handler.run_error_callback(vm, global_object, err); diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 8c85dd129c75..a02214c11493 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2179,7 +2179,9 @@ where let wrapped = if new_config.on_node_http_request.is_empty() { JSValue::ZERO } else { - new_config.on_node_http_request.with_async_context_if_needed(global) + new_config + .on_node_http_request + .with_async_context_if_needed(global) }; if let Some(server_js) = server_js { Self::js_gc_on_node_http_request_set(server_js, global, wrapped); @@ -3361,14 +3363,15 @@ where let request_value = args[0]; request_value.ensure_still_alive(); - let response_value = match this.config.on_request.call( - &global, - this.js_value_assert_alive(), - &args, - ) { - Ok(v) => v, - Err(err) => global.take_exception(err), - }; + let response_value = + match this + .config + .on_request + .call(&global, this.js_value_assert_alive(), &args) + { + Ok(v) => v, + Err(err) => global.take_exception(err), + }; let request_object_ptr: *mut Request = request_object; scopeguard::defer! { // uWS request will not live longer than this function From 33b691a2ddd4bdb2539dee9f08d9eda929b3ce75 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 14:02:56 -0700 Subject: [PATCH 23/45] serve: clear ws slots when reload removes websocket; normalize undefined/null handler clears --- src/runtime/server/mod.rs | 14 ++++++++++++++ src/runtime/server/server_body.rs | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 92661d4ad831..2f52be410c63 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -3044,6 +3044,20 @@ impl NewServer { /// `on_reload_from_zig`; dispatch keeps reading the shadow. pub fn write_ws_handler_slots(&mut self, server_js: JSValue, global: &JSGlobalObject) { let Some(ws) = self.config.websocket.as_mut() else { + // No websocket config: clear all 7 slots so a config transition + // to "no websocket" drops the previous roots (G6). Current + // callers never reach this (initial-serve slots are already + // ZERO; `on_reload_from_zig` only calls us right after + // assigning `self.config.websocket = Some(_)`), but the doc'd + // contract is "writes all slots unconditionally" — honor it so + // future call sites can't leave stale roots behind. + Self::js_gc_ws_on_open_set(server_js, global, JSValue::ZERO); + Self::js_gc_ws_on_message_set(server_js, global, JSValue::ZERO); + Self::js_gc_ws_on_close_set(server_js, global, JSValue::ZERO); + Self::js_gc_ws_on_drain_set(server_js, global, JSValue::ZERO); + Self::js_gc_ws_on_error_set(server_js, global, JSValue::ZERO); + Self::js_gc_ws_on_ping_set(server_js, global, JSValue::ZERO); + Self::js_gc_ws_on_pong_set(server_js, global, JSValue::ZERO); return; }; let h = &mut ws.handler; diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index a02214c11493..52f4f886519e 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2176,7 +2176,7 @@ where // config omits the handler, so subsequent `on_web_socket_upgrade` / // `set_routes` stop routing through the node:http path. if self.config.on_node_http_request != new_config.on_node_http_request { - let wrapped = if new_config.on_node_http_request.is_empty() { + let wrapped = if new_config.on_node_http_request.is_empty_or_undefined_or_null() { JSValue::ZERO } else { new_config From 27b7600436bdd694bd9488442805b324fb6530ff Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 14:35:28 -0700 Subject: [PATCH 24/45] serve: extract wrap_handler_slot + respond_stopped_503 helpers; fix gaps they exposed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wrap→slot→shadow pattern was hand-copied across serve/reload/ws with diverging is_empty checks; the 503 guard was open-coded 6× and missed the node:http trampoline. One helper per pattern; on_clienterror now wraps like the others; reload re-wraps unconditionally so ALS context follows the call-time frame; reload adopts any websocket config (not just open|message). Dropped the dead allClosedPromise slot. --- src/runtime/api/BunObject.rs | 47 +++++----- src/runtime/server/ServerWebSocket.rs | 10 +-- src/runtime/server/mod.rs | 122 ++++++++++++++++---------- src/runtime/server/server.classes.ts | 1 - src/runtime/server/server_body.rs | 112 +++++++++++------------ 5 files changed, 160 insertions(+), 132 deletions(-) diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 67e27e868792..67989d04494d 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1655,31 +1655,30 @@ pub(crate) fn serve(global_object: &JSGlobalObject, callframe: &CallFrame) -> Js // freshly-allocated wrapper fn is rooted by the slot immediately; // the unwrapped fn is held live by the user's options object on the // `serve()` stack across `init`/`listen` until this point. - if !server_ref.config.on_request.is_empty() { - let wrapped = server_ref - .config - .on_request - .with_async_context_if_needed(global_object); - <$ServerType>::js_gc_on_request_set(obj, global_object, wrapped); - server_ref.config.on_request = wrapped; - } - if !server_ref.config.on_error.is_empty() { - let wrapped = server_ref - .config - .on_error - .with_async_context_if_needed(global_object); - <$ServerType>::js_gc_on_error_set(obj, global_object, wrapped); - server_ref.config.on_error = wrapped; - } - if !server_ref.config.on_node_http_request.is_empty() { - let wrapped = server_ref - .config - .on_node_http_request - .with_async_context_if_needed(global_object); - <$ServerType>::js_gc_on_node_http_request_set(obj, global_object, wrapped); - server_ref.config.on_node_http_request = wrapped; + crate::server::wrap_handler_slot( + &mut server_ref.config.on_request, + Some(obj), + global_object, + <$ServerType>::js_gc_on_request_set, + ); + crate::server::wrap_handler_slot( + &mut server_ref.config.on_error, + Some(obj), + global_object, + <$ServerType>::js_gc_on_error_set, + ); + crate::server::wrap_handler_slot( + &mut server_ref.config.on_node_http_request, + Some(obj), + global_object, + <$ServerType>::js_gc_on_node_http_request_set, + ); + // Skip the 7-slot write when there's no websocket config: the + // slots default ZERO so `write_ws_handler_slots`'s clear path + // would be 7 wasted FFI calls. + if server_ref.config.websocket.is_some() { + server_ref.write_ws_handler_slots(obj, global_object); } - server_ref.write_ws_handler_slots(obj, global_object); server_ref.js_value.set_strong(obj, global_object); if global_object.bun_vm().test_isolation_enabled { diff --git a/src/runtime/server/ServerWebSocket.rs b/src/runtime/server/ServerWebSocket.rs index 311813dfff15..6c3e11da4ac7 100644 --- a/src/runtime/server/ServerWebSocket.rs +++ b/src/runtime/server/ServerWebSocket.rs @@ -361,11 +361,11 @@ impl ServerWebSocket { .this_value .set(JsRef::init_strong(this_value, global_object)); js::data_set_cached(this_value, global_object, data_value); - if let Some(server) = handler.server { - let server_js = server.js_value(); - if !server_js.is_undefined() { - js::server_set_cached(this_value, global_object, server_js); - } + // Only mirror the server wrapper while it is strongly rooted — + // `js_value()` would return a weak (potentially dead-but-unswept) + // address once the server has gone idle and downgraded. + if let Some(server_js) = handler.server.and_then(|s| s.js_value_if_strong()) { + js::server_set_cached(this_value, global_object, server_js); } this } diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 2f52be410c63..d1c8a8300467 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -277,10 +277,10 @@ pub struct NewServer { pub h3_request_pool: *mut request_context::RequestContextStackAllocator, /// Authoritative GC root for the `server.stop()` promise. Lazily filled by /// `get_all_closed_promise`; read in `deinit_if_we_can` (which can run - /// after the wrapper is collected, so the wrapper's `m_allClosedPromise` - /// slot alone is not sufficient — this Strong is what keeps the cell live - /// across that window). The slot is still written as a traced edge so the - /// cycle through user `.then(cb)` is collectable. + /// after the wrapper is collected, so a wrapper-traced slot would not + /// suffice — this Strong is what keeps the cell live across that window). + /// The cycle through a user `.then(cb)` is broken by resolving the + /// promise in `deinit_if_we_can`, after which the Strong is dropped. pub all_closed_promise: jsc::JSPromiseStrong, pub listen_callback: jsc::AnyTask::AnyTask, @@ -588,6 +588,35 @@ impl NewServer { _ => None, } } +} + +/// Single point of truth for "wrap a handler callback and mirror it into the +/// wrapper's WriteBarrier slot". If `*shadow` is unset (empty/undefined/null), +/// normalize it to `ZERO` and clear the slot; otherwise apply the +/// async-context wrap and write the wrapped fn into both the slot and +/// `*shadow`. `server_js` may be `None` (wrapper already collected during a +/// late reload), in which case only the shadow is updated. Keeping the +/// is-empty check, the wrap step, and the shadow↔slot pairing in one helper +/// is what stops the serve / reload / ws / clientError sites from drifting. +#[inline] +pub(crate) fn wrap_handler_slot( + shadow: &mut JSValue, + server_js: Option, + global: &JSGlobalObject, + set: fn(JSValue, &JSGlobalObject, JSValue), +) { + let v = if shadow.is_empty_or_undefined_or_null() { + JSValue::ZERO + } else { + shadow.with_async_context_if_needed(global) + }; + if let Some(server_js) = server_js { + set(server_js, global, v); + } + *shadow = v; +} + +impl NewServer { /// Per-monomorphization static. /// Rust statics cannot be const-generic; routed through a @@ -1055,9 +1084,7 @@ impl NewServer { // rather than panicking in js_value_assert_alive() below. // SAFETY: `this` is the live server backref for this request. if unsafe { &*this }.js_value_for_dispatch().is_none() { - let resp_ref = bun_opaque::opaque_deref_mut(resp); - resp_ref.write_status(b"503 Service Unavailable"); - resp_ref.end_without_body(true); + server_body::respond_stopped_503(bun_opaque::opaque_deref_mut(resp)); return; } let should_deinit_context = core::cell::Cell::new(false); @@ -1108,9 +1135,7 @@ impl NewServer { // SAFETY: `server` is the live backref stored in `user_route`. if unsafe { &*server }.js_value_for_dispatch().is_none() { - let resp_ref = bun_opaque::opaque_deref_mut(resp); - resp_ref.write_status(b"503 Service Unavailable"); - resp_ref.end_without_body(true); + server_body::respond_stopped_503(bun_opaque::opaque_deref_mut(resp)); return; } @@ -1199,6 +1224,11 @@ impl NewServer { // SAFETY: `this` is the live server backref registered as the uws // userdata; only one borrow derived from it is alive at a time. + if unsafe { &*this }.js_value_for_dispatch().is_none() { + server_body::respond_stopped_503(resp); + return; + } + // SAFETY: same `this` as above. unsafe { (*this).on_pending_request() }; // Read-only access goes through `BackRef` (safe `Deref`); each use // materialises a fresh short-lived `&Self`, so the JS-reentrant calls @@ -1504,7 +1534,13 @@ impl NewServer { /// Returns true when this close drained the last live websocket. pub(crate) fn note_websocket_closed(&self) -> bool { - let remaining = self.active_websocket_count.get().saturating_sub(1); + let prev = self.active_websocket_count.get(); + // Guard underflow: a `close` reaching us without a matching `open` + // (double-close on the JS side) must not wrap and return `true`. + if prev == 0 { + return false; + } + let remaining = prev - 1; self.active_websocket_count.set(remaining); remaining == 0 } @@ -2918,22 +2954,22 @@ use server_body::{Bun__ServerRouteList__callRoute, Bun__ServerRouteList__create} mod route_list_cached { pub(super) mod http { bun_jsc::codegen_cached_accessors!( - "HTTPServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong, allClosedPromise + "HTTPServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong ); } pub(super) mod https { bun_jsc::codegen_cached_accessors!( - "HTTPSServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong, allClosedPromise + "HTTPSServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong ); } pub(super) mod debug_http { bun_jsc::codegen_cached_accessors!( - "DebugHTTPServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong, allClosedPromise + "DebugHTTPServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong ); } pub(super) mod debug_https { bun_jsc::codegen_cached_accessors!( - "DebugHTTPSServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong, allClosedPromise + "DebugHTTPSServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong ); } } @@ -3028,29 +3064,22 @@ impl NewServer { ws_on_pong_get_cached, ws_on_pong_set_cached ); - cached_value_dispatch!( - js_gc_all_closed_promise_get, - js_gc_all_closed_promise_set, - all_closed_promise_get_cached, - all_closed_promise_set_cached - ); /// Mirror all 7 `Handler.on_*` shadows into the wrapper's `m_wsOn*` /// WriteBarrier slots, applying the async-context wrap (deferred from /// `Handler::from_js` so the wrapped fn is rooted the moment it exists). /// Writes all slots unconditionally — `JSValue::ZERO` clears, so a reload - /// that omits a callback drops the previous root (G6). Called after + /// that omits a callback drops the previous root. Called after /// `ptr_to_js` in `serve()` and after the websocket-context swap in /// `on_reload_from_zig`; dispatch keeps reading the shadow. pub fn write_ws_handler_slots(&mut self, server_js: JSValue, global: &JSGlobalObject) { let Some(ws) = self.config.websocket.as_mut() else { // No websocket config: clear all 7 slots so a config transition - // to "no websocket" drops the previous roots (G6). Current - // callers never reach this (initial-serve slots are already - // ZERO; `on_reload_from_zig` only calls us right after - // assigning `self.config.websocket = Some(_)`), but the doc'd - // contract is "writes all slots unconditionally" — honor it so - // future call sites can't leave stale roots behind. + // to "no websocket" drops the previous roots. Redundant on + // initial serve (slots default ZERO) — `serve()` gates this call + // on `websocket.is_some()` — but the doc'd contract is "writes + // all slots unconditionally" so a future call site can't leave + // stale roots behind. Self::js_gc_ws_on_open_set(server_js, global, JSValue::ZERO); Self::js_gc_ws_on_message_set(server_js, global, JSValue::ZERO); Self::js_gc_ws_on_close_set(server_js, global, JSValue::ZERO); @@ -3061,25 +3090,14 @@ impl NewServer { return; }; let h = &mut ws.handler; - macro_rules! wrap_slot { - ($field:ident, $set:ident) => {{ - let raw = h.$field; - if raw.is_empty() { - Self::$set(server_js, global, JSValue::ZERO); - } else { - let wrapped = raw.with_async_context_if_needed(global); - Self::$set(server_js, global, wrapped); - h.$field = wrapped; - } - }}; - } - wrap_slot!(on_open, js_gc_ws_on_open_set); - wrap_slot!(on_message, js_gc_ws_on_message_set); - wrap_slot!(on_close, js_gc_ws_on_close_set); - wrap_slot!(on_drain, js_gc_ws_on_drain_set); - wrap_slot!(on_error, js_gc_ws_on_error_set); - wrap_slot!(on_ping, js_gc_ws_on_ping_set); - wrap_slot!(on_pong, js_gc_ws_on_pong_set); + let sj = Some(server_js); + wrap_handler_slot(&mut h.on_open, sj, global, Self::js_gc_ws_on_open_set); + wrap_handler_slot(&mut h.on_message, sj, global, Self::js_gc_ws_on_message_set); + wrap_handler_slot(&mut h.on_close, sj, global, Self::js_gc_ws_on_close_set); + wrap_handler_slot(&mut h.on_drain, sj, global, Self::js_gc_ws_on_drain_set); + wrap_handler_slot(&mut h.on_error, sj, global, Self::js_gc_ws_on_error_set); + wrap_handler_slot(&mut h.on_ping, sj, global, Self::js_gc_ws_on_ping_set); + wrap_handler_slot(&mut h.on_pong, sj, global, Self::js_gc_ws_on_pong_set); } } @@ -3653,6 +3671,16 @@ impl AnyServer { any_server_dispatch!(self, |s| s.js_value.try_get().unwrap_or(JSValue::UNDEFINED)) } + /// Like [`Self::js_value`] but `None` once the server has gone idle and + /// the `JsRef` downgraded to weak — same gate as + /// [`NewServer::js_value_for_dispatch`], closing the dead-but-unswept + /// window where a `Weak` may hold a stale address. Use this when writing + /// the wrapper into another GC-traced slot. + #[inline] + pub fn js_value_if_strong(&self) -> Option { + any_server_dispatch!(self, |s| s.js_value_for_dispatch()) + } + pub fn h3_alt_svc(&self) -> Option<&[u8]> { match self.tag { AnyServerTag::HTTPSServer => self.as_https().h3_alt_svc(), diff --git a/src/runtime/server/server.classes.ts b/src/runtime/server/server.classes.ts index c319c87f279e..8528cf76b031 100644 --- a/src/runtime/server/server.classes.ts +++ b/src/runtime/server/server.classes.ts @@ -103,7 +103,6 @@ function generate(name) { "wsOnError", "wsOnPing", "wsOnPong", - "allClosedPromise", ], }); } diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 52f4f886519e..5ab89e6faa66 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -401,6 +401,17 @@ impl RespLike for uws_sys::h3::Response { } } +/// Answer a request that arrived after the server's JS wrapper was downgraded +/// (idle keep-alive sockets aren't counted in `pending_requests`, so the +/// wrapper can be gone before the next request fires). 503-and-close instead +/// of dispatching into a dead handler shadow. One helper so every dispatch +/// trampoline gets the same guard. +#[inline] +pub(super) fn respond_stopped_503(resp: &mut R) { + resp.write_status(b"503 Service Unavailable"); + resp.end_without_body(true); +} + pub(super) type ServerRequestContext = NewRequestContext, SSL, DEBUG, false>; pub(super) type ServerH3RequestContext = @@ -2159,56 +2170,52 @@ where } } - // Only reload `on_request` / `on_error` when the new config actually - // specifies one. The shadow `JSValue` is updated alongside the - // wrapper's WriteBarrier slot (the GC root); both must agree. The - // async-context wrap is applied here (deferred from `from_js`) so the - // wrapped fn is rooted by the slot the moment it exists. + // `on_request` / `on_error` keep their previous value when the reload + // config omits them; `on_node_http_request` swaps unconditionally + // (clearing to ZERO when omitted) so `on_web_socket_upgrade` / + // `set_routes` stop routing through the node:http path. The + // async-context re-wrap is unconditional — `with_async_context_if_needed` + // is a no-op when no ALS frame is active, so re-wrapping on every + // reload keeps the captured frame in sync with the call-time context. let server_js = self.js_value.try_get().filter(|v| !v.is_empty()); if !new_config.on_request.is_empty_or_undefined_or_null() { - let wrapped = new_config.on_request.with_async_context_if_needed(global); - if let Some(server_js) = server_js { - Self::js_gc_on_request_set(server_js, global, wrapped); - } - self.config.on_request = wrapped; - } - // Swap on any change, *including* clearing to `.zero` when the reload - // config omits the handler, so subsequent `on_web_socket_upgrade` / - // `set_routes` stop routing through the node:http path. - if self.config.on_node_http_request != new_config.on_node_http_request { - let wrapped = if new_config.on_node_http_request.is_empty_or_undefined_or_null() { - JSValue::ZERO - } else { - new_config - .on_node_http_request - .with_async_context_if_needed(global) - }; - if let Some(server_js) = server_js { - Self::js_gc_on_node_http_request_set(server_js, global, wrapped); - } - self.config.on_node_http_request = wrapped; + super::wrap_handler_slot( + &mut new_config.on_request, + server_js, + global, + Self::js_gc_on_request_set, + ); + self.config.on_request = new_config.on_request; } + super::wrap_handler_slot( + &mut new_config.on_node_http_request, + server_js, + global, + Self::js_gc_on_node_http_request_set, + ); + self.config.on_node_http_request = new_config.on_node_http_request; if !new_config.on_error.is_empty_or_undefined_or_null() { - let wrapped = new_config.on_error.with_async_context_if_needed(global); - if let Some(server_js) = server_js { - Self::js_gc_on_error_set(server_js, global, wrapped); - } - self.config.on_error = wrapped; + super::wrap_handler_slot( + &mut new_config.on_error, + server_js, + global, + Self::js_gc_on_error_set, + ); + self.config.on_error = new_config.on_error; } if let Some(mut ws) = new_config.websocket.take() { + // `Handler::from_js` already rejected configs with no non-error + // callback, so any `Some(ws)` is adoptable — match initial-serve + // and adopt unconditionally. ws.handler .flags .set(super::web_socket_server_context::HandlerFlags::SSL, SSL); - if !ws.handler.on_message.is_empty() || !ws.handler.on_open.is_empty() { - ws.global_object = bun_ptr::BackRef::new(global); - self.config.websocket = Some(ws); - if let Some(server_js) = server_js { - self.write_ws_handler_slots(server_js, global); - } + ws.global_object = bun_ptr::BackRef::new(global); + self.config.websocket = Some(ws); + if let Some(server_js) = server_js { + self.write_ws_handler_slots(server_js, global); } - // Not adopting it: nothing to release — handler callbacks are - // unrooted shadows until written into the wrapper's `m_wsOn*` slots. } // These get re-applied when we set the static routes again. @@ -2708,13 +2715,7 @@ where return self.all_closed_promise.value(); } self.all_closed_promise = jsc::JSPromiseStrong::init(global); - let prom = self.all_closed_promise.value(); - // Only reachable via `server.stop()` (a JS-called method) — wrapper is - // alive. Mirror into the slot as a traced edge so the cycle through - // user `.then(cb)` is collectable; the Strong field above is the - // authoritative root that survives wrapper collection. - Self::js_gc_all_closed_promise_set(self.js_value_assert_alive(), global, prom); - prom + self.all_closed_promise.value() } // `notify_inspector_server_stopped` lives in the unbounded impl block @@ -2821,8 +2822,7 @@ where let index = user_route.id; if server.js_value_for_dispatch().is_none() { - RespLike::write_status(resp, b"503 Service Unavailable"); - RespLike::end_without_body(resp, true); + respond_stopped_503(resp); return; } @@ -2911,8 +2911,7 @@ where resp: &mut Ctx::Resp, ) { if self.js_value_for_dispatch().is_none() { - RespLike::write_status(resp, b"503 Service Unavailable"); - RespLike::end_without_body(resp, true); + respond_stopped_503(resp); return; } let self_ptr: *mut Self = self; @@ -3210,8 +3209,7 @@ where let index = this.id; if server_ref.js_value_for_dispatch().is_none() { - resp.write_status(b"503 Service Unavailable"); - resp.end_without_body(true); + respond_stopped_503(resp); return; } @@ -3292,8 +3290,7 @@ where // duration. let this = unsafe { &mut *self_ptr }; if this.js_value_for_dispatch().is_none() { - resp.write_status(b"503 Service Unavailable"); - resp.end_without_body(true); + respond_stopped_503(resp); return; } if !this.config.on_node_http_request.is_empty() { @@ -3666,7 +3663,12 @@ pub(super) fn server_set_on_client_error_( let this = unsafe { &mut *this }; if let Some(app) = this.app { this.on_clienterror = callback; - <$T>::js_gc_on_client_error_set(server, global, callback); + super::wrap_handler_slot( + &mut this.on_clienterror, + Some(server), + global, + <$T>::js_gc_on_client_error_set, + ); // uws_sys::App::on_client_error takes the raw C-ABI handler shape; // wrap our typed callback in an extern "C" thunk that slices raw_packet. extern "C" fn thunk( From 2de5aff4c914c9a8b513d8ccbd74e2ee185c49a5 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 14:44:46 -0700 Subject: [PATCH 25/45] serve: respond_stopped_503 honors per-protocol close semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit H3 reject paths pass close_connection=false so sibling streams on the same QUIC connection survive (matching the existing !Ctx::IS_H3 pattern at the other reject sites). Added RespLike::IS_H3 so the helper can resolve this without a per-caller arg. The on_error reload-null finding is a no-op: ServerConfig::from_js uses get_truthy("error"), so null/undefined/omitted all parse to JSValue::ZERO on this branch (and to None on main) — null could never clear the handler on main either. Semantics are unchanged. The get_all_closed_promise comment nit was already addressed in 27b7600436 (the inaccurate comment was removed when the wrapper-slot mirror was dropped). --- src/runtime/server/server_body.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 5ab89e6faa66..ca00b864a7ac 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -329,6 +329,7 @@ impl ReqLike for uws_sys::h3::Request { } pub trait RespLike { + const IS_H3: bool; fn write_status(&mut self, status: &[u8]); fn end_without_body(&mut self, close_connection: bool); fn timeout(&mut self, seconds: u8); @@ -336,6 +337,7 @@ pub trait RespLike { fn to_any_response(&mut self) -> uws::AnyResponse; } impl RespLike for uws_sys::NewAppResponse { + const IS_H3: bool = false; #[inline] fn write_status(&mut self, s: &[u8]) { uws_sys::NewAppResponse::::write_status(self, s) @@ -375,6 +377,7 @@ impl RespLike for uws_sys::NewAppResponse { } } impl RespLike for uws_sys::h3::Response { + const IS_H3: bool = true; #[inline] fn write_status(&mut self, s: &[u8]) { uws_sys::h3::Response::write_status(self, s) @@ -403,13 +406,15 @@ impl RespLike for uws_sys::h3::Response { /// Answer a request that arrived after the server's JS wrapper was downgraded /// (idle keep-alive sockets aren't counted in `pending_requests`, so the -/// wrapper can be gone before the next request fires). 503-and-close instead -/// of dispatching into a dead handler shadow. One helper so every dispatch -/// trampoline gets the same guard. +/// wrapper can be gone before the next request fires). 503 instead of +/// dispatching into a dead handler shadow. One helper so every dispatch +/// trampoline gets the same guard. H1 closes the connection; H3 ends only this +/// stream (`!R::IS_H3`) so sibling streams on the same QUIC connection survive +/// — same per-protocol close treatment as the other reject fast paths. #[inline] pub(super) fn respond_stopped_503(resp: &mut R) { resp.write_status(b"503 Service Unavailable"); - resp.end_without_body(true); + resp.end_without_body(!R::IS_H3); } pub(super) type ServerRequestContext = From 425891dd6ebeccb02e4c91240a5f8309b7b57bff Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 14:48:17 -0700 Subject: [PATCH 26/45] test: cover node:http late-request, reload-drops-ws-handler; fix self-defeating keepalive + stress hangs --- test/js/bun/http/bun-server.test.ts | 245 +++++++++++++++++++++++++--- 1 file changed, 223 insertions(+), 22 deletions(-) diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index faf7930bcf17..eecfe42e4775 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -622,13 +622,20 @@ test("should be able to await server.stop(true) with keep alive", async () => { expect(async () => await fetch(server.url)).toThrow(); }); -test("late keep-alive request to a route after stop()+GC does not crash", async () => { +test("late keep-alive request to a route after stop()+GC answers 503", async () => { // Per-route handlers live in ServerRouteList, which is reachable from JS only // through the Server wrapper. After a graceful stop() the user may drop the // wrapper, and a subsequent GC may finalize it before an idle keep-alive // connection sends one more request. Run in a subprocess so the previous // js_value_assert_alive() panic on that path surfaces as a non-zero exit // instead of taking down the test runner. + // + // Graceful stop() calls close_idle_connections(), which would normally close + // our keep-alive socket before the late request can reach the 503 guard. To + // exercise that guard we hold the FIRST request in-flight (so the socket is + // non-idle and survives close_idle) and pipeline the LATE request behind it. + // When the first completes, pending_requests → 0 → js_value downgrades, and + // uws then dispatches the pipelined request into a None wrapper → 503. await using proc = Bun.spawn({ cmd: [ bunExe(), @@ -663,17 +670,28 @@ test("late keep-alive request to a route after stop()+GC does not crash", async })(); }; - const port = await (async () => { + const release = Promise.withResolvers(); + const inflight = Promise.withResolvers(); + let hits = 0; + + await (async () => { const server = Bun.serve({ port: 0, hostname: "127.0.0.1", - routes: { "/r": () => new Response("ok") }, + routes: { + "/r": async () => { + if (++hits === 1) { + inflight.resolve(); + await release.promise; // hold socket non-idle through stop() + } + return new Response("ok"); + }, + }, }); - const port = server.port; globalThis.sock = await Bun.connect({ hostname: "127.0.0.1", - port, + port: server.port, socket: { data(socket, data) { received += data.toString("latin1"); @@ -684,27 +702,137 @@ test("late keep-alive request to a route after stop()+GC does not crash", async }, }); + // First request: handler parks on \`release\`, keeping the socket + // in-flight so close_idle_connections() skips it. sock.write("GET /r HTTP/1.1\\r\\nHost: x\\r\\nConnection: keep-alive\\r\\n\\r\\n"); - const first = await nextResponse(); - if (!first.includes("200")) throw new Error("first request failed: " + first); + await inflight.promise; + + // Pipeline the late request behind the held one. uws won't read it + // until the first response is sent, by which time js_value is Weak. + sock.write("GET /r HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\n\\r\\n"); - // Graceful stop: existing connections stay open, listener closes. + // Graceful stop: listener closes, close_idle skips this socket. server.stop(); - return port; })(); // The only \`server\` binding is now out of scope. + // Let the first request complete: pending_requests → 0, + // deinit_if_we_can() downgrades js_value to Weak. uws then reads the + // pipelined request and on_user_route_request sees + // js_value_for_dispatch() == None → 503. Previously: panic. + release.resolve(); + const first = await nextResponse(); + if (!first.includes("200")) throw new Error("first request failed: " + first); + const second = await nextResponse(); + + // Wrapper is now Weak and unreferenced; GC must collect it cleanly. for (let i = 0; i < 10; i++) { Bun.gc(true); fullGC(); - await Bun.sleep(10); + await Bun.sleep(0); } + console.log(second); + + sock.end(); + process.exit(0); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + // Must actually reach the 503 guard — empty would mean the socket was closed + // before dispatch and the guard was never exercised. + expect(stdout.trim()).toMatch(/^HTTP\/1\.1 503\b/); + expect(exitCode).toBe(0); +}); + +test("late keep-alive request to a node:http server after close()+GC answers 503", async () => { + // Same shape as the route test above, but through node:http so the request + // dispatches via on_node_http_request_with_upgrade_ctx — the trampoline that + // was missing the 503 guard until the respond_stopped_503 helper sweep. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("node:http"); + const { fullGC } = require("bun:jsc"); + + let received = ""; + let sockClosed = false; + let waiter = Promise.withResolvers(); + const nextResponse = async () => { + while (true) { + const headerEnd = received.indexOf("\\r\\n\\r\\n"); + if (headerEnd !== -1) { + const head = received.slice(0, headerEnd); + const m = /content-length: (\\d+)/i.exec(head); + const bodyLen = m ? Number(m[1]) : 0; + const total = headerEnd + 4 + bodyLen; + if (received.length >= total) { + const status = head.split("\\r\\n")[0]; + received = received.slice(total); + return status; + } + } + if (sockClosed) return ""; + await waiter.promise; + waiter = Promise.withResolvers(); + } + }; + + const release = Promise.withResolvers(); + const inflight = Promise.withResolvers(); + let hits = 0; - // Reuse the same keep-alive socket. Previously this would dispatch into - // a finalized route list and panic; now it must answer (200 if the - // wrapper happened to survive, 503 if it was collected). - sock.write("GET /r HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\n\\r\\n"); + await (async () => { + const srv = http.createServer(async (req, res) => { + if (++hits === 1) { + inflight.resolve(); + await release.promise; // hold socket non-idle through close() + } + res.writeHead(200, { "content-length": 2 }); + res.end("ok"); + }); + await new Promise(r => srv.listen(0, "127.0.0.1", r)); + + globalThis.sock = await Bun.connect({ + hostname: "127.0.0.1", + port: srv.address().port, + socket: { + data(_s, d) { received += d.toString("latin1"); waiter.resolve(); }, + close() { sockClosed = true; waiter.resolve(); }, + error() { sockClosed = true; waiter.resolve(); }, + }, + }); + + sock.write("GET / HTTP/1.1\\r\\nHost: x\\r\\nConnection: keep-alive\\r\\n\\r\\n"); + await inflight.promise; + // Pipeline the late request while the first is still in-flight. + sock.write("GET / HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\n\\r\\n"); + + // close() → closeIdleConnections() (skips this socket) → stop(). + // Also drops node:http's own reference to the Bun server. + srv.close(); + })(); + + // First request completes → pending_requests → 0 → js_value downgrades. + // The pipelined request then hits on_node_http_request with the + // wrapper gone → 503. + release.resolve(); + const first = await nextResponse(); + if (!first.includes("200")) throw new Error("first request failed: " + first); const second = await nextResponse(); + + for (let i = 0; i < 10; i++) { + Bun.gc(true); + fullGC(); + await Bun.sleep(0); + } console.log(second); sock.end(); @@ -718,11 +846,7 @@ test("late keep-alive request to a route after stop()+GC does not crash", async const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); - // The second request either dispatches (wrapper survived → 200), is refused - // (wrapper downgraded → 503), or never arrives because graceful stop closed - // the idle keep-alive socket (empty). Any of these is correct; the point is - // the process didn't crash. - expect(stdout.trim()).toMatch(/^(|HTTP\/1\.1 (200|503).*)$/s); + expect(stdout.trim()).toMatch(/^HTTP\/1\.1 503\b/); expect(exitCode).toBe(0); }); @@ -2035,6 +2159,76 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { expect(exitCode).toBe(0); }); + // reload({websocket}) that omits a previously-set per-event handler must + // CLEAR that wrapper slot, not leave the old handler pinned. + test("reload() that drops a websocket handler clears its slot", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + /* js */ ` + const { heapStats, fullGC } = require("bun:jsc"); + const liveAsync = () => heapStats().objectTypeCounts.AsyncFunction ?? 0; + + let oldPingFired = 0; + + const baseline = liveAsync(); + const server = Bun.serve({ + port: 0, + fetch: (req, s) => s.upgrade(req) ? undefined : new Response("ok"), + websocket: { + message(ws, m) { ws.send(m); }, + // async so it shows up in objectTypeCounts.AsyncFunction. + ping: async () => { oldPingFired++; }, + }, + }); + const withPing = liveAsync(); + + // Reload with a websocket config that omits ping. The wsOnPing slot + // must be cleared (not left holding the old async closure). + server.reload({ + fetch: (req, s) => s.upgrade(req) ? undefined : new Response("ok"), + websocket: { message(ws, m) { ws.send(m); } }, + }); + for (let i = 0; i < 20 && liveAsync() > baseline; i++) { + Bun.gc(true); + fullGC(); + await Bun.sleep(10); + } + const afterReload = liveAsync(); + + // Behavioral check: a client ping must not reach the dropped handler. + const opened = Promise.withResolvers(); + const echoed = Promise.withResolvers(); + const ws = new WebSocket(server.url.href.replace("http", "ws")); + ws.onopen = () => opened.resolve(); + ws.onerror = e => { opened.reject(e); echoed.reject(e); }; + ws.onmessage = e => echoed.resolve(e.data); + await opened.promise; + ws.ping("p"); + ws.send("hi"); // round-trip after the ping so any ping dispatch has happened + await echoed.promise; + ws.close(); + server.stop(true); + + console.log(JSON.stringify({ baseline, withPing, afterReload, oldPingFired })); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + const { baseline, withPing, afterReload, oldPingFired } = JSON.parse(stdout.trim()); + expect(withPing).toBeGreaterThan(baseline); // sanity: async ping was counted + expect(afterReload).toBeLessThan(withPing); // dropped slot cleared → old ping collected + expect(oldPingFired).toBe(0); // and never dispatched after reload + expect(exitCode).toBe(0); + }); + // Stress test under aggressive GC — catches missing write barriers. test("serve+ws+reload survives BUN_JSC_collectContinuously=1", async () => { await using proc = Bun.spawn({ @@ -2049,11 +2243,18 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { }); for (let i = 0; i < 10; i++) { const ws = new WebSocket(server.url.href.replace("http", "ws")); - await new Promise(r => { ws.onopen = r; }); + // Reject (don't hang) if the connection drops mid-await — under + // collectContinuously a missing write barrier surfaces as an abrupt + // close/error, and a bare onopen-only resolver would just time out. + const fail = Promise.withResolvers(); + ws.onerror = e => fail.reject(e.error ?? new Error("ws error on iter " + i)); + ws.onclose = e => fail.reject(new Error("ws closed (" + e.code + ") on iter " + i)); + await Promise.race([new Promise(r => { ws.onopen = r; }), fail.promise]); ws.send("hi"); - await new Promise(r => { ws.onmessage = r; }); + await Promise.race([new Promise(r => { ws.onmessage = r; }), fail.promise]); + const closed = new Promise(r => { ws.onclose = r; }); // before close(): event may fire synchronously ws.close(); - await new Promise(r => { ws.onclose = r; }); + await closed; server.reload({ fetch: (req, s) => s.upgrade(req) ? undefined : new Response("ok " + i), websocket: { open() {}, message(ws, m) { ws.send(m + i); } }, From bc209fb5bd304ebc0b3acb2783f6bd9ddb510948 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 15:15:52 -0700 Subject: [PATCH 27/45] serve: gate on_client_error_callback + reload_static_routes on Strong wrapper on_client_error_callback is registered on httpContextData (survives listener close); a socket non-idle at stop can complete after, become idle without re-close, then deliver malformed bytes with js_value=Weak and read a stale on_clienterror shadow. reload_static_routes (pre-existing) wrote routeList on a dead-but-unswept cell post-stop. --- src/runtime/server/server_body.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index ca00b864a7ac..059150355bd2 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2275,10 +2275,8 @@ where } let route_list_value = self.set_routes(); if !route_list_value.is_empty() { - if let Some(server_js_value) = self.js_value.try_get() { - if !server_js_value.is_empty() { - Self::js_gc_route_list_set(server_js_value, &self.global(), route_list_value); - } + if let Some(server_js_value) = self.js_value_for_dispatch() { + Self::js_gc_route_list_set(server_js_value, &self.global(), route_list_value); } } Ok(true) @@ -3519,6 +3517,9 @@ where error_code: u8, raw_packet: &[u8], ) { + if self.js_value_for_dispatch().is_none() { + return; + } let callback = self.on_clienterror; if callback.is_empty() { return; From c502b16b21dd0337656e300aa6351e9ce74eeb22 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 15:54:32 -0700 Subject: [PATCH 28/45] serve: balance websocket count for server-initiated ws.close/terminate close() and terminate() set the closed flag before uws synchronously re-enters on_close, so on_close sees was_closed=true and skips its on_websocket_closed call. Balance the count at the call site so the idle predicate (which now gates downgrade) can go true. --- src/runtime/server/ServerWebSocket.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/runtime/server/ServerWebSocket.rs b/src/runtime/server/ServerWebSocket.rs index 6c3e11da4ac7..0ff57bbedf54 100644 --- a/src/runtime/server/ServerWebSocket.rs +++ b/src/runtime/server/ServerWebSocket.rs @@ -1377,6 +1377,11 @@ impl ServerWebSocket { self.update_flags(|f| f.set_closed(true)); self.websocket().end(code, message_value.slice()); + // on_close re-entered with was_closed=true so it skipped the + // accounting; balance the count here. + if let Some(server) = self.handler().server { + server.on_websocket_closed(); + } Ok(JSValue::UNDEFINED) } @@ -1396,6 +1401,11 @@ impl ServerWebSocket { self.update_flags(|f| f.set_closed(true)); self.websocket().close(); + // on_close re-entered with was_closed=true so it skipped the + // accounting; balance the count here. + if let Some(server) = self.handler().server { + server.on_websocket_closed(); + } Ok(JSValue::UNDEFINED) } From cce3f383f33dbb3eb78502700a2a883d83ca199f Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 12 Jun 2026 22:56:33 +0000 Subject: [PATCH 29/45] [autofix.ci] apply automated fixes --- src/runtime/server/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index d1c8a8300467..a71dcadcbecb 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -617,7 +617,6 @@ pub(crate) fn wrap_handler_slot( } impl NewServer { - /// Per-monomorphization static. /// Rust statics cannot be const-generic; routed through a /// `&'static AtomicBool` so the four (SSL,DEBUG) instantiations share one From 831dcfe94a39ea5f8841174a40eeb9b00e3f368f Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 16:25:07 -0700 Subject: [PATCH 30/45] test: capture true heapStats baseline; longer drain for libuv platforms --- test/js/bun/http/bun-server.test.ts | 145 +++++++++++++++++++--------- 1 file changed, 101 insertions(+), 44 deletions(-) diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index eecfe42e4775..6e8adb52288c 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -871,6 +871,25 @@ test("server wrapper survives GC while a websocket is connected after stop()", a return (c.DebugHTTPServer ?? 0) + (c.HTTPServer ?? 0); }; + async function drain(target) { + for (let i = 0; i < 30 && serverCount() > target; i++) { + Bun.gc(true); + fullGC(); + await new Promise(r => setImmediate(r)); + await Bun.sleep(10); + } + } + + // objectTypeCounts includes the (lazily created) prototype object(s) + // once the first server is constructed — and on libuv platforms both + // Debug and non-Debug prototypes may end up materialized. Create+stop + // a trivial server first so the baseline captures whatever prototype + // floor this build settles at; assertions are then relative to it. + await (async () => { + const s = Bun.serve({ port: 0, fetch: () => new Response("ok") }); + s.stop(true); + })(); + await drain(0); const baseline = serverCount(); const ws = await (async () => { @@ -898,9 +917,10 @@ test("server wrapper survives GC while a websocket is connected after stop()", a // The only \`server\` binding is now out of scope; only the live // websocket keeps the native side around. - for (let i = 0; i < 10; i++) { + for (let i = 0; i < 30; i++) { Bun.gc(true); fullGC(); + await new Promise(r => setImmediate(r)); await Bun.sleep(10); } const afterStopGC = serverCount(); @@ -910,11 +930,7 @@ test("server wrapper survives GC while a websocket is connected after stop()", a ws.close(); await closed.promise; - for (let i = 0; i < 10; i++) { - Bun.gc(true); - fullGC(); - await Bun.sleep(10); - } + await drain(baseline); const afterCloseGC = serverCount(); console.log(JSON.stringify({ baseline, afterStopGC, afterCloseGC })); @@ -930,12 +946,12 @@ test("server wrapper survives GC while a websocket is connected after stop()", a expect(stderr).toBe(""); const { baseline, afterStopGC, afterCloseGC } = JSON.parse(stdout.trim()); // js_value stays Strong while a websocket is connected → GC must not - // collect the wrapper. objectTypeCounts includes the prototype (1) once the - // first server has been created, so the live instance shows as baseline+2. - expect(afterStopGC).toBeGreaterThan(baseline + 1); + // collect the wrapper. baseline already includes the prototype(s), so the + // live instance shows as baseline+1. + expect(afterStopGC).toBeGreaterThan(baseline); // Last websocket closing triggers deinit_if_we_can → downgrade → wrapper // becomes collectable again (no leak). - expect(afterCloseGC).toBeLessThanOrEqual(baseline + 1); + expect(afterCloseGC).toBe(baseline); expect(exitCode).toBe(0); }); @@ -1943,9 +1959,28 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { "-e", /* js */ ` const { heapStats, fullGC } = require("bun:jsc"); - const live = () => heapStats().objectTypeCounts.DebugHTTPServer ?? 0; + const live = () => { + const c = heapStats().objectTypeCounts; + return (c.DebugHTTPServer ?? 0) + (c.HTTPServer ?? 0); + }; + async function drain(target) { + for (let i = 0; i < 30 && live() > target; i++) { + Bun.gc(true); + fullGC(); + await new Promise(r => setImmediate(r)); + await new Promise(r => setTimeout(r, 50)); + } + } + + // Materialize prototype(s) first so baseline = whatever floor this + // build settles at (libuv platforms may surface 2, not 1). + await (async () => { + const s = Bun.serve({ port: 0, development: true, fetch: () => new Response("ok") }); + s.stop(true); + })(); + await drain(0); + const baseline = live(); - const before = live(); await (async () => { const server = Bun.serve({ port: 0, @@ -1959,12 +1994,8 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { server.stop(true); })(); // No live reference to server or its handlers from here. - for (let i = 0; i < 20 && live() > before; i++) { - Bun.gc(true); - fullGC(); - await new Promise(r => setTimeout(r, 50)); - } - console.log(JSON.stringify({ before, after: live() })); + await drain(baseline); + console.log(JSON.stringify({ baseline, after: live() })); `, ], env: bunEnv, @@ -1975,11 +2006,11 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); - const { before, after } = JSON.parse(stdout.trim()); - // objectTypeCounts includes the prototype once the first server is created - // (1), so the post-stop count is before+1 when the instance was collected. - // On main this fails: the cycle keeps the instance alive (after = before+2). - expect(after).toBe(before + 1); + const { baseline, after } = JSON.parse(stdout.trim()); + // baseline already includes the prototype(s); a collected instance returns + // to it exactly. On main this fails: the cycle keeps the instance alive + // (after = baseline+1). + expect(after).toBe(baseline); expect(exitCode).toBe(0); }); @@ -1992,8 +2023,24 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { "-e", /* js */ ` const { heapStats, fullGC } = require("bun:jsc"); - const live = () => heapStats().objectTypeCounts.DebugHTTPServer ?? 0; - const before = live(); + const live = () => { + const c = heapStats().objectTypeCounts; + return (c.DebugHTTPServer ?? 0) + (c.HTTPServer ?? 0); + }; + async function drain(target) { + for (let i = 0; i < 30 && live() > target; i++) { + Bun.gc(true); fullGC(); + await new Promise(r => setImmediate(r)); + await new Promise(r => setTimeout(r, 50)); + } + } + await (async () => { + const s = Bun.serve({ port: 0, development: true, fetch: () => new Response("ok") }); + s.stop(true); + })(); + await drain(0); + const baseline = live(); + await (async () => { const server = Bun.serve({ port: 0, development: true, @@ -2002,11 +2049,8 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { await fetch(server.url, { keepalive: false }); server.stop(true); })(); - for (let i = 0; i < 20 && live() > before + 1; i++) { - Bun.gc(true); fullGC(); - await new Promise(r => setTimeout(r, 50)); - } - console.log(JSON.stringify({ before, after: live() })); + await drain(baseline); + console.log(JSON.stringify({ baseline, after: live() })); `, ], env: bunEnv, @@ -2015,8 +2059,8 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); - const { before, after } = JSON.parse(stdout.trim()); - expect(after).toBe(before + 1); + const { baseline, after } = JSON.parse(stdout.trim()); + expect(after).toBe(baseline); expect(exitCode).toBe(0); }); @@ -2030,19 +2074,30 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { "-e", /* js */ ` const { heapStats, fullGC } = require("bun:jsc"); - const liveServer = () => heapStats().objectTypeCounts.DebugHTTPServer ?? 0; + const liveServer = () => { + const c = heapStats().objectTypeCounts; + return (c.DebugHTTPServer ?? 0) + (c.HTTPServer ?? 0); + }; async function gcUntilCountAtMost(max) { - for (let i = 0; i < 50; i++) { + for (let i = 0; i < 30; i++) { Bun.gc(true); fullGC(); if (liveServer() <= max) return liveServer(); + await new Promise(r => setImmediate(r)); await Bun.sleep(10); } return liveServer(); } - const before = liveServer(); + // Materialize prototype(s) first; baseline = the floor count. + await (async () => { + const s = Bun.serve({ port: 0, development: true, fetch: () => new Response("ok") }); + s.stop(true); + })(); + await gcUntilCountAtMost(0); + const baseline = liveServer(); + const opened = Promise.withResolvers(); const clientOpen = Promise.withResolvers(); const echoed = Promise.withResolvers(); @@ -2093,9 +2148,9 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { // event-loop ref. The WeakRef adds no root — if tracing failed above, // deref() would already be undefined. serverWeak.deref()?.stop(true); - const afterClose = await gcUntilCountAtMost(before + 1); + const afterClose = await gcUntilCountAtMost(baseline); - console.log(JSON.stringify({ before, whileConnected, echo, afterClose })); + console.log(JSON.stringify({ baseline, whileConnected, echo, afterClose })); `, ], env: bunEnv, @@ -2106,12 +2161,12 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); - const { before, whileConnected, echo, afterClose } = JSON.parse(stdout.trim()); - // objectTypeCounts includes the prototype (+1); the instance must also be - // present to prove the ws traced root kept it alive across GC. - expect(whileConnected).toBeGreaterThan(before + 1); + const { baseline, whileConnected, echo, afterClose } = JSON.parse(stdout.trim()); + // baseline already includes the prototype(s); the instance on top of it + // proves the ws traced root kept it alive across GC. + expect(whileConnected).toBeGreaterThan(baseline); expect(echo).toMatch(/^\d+:hi$/); // handler dispatched (server.port captured) - expect(afterClose).toBeLessThanOrEqual(before + 1); // instance collected, prototype may remain + expect(afterClose).toBe(baseline); // instance collected, back to prototype floor expect(exitCode).toBe(0); }); @@ -2136,9 +2191,10 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { }); const beforeReload = liveAsync(); server.reload({ fetch: () => new Response("new") }); - for (let i = 0; i < 20 && liveAsync() > baseline; i++) { + for (let i = 0; i < 30 && liveAsync() > baseline; i++) { Bun.gc(true); fullGC(); + await new Promise(r => setImmediate(r)); await new Promise(r => setTimeout(r, 50)); } console.log(JSON.stringify({ baseline, beforeReload, afterReload: liveAsync() })); @@ -2190,9 +2246,10 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { fetch: (req, s) => s.upgrade(req) ? undefined : new Response("ok"), websocket: { message(ws, m) { ws.send(m); } }, }); - for (let i = 0; i < 20 && liveAsync() > baseline; i++) { + for (let i = 0; i < 30 && liveAsync() > baseline; i++) { Bun.gc(true); fullGC(); + await new Promise(r => setImmediate(r)); await Bun.sleep(10); } const afterReload = liveAsync(); From ee84a666400d6ac420a7d2bae3ef894b3928aba5 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 16:25:28 -0700 Subject: [PATCH 31/45] serve: drop slot getter accessors; revert close_idle_connections in graceful stop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The slot getters were dead — dispatch reads the shadow JSValue. Dropping them removes the #[allow(dead_code)] escapes the inventory test rejects. close_idle_connections in stop_listening(false) was belt-and-suspenders for the late-dispatch window, but it changes the observable graceful-stop contract (existing keep-alive connections survive); the is_strong gate in js_value_for_dispatch is the actual guard. --- src/runtime/server/mod.rs | 103 ++++++-------------------------------- 1 file changed, 16 insertions(+), 87 deletions(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index a71dcadcbecb..0c00cc7f82f4 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -1648,15 +1648,6 @@ impl NewServer { if !abrupt { // S012: `app::ListenSocket` is a ZST opaque — safe deref. bun_opaque::opaque_deref_mut(listener).close(); - // Shut idle keep-alive sockets so a late request can't dispatch - // after the JS wrapper drops its strong root. `JsRef::Weak` holds - // a raw JSValue (not a JSC::Weak), so `try_get` can hand back a - // dead-but-unswept cell and the Finalized→503 guard alone leaves a - // window. In-flight requests aren't idle and drain normally. - if let Some(app) = self.app { - // S012: `NewApp` is a ZST opaque — safe `*mut → &mut` deref. - bun_opaque::opaque_deref_mut(app).close_idle_connections(); - } } else if !self.flags.contains(ServerFlags::TERMINATED) { if let Some(ws) = self.config.websocket.as_mut() { ws.handler.app = None; @@ -2973,18 +2964,11 @@ mod route_list_cached { } } -macro_rules! cached_value_dispatch { - ($get_fn:ident, $set_fn:ident, $get_cached:ident, $set_cached:ident) => { - #[allow(dead_code)] - pub fn $get_fn(server_js: JSValue) -> Option { - match (SSL, DEBUG) { - (false, false) => route_list_cached::http::$get_cached(server_js), - (true, false) => route_list_cached::https::$get_cached(server_js), - (false, true) => route_list_cached::debug_http::$get_cached(server_js), - (true, true) => route_list_cached::debug_https::$get_cached(server_js), - } - } - #[allow(dead_code)] +// Dispatch reads from the shadow JSValue fields, not the wrapper slots, so +// only the slot setter is generated. The slot is the GC-traced root; the +// shadow is the hot-path read. +macro_rules! cached_value_set_dispatch { + ($set_fn:ident, $set_cached:ident) => { pub fn $set_fn(server_js: JSValue, global: &JSGlobalObject, v: JSValue) { match (SSL, DEBUG) { (false, false) => route_list_cached::http::$set_cached(server_js, global, v), @@ -2997,72 +2981,17 @@ macro_rules! cached_value_dispatch { } impl NewServer { - cached_value_dispatch!( - js_gc_on_request_get, - js_gc_on_request_set, - on_request_get_cached, - on_request_set_cached - ); - cached_value_dispatch!( - js_gc_on_error_get, - js_gc_on_error_set, - on_error_get_cached, - on_error_set_cached - ); - cached_value_dispatch!( - js_gc_on_node_http_request_get, - js_gc_on_node_http_request_set, - on_node_h_t_t_p_request_get_cached, - on_node_h_t_t_p_request_set_cached - ); - cached_value_dispatch!( - js_gc_on_client_error_get, - js_gc_on_client_error_set, - on_client_error_get_cached, - on_client_error_set_cached - ); - cached_value_dispatch!( - js_gc_ws_on_open_get, - js_gc_ws_on_open_set, - ws_on_open_get_cached, - ws_on_open_set_cached - ); - cached_value_dispatch!( - js_gc_ws_on_message_get, - js_gc_ws_on_message_set, - ws_on_message_get_cached, - ws_on_message_set_cached - ); - cached_value_dispatch!( - js_gc_ws_on_close_get, - js_gc_ws_on_close_set, - ws_on_close_get_cached, - ws_on_close_set_cached - ); - cached_value_dispatch!( - js_gc_ws_on_drain_get, - js_gc_ws_on_drain_set, - ws_on_drain_get_cached, - ws_on_drain_set_cached - ); - cached_value_dispatch!( - js_gc_ws_on_error_get, - js_gc_ws_on_error_set, - ws_on_error_get_cached, - ws_on_error_set_cached - ); - cached_value_dispatch!( - js_gc_ws_on_ping_get, - js_gc_ws_on_ping_set, - ws_on_ping_get_cached, - ws_on_ping_set_cached - ); - cached_value_dispatch!( - js_gc_ws_on_pong_get, - js_gc_ws_on_pong_set, - ws_on_pong_get_cached, - ws_on_pong_set_cached - ); + cached_value_set_dispatch!(js_gc_on_request_set, on_request_set_cached); + cached_value_set_dispatch!(js_gc_on_error_set, on_error_set_cached); + cached_value_set_dispatch!(js_gc_on_node_http_request_set, on_node_h_t_t_p_request_set_cached); + cached_value_set_dispatch!(js_gc_on_client_error_set, on_client_error_set_cached); + cached_value_set_dispatch!(js_gc_ws_on_open_set, ws_on_open_set_cached); + cached_value_set_dispatch!(js_gc_ws_on_message_set, ws_on_message_set_cached); + cached_value_set_dispatch!(js_gc_ws_on_close_set, ws_on_close_set_cached); + cached_value_set_dispatch!(js_gc_ws_on_drain_set, ws_on_drain_set_cached); + cached_value_set_dispatch!(js_gc_ws_on_error_set, ws_on_error_set_cached); + cached_value_set_dispatch!(js_gc_ws_on_ping_set, ws_on_ping_set_cached); + cached_value_set_dispatch!(js_gc_ws_on_pong_set, ws_on_pong_set_cached); /// Mirror all 7 `Handler.on_*` shadows into the wrapper's `m_wsOn*` /// WriteBarrier slots, applying the async-context wrap (deferred from From 560b61733500a8ec8a7af925744ca26ce73a4c95 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 12 Jun 2026 23:27:28 +0000 Subject: [PATCH 32/45] [autofix.ci] apply automated fixes --- src/runtime/server/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 0c00cc7f82f4..8ff6a922a079 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -2983,7 +2983,10 @@ macro_rules! cached_value_set_dispatch { impl NewServer { cached_value_set_dispatch!(js_gc_on_request_set, on_request_set_cached); cached_value_set_dispatch!(js_gc_on_error_set, on_error_set_cached); - cached_value_set_dispatch!(js_gc_on_node_http_request_set, on_node_h_t_t_p_request_set_cached); + cached_value_set_dispatch!( + js_gc_on_node_http_request_set, + on_node_h_t_t_p_request_set_cached + ); cached_value_set_dispatch!(js_gc_on_client_error_set, on_client_error_set_cached); cached_value_set_dispatch!(js_gc_ws_on_open_set, ws_on_open_set_cached); cached_value_set_dispatch!(js_gc_ws_on_message_set, ws_on_message_set_cached); From ca7a771bb34cc3d5f0dd28173456e626e0c42bab Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 16:34:03 -0700 Subject: [PATCH 33/45] serve: re-entrance flag on deinit_if_we_can instead of TERMINATED proxy in ws-close trigger --- src/runtime/server/mod.rs | 52 ++++++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 8ff6a922a079..17ca2db90eef 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -220,6 +220,11 @@ bitflags::bitflags! { const DEINIT_SCHEDULED = 1 << 0; const TERMINATED = 1 << 1; const HAS_HANDLED_ALL_CLOSED_PROMISE = 1 << 2; + /// Set for the duration of [`NewServer::deinit_if_we_can`]; lets a + /// nested call (reached via a callback the body fires) early-return + /// instead of re-running the downgrade/teardown while the outer frame + /// still holds `&mut self`. + const DEINIT_RUNNING = 1 << 3; } } @@ -1653,8 +1658,16 @@ impl NewServer { ws.handler.app = None; } self.flags.insert(ServerFlags::TERMINATED); + // `app.close()` synchronously drains every open websocket; their + // `on_close` defers call `on_websocket_closed`, which would + // dispatch `deinit_if_we_can` through a fresh `&mut NewServer` + // while this frame still holds `&mut self`. Hold DEINIT_RUNNING + // across the drain so that nested call early-returns — `stop()` + // runs `deinit_if_we_can` itself right after this returns. + self.flags.insert(ServerFlags::DEINIT_RUNNING); // S012: `NewApp` is a ZST opaque — safe `*mut → &mut` deref. bun_opaque::opaque_deref_mut(self.app.unwrap()).close(); + self.flags.remove(ServerFlags::DEINIT_RUNNING); } } @@ -1677,6 +1690,27 @@ impl NewServer { #[inline] pub fn deinit_if_we_can(&mut self) { + // Re-entrance guard. The websocket-close trigger reaches this through + // an `AnyServer` raw-ptr dispatch and can land here while an outer + // `&mut self` frame is already on the stack (abrupt `app.close()` + // drains sockets synchronously; their close defers call back in). The + // body is idempotent, so the outer call will finish the work — skip + // the nested run rather than mutate under the aliased borrow. + // This replaces the old `!TERMINATED` proxy in `on_websocket_closed`, + // which was permanent and so also blocked the *post*-stop close defer + // that should fire the downgrade. + if self.flags.contains(ServerFlags::DEINIT_RUNNING) { + return; + } + self.flags.insert(ServerFlags::DEINIT_RUNNING); + let flags = core::ptr::addr_of_mut!(self.flags); + scopeguard::defer! { + // SAFETY: `flags` points into `*self`, which is live for the whole + // body; every intermediate `self.flags` access goes through the + // same `&mut self` this raw ptr was derived from. + unsafe { (*flags).remove(ServerFlags::DEINIT_RUNNING) }; + } + httplog!( "deinitIfWeCan. requests={}, listener={}, websockets={}, has_handled_all_closed_promise={}, all_closed_promise={}, has_js_deinited={}", self.pending_requests, @@ -3632,16 +3666,16 @@ impl AnyServer { /// an already-stopped server, run the idle pass so the `JsRef` downgrade /// (and deferred deinit) that was held back by the open sockets fires. /// - /// Skipped while still listening (the idle pass would no-op) and while - /// TERMINATED: the abrupt-stop path drains every socket synchronously - /// from inside `stop_listening` (which holds `&mut self`), and `stop()` - /// runs `deinit_if_we_can` itself right after. + /// Skipped while still listening (the idle pass would no-op). Re-entrance + /// during the abrupt-stop synchronous drain is handled by the + /// `DEINIT_RUNNING` guard inside `deinit_if_we_can` itself — gating on + /// `TERMINATED` here also blocked the post-`stop(true)` close defer that + /// must fire the downgrade when `stop` was called from inside a close + /// handler (the socket whose handler ran decrements only after `stop` + /// returns, so `stop`'s own idle pass still sees it live). pub(crate) fn on_websocket_closed(&self) { - let drained = any_server_dispatch!(self, |s| { - s.note_websocket_closed() - && !s.has_listener() - && !s.flags.contains(ServerFlags::TERMINATED) - }); + let drained = + any_server_dispatch!(self, |s| s.note_websocket_closed() && !s.has_listener()); if drained { any_server_dispatch_mut!(self, |s| s.deinit_if_we_can()); } From 2f8be6443cf5013bfb17f74499000a4c123a929c Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Fri, 12 Jun 2026 17:09:41 -0700 Subject: [PATCH 34/45] serve: drop dead AnyServer::js_value; fix stale close_idle prose after revert --- src/runtime/server/mod.rs | 14 +++----------- test/js/bun/http/bun-server.test.ts | 20 ++++++++++---------- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 17ca2db90eef..fe1facca99e0 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -3629,18 +3629,10 @@ impl AnyServer { any_server_dispatch!(self, |s| &s.config) } - /// The server's JS wrapper object, or `UNDEFINED` if the `JsRef` has been - /// downgraded and the wrapper collected (post-`stop()` with no user refs). - #[inline] - pub fn js_value(&self) -> JSValue { - any_server_dispatch!(self, |s| s.js_value.try_get().unwrap_or(JSValue::UNDEFINED)) - } - - /// Like [`Self::js_value`] but `None` once the server has gone idle and - /// the `JsRef` downgraded to weak — same gate as + /// The server's JS wrapper object, or `None` once the server has gone idle + /// and the `JsRef` downgraded — same gate as /// [`NewServer::js_value_for_dispatch`], closing the dead-but-unswept - /// window where a `Weak` may hold a stale address. Use this when writing - /// the wrapper into another GC-traced slot. + /// window where a `Weak` may hold a stale address. #[inline] pub fn js_value_if_strong(&self) -> Option { any_server_dispatch!(self, |s| s.js_value_for_dispatch()) diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index 6e8adb52288c..5feff90b8788 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -630,12 +630,12 @@ test("late keep-alive request to a route after stop()+GC answers 503", async () // js_value_assert_alive() panic on that path surfaces as a non-zero exit // instead of taking down the test runner. // - // Graceful stop() calls close_idle_connections(), which would normally close - // our keep-alive socket before the late request can reach the 503 guard. To - // exercise that guard we hold the FIRST request in-flight (so the socket is - // non-idle and survives close_idle) and pipeline the LATE request behind it. - // When the first completes, pending_requests → 0 → js_value downgrades, and - // uws then dispatches the pipelined request into a None wrapper → 503. + // To reach the 503 guard the wrapper must already be downgraded when the + // late request dispatches. We sequence that by holding the FIRST request + // in-flight (pending_requests > 0) across stop(), pipelining the LATE + // request behind it, then releasing: first completes → pending_requests + // drops to 0 → deinit_if_we_can() downgrades js_value → uws reads the + // pipelined request → on_user_route_request hits the is_strong gate → 503. await using proc = Bun.spawn({ cmd: [ bunExe(), @@ -682,7 +682,7 @@ test("late keep-alive request to a route after stop()+GC answers 503", async () "/r": async () => { if (++hits === 1) { inflight.resolve(); - await release.promise; // hold socket non-idle through stop() + await release.promise; // keep pending_requests > 0 across stop() } return new Response("ok"); }, @@ -702,8 +702,8 @@ test("late keep-alive request to a route after stop()+GC answers 503", async () }, }); - // First request: handler parks on \`release\`, keeping the socket - // in-flight so close_idle_connections() skips it. + // First request: handler parks on \`release\`, keeping + // pending_requests > 0 so stop() defers the js_value downgrade. sock.write("GET /r HTTP/1.1\\r\\nHost: x\\r\\nConnection: keep-alive\\r\\n\\r\\n"); await inflight.promise; @@ -711,7 +711,7 @@ test("late keep-alive request to a route after stop()+GC answers 503", async () // until the first response is sent, by which time js_value is Weak. sock.write("GET /r HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\n\\r\\n"); - // Graceful stop: listener closes, close_idle skips this socket. + // Graceful stop: listener closes; downgrade deferred (request in flight). server.stop(); })(); // The only \`server\` binding is now out of scope. From 503496707c8c4f895c5f68073854e039dc991013 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Sun, 14 Jun 2026 13:55:06 -0700 Subject: [PATCH 35/45] serve: review fixups for handler GC-tracing redesign MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - replace DEINIT_RUNNING bitflag with a Cell field; the scopeguard's raw *mut self.flags was invalidated under Stacked Borrows by the &mut self reborrows in the body. Clear inline at the tail (no early returns) instead of holding a borrow across them. - write_ws_handler_slots now takes Option and is called unconditionally on reload, matching wrap_handler_slot's contract so the ws shadows get async-context-wrapped even when the wrapper is gone. Collapse the None/Some branches to one 7-setter list. - bind let-Some(server_js)=js_value_for_dispatch() at each trampoline gate and reuse it instead of re-matching via js_value_assert_alive(). Adds the gate to on_saved_request (defensive — pending_requests currently blocks the downgrade) and notes why on_web_socket_upgrade's id==0 gate is kept despite the inner re-check. - unify on_reload_from_zig's three liveness spellings to js_value_for_dispatch(); fix its stale "declined to adopt" doc. - clear ws.handler.server alongside .app in stop_listening / deinit_if_we_can teardown. - on_drain: copy handler.on_drain to a stack local once, matching the read-once invariant the other five ws callbacks use. - rename AnyServer::js_value_if_strong -> js_value_for_dispatch so a grep for the gate finds every site. - fold the hand-expanded js_gc_route_list_set into cached_value_set_dispatch\! with the other 11 slots. --- src/runtime/api/BunObject.rs | 2 +- src/runtime/server/ServerWebSocket.rs | 7 +- src/runtime/server/mod.rs | 156 +++++++++++++------------- src/runtime/server/server_body.rs | 54 ++++----- 4 files changed, 109 insertions(+), 110 deletions(-) diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 67989d04494d..ad017d0e2f42 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1677,7 +1677,7 @@ pub(crate) fn serve(global_object: &JSGlobalObject, callframe: &CallFrame) -> Js // slots default ZERO so `write_ws_handler_slots`'s clear path // would be 7 wasted FFI calls. if server_ref.config.websocket.is_some() { - server_ref.write_ws_handler_slots(obj, global_object); + server_ref.write_ws_handler_slots(Some(obj), global_object); } server_ref.js_value.set_strong(obj, global_object); diff --git a/src/runtime/server/ServerWebSocket.rs b/src/runtime/server/ServerWebSocket.rs index 0ff57bbedf54..3612a78b6f29 100644 --- a/src/runtime/server/ServerWebSocket.rs +++ b/src/runtime/server/ServerWebSocket.rs @@ -364,7 +364,7 @@ impl ServerWebSocket { // Only mirror the server wrapper while it is strongly rooted — // `js_value()` would return a weak (potentially dead-but-unswept) // address once the server has gone idle and downgraded. - if let Some(server_js) = handler.server.and_then(|s| s.js_value_if_strong()) { + if let Some(server_js) = handler.server.and_then(|s| s.js_value_for_dispatch()) { js::server_set_cached(this_value, global_object, server_js); } this @@ -547,7 +547,8 @@ impl ServerWebSocket { return; } - if !handler.on_drain.is_empty() { + let on_drain = handler.on_drain; + if !on_drain.is_empty() { let global_object = handler.global_object(); let args = [self @@ -559,7 +560,7 @@ impl ServerWebSocket { args: &args, global_object, this_value: JSValue::ZERO, - callback: handler.on_drain, + callback: on_drain, result: JSValue::ZERO, }; let _loop_guard = vm.enter_event_loop_scope(); diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index fe1facca99e0..89f709985412 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -220,11 +220,6 @@ bitflags::bitflags! { const DEINIT_SCHEDULED = 1 << 0; const TERMINATED = 1 << 1; const HAS_HANDLED_ALL_CLOSED_PROMISE = 1 << 2; - /// Set for the duration of [`NewServer::deinit_if_we_can`]; lets a - /// nested call (reached via a callback the body fires) early-return - /// instead of re-running the downgrade/teardown while the outer frame - /// still holds `&mut self`. - const DEINIT_RUNNING = 1 << 3; } } @@ -275,6 +270,14 @@ pub struct NewServer { /// `Cell` because the open/close accounting arrives through shared /// `AnyServer` handles on the JS thread. pub active_websocket_count: core::cell::Cell, + /// Set for the duration of [`NewServer::deinit_if_we_can`]; lets a nested + /// call (reached via a callback the body fires) early-return instead of + /// re-running the downgrade/teardown while the outer frame still holds + /// `&mut self`. A `Cell` rather than a `ServerFlags` bit so the + /// scopeguard's deferred clear holds a `&Cell` (interior mutability) — a + /// raw `*mut ServerFlags` captured before the body forms fresh `&mut self` + /// reborrows would be invalidated under Stacked Borrows. + deinit_running: core::cell::Cell, pub request_pool: *mut request_context::RequestContextStackAllocator, /// Null until the H3 listen path runs (`HAS_H3 && config.http3`); never /// allocated when `!SSL`. Kept as a raw nullable pointer rather than a @@ -903,6 +906,15 @@ impl NewServer { callback: JSValue, extra_args: [JSValue; ARG_COUNT], ) { + // Same is-Strong gate as the network trampolines. Unreachable today — + // the saved request's `pending_requests` increment blocks the + // downgrade — but explicit so a future accounting bug 503s instead of + // panicking in `js_value_assert_alive` below. + // SAFETY: `this` is the live server backref for this request. + let Some(server_js) = unsafe { &*this }.js_value_for_dispatch() else { + server_body::respond_stopped_503(bun_opaque::opaque_deref_mut(resp)); + return; + }; let prepared: PreparedRequest = match &req { SavedRequestUnion::Stack(r) => { // reshaped for borrowck — decouple the inner @@ -949,7 +961,7 @@ impl NewServer { // SAFETY: `this` is the live server backref for this request. let server = unsafe { &*this }; let global = server.global_this(); - let response_value = match callback.call(global, server.js_value_assert_alive(), &args) { + let response_value = match callback.call(global, server_js, &args) { Ok(v) => v, Err(err) => global.take_exception(err), }; @@ -1087,10 +1099,10 @@ impl NewServer { // wrapper can have been collected before this fires. Refuse and close // rather than panicking in js_value_assert_alive() below. // SAFETY: `this` is the live server backref for this request. - if unsafe { &*this }.js_value_for_dispatch().is_none() { + let Some(js_value) = unsafe { &*this }.js_value_for_dispatch() else { 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, @@ -1109,7 +1121,6 @@ impl NewServer { debug_assert!(!on_request.is_empty()); let global = server.global_this(); - let js_value = server.js_value_assert_alive(); let response_value = match on_request.call(global, js_value, &[prepared.js_request, js_value]) { Ok(v) => v, @@ -1138,10 +1149,10 @@ impl NewServer { let index = user_route.id; // SAFETY: `server` is the live backref stored in `user_route`. - if unsafe { &*server }.js_value_for_dispatch().is_none() { + let Some(server_js) = unsafe { &*server }.js_value_for_dispatch() else { server_body::respond_stopped_503(bun_opaque::opaque_deref_mut(resp)); return; - } + }; let should_deinit_context = core::cell::Cell::new(false); let Some(mut prepared) = Self::prepare_js_request_context( @@ -1161,7 +1172,6 @@ impl NewServer { // SAFETY: `server` is the live backref stored in `user_route`. let server_ref = unsafe { &*server }; let global = server_ref.global_this(); - let server_js = server_ref.js_value_assert_alive(); let server_request_list = Self::js_route_list_get_cached(server_js).expect("routeList cached value missing"); let response_value = bun_jsc::host_fn::from_js_host_call(global, || { @@ -1484,24 +1494,8 @@ impl NewServer { } } - /// `js.gc.routeList.set` — write the codegen'd `WriteBarrier` - /// slot on the per-type C++ wrapper so route JS objects stay GC-rooted. - pub fn js_gc_route_list_set(server_js: JSValue, global: &JSGlobalObject, route_list: JSValue) { - match (SSL, DEBUG) { - (false, false) => { - route_list_cached::http::route_list_set_cached(server_js, global, route_list) - } - (true, false) => { - route_list_cached::https::route_list_set_cached(server_js, global, route_list) - } - (false, true) => { - route_list_cached::debug_http::route_list_set_cached(server_js, global, route_list) - } - (true, true) => { - route_list_cached::debug_https::route_list_set_cached(server_js, global, route_list) - } - } - } + // `js_gc_route_list_set` lives with the other slot setters below + // (`cached_value_set_dispatch!` invocation). /// Wrap an already-heap-allocated server pointer in its JS object. /// Ownership transfers to the C++ wrapper (freed via `finalize`). @@ -1656,18 +1650,19 @@ impl NewServer { } else if !self.flags.contains(ServerFlags::TERMINATED) { if let Some(ws) = self.config.websocket.as_mut() { ws.handler.app = None; + ws.handler.server = None; } self.flags.insert(ServerFlags::TERMINATED); // `app.close()` synchronously drains every open websocket; their // `on_close` defers call `on_websocket_closed`, which would // dispatch `deinit_if_we_can` through a fresh `&mut NewServer` - // while this frame still holds `&mut self`. Hold DEINIT_RUNNING - // across the drain so that nested call early-returns — `stop()` - // runs `deinit_if_we_can` itself right after this returns. - self.flags.insert(ServerFlags::DEINIT_RUNNING); + // while this frame still holds `&mut self`. Hold the re-entrance + // guard across the drain so that nested call early-returns — + // `stop()` runs `deinit_if_we_can` itself right after this returns. + self.deinit_running.set(true); // S012: `NewApp` is a ZST opaque — safe `*mut → &mut` deref. bun_opaque::opaque_deref_mut(self.app.unwrap()).close(); - self.flags.remove(ServerFlags::DEINIT_RUNNING); + self.deinit_running.set(false); } } @@ -1699,17 +1694,17 @@ impl NewServer { // This replaces the old `!TERMINATED` proxy in `on_websocket_closed`, // which was permanent and so also blocked the *post*-stop close defer // that should fire the downgrade. - if self.flags.contains(ServerFlags::DEINIT_RUNNING) { + if self.deinit_running.get() { return; } - self.flags.insert(ServerFlags::DEINIT_RUNNING); - let flags = core::ptr::addr_of_mut!(self.flags); - scopeguard::defer! { - // SAFETY: `flags` points into `*self`, which is live for the whole - // body; every intermediate `self.flags` access goes through the - // same `&mut self` this raw ptr was derived from. - unsafe { (*flags).remove(ServerFlags::DEINIT_RUNNING) }; - } + self.deinit_running.set(true); + // Cleared inline at the tail (no early returns below). Not a + // scopeguard: the body forms `&mut self` reborrows for + // `unref()`/`schedule_deinit()`, and a guard holding either `&Cell` or + // a raw `*mut` into `*self` across those would either fail + // borrow-check or have its provenance tag popped under Stacked + // Borrows. A panic mid-body leaves the flag set — acceptable, the idle + // pass panicking means this server is unrecoverable anyway. httplog!( "deinitIfWeCan. requests={}, listener={}, websockets={}, has_handled_all_closed_promise={}, all_closed_promise={}, has_js_deinited={}", @@ -1776,6 +1771,7 @@ impl NewServer { self.js_value.downgrade(); if let Some(ws) = self.config.websocket.as_mut() { ws.handler.app = None; + ws.handler.server = None; } self.unref(); @@ -1795,6 +1791,7 @@ impl NewServer { self.schedule_deinit(); } } + self.deinit_running.set(false); } pub fn schedule_deinit(&mut self) { @@ -2016,6 +2013,7 @@ impl NewServer { js_value: jsc::JsRef::empty(), pending_requests: 0, active_websocket_count: core::cell::Cell::new(0), + deinit_running: core::cell::Cell::new(false), request_pool: >::request_pool(), // Plain HTTP servers never allocate the ~816 KB H3 pool; defer to // the H3-listen path (`listen()` below) so HTTPS servers that @@ -3015,6 +3013,7 @@ macro_rules! cached_value_set_dispatch { } impl NewServer { + cached_value_set_dispatch!(js_gc_route_list_set, route_list_set_cached); cached_value_set_dispatch!(js_gc_on_request_set, on_request_set_cached); cached_value_set_dispatch!(js_gc_on_error_set, on_error_set_cached); cached_value_set_dispatch!( @@ -3034,35 +3033,42 @@ impl NewServer { /// WriteBarrier slots, applying the async-context wrap (deferred from /// `Handler::from_js` so the wrapped fn is rooted the moment it exists). /// Writes all slots unconditionally — `JSValue::ZERO` clears, so a reload - /// that omits a callback drops the previous root. Called after - /// `ptr_to_js` in `serve()` and after the websocket-context swap in - /// `on_reload_from_zig`; dispatch keeps reading the shadow. - pub fn write_ws_handler_slots(&mut self, server_js: JSValue, global: &JSGlobalObject) { - let Some(ws) = self.config.websocket.as_mut() else { - // No websocket config: clear all 7 slots so a config transition - // to "no websocket" drops the previous roots. Redundant on - // initial serve (slots default ZERO) — `serve()` gates this call - // on `websocket.is_some()` — but the doc'd contract is "writes - // all slots unconditionally" so a future call site can't leave - // stale roots behind. - Self::js_gc_ws_on_open_set(server_js, global, JSValue::ZERO); - Self::js_gc_ws_on_message_set(server_js, global, JSValue::ZERO); - Self::js_gc_ws_on_close_set(server_js, global, JSValue::ZERO); - Self::js_gc_ws_on_drain_set(server_js, global, JSValue::ZERO); - Self::js_gc_ws_on_error_set(server_js, global, JSValue::ZERO); - Self::js_gc_ws_on_ping_set(server_js, global, JSValue::ZERO); - Self::js_gc_ws_on_pong_set(server_js, global, JSValue::ZERO); - return; - }; - let h = &mut ws.handler; - let sj = Some(server_js); - wrap_handler_slot(&mut h.on_open, sj, global, Self::js_gc_ws_on_open_set); - wrap_handler_slot(&mut h.on_message, sj, global, Self::js_gc_ws_on_message_set); - wrap_handler_slot(&mut h.on_close, sj, global, Self::js_gc_ws_on_close_set); - wrap_handler_slot(&mut h.on_drain, sj, global, Self::js_gc_ws_on_drain_set); - wrap_handler_slot(&mut h.on_error, sj, global, Self::js_gc_ws_on_error_set); - wrap_handler_slot(&mut h.on_ping, sj, global, Self::js_gc_ws_on_ping_set); - wrap_handler_slot(&mut h.on_pong, sj, global, Self::js_gc_ws_on_pong_set); + /// that omits a callback drops the previous root. `server_js` may be + /// `None` (wrapper already collected during a late reload), in which case + /// the shadows are still wrapped but the slot writes are skipped — same + /// contract as [`wrap_handler_slot`]. Called after `ptr_to_js` in + /// `serve()` and after the websocket-context swap in `on_reload_from_zig`; + /// dispatch keeps reading the shadow. + pub fn write_ws_handler_slots(&mut self, server_js: Option, global: &JSGlobalObject) { + // No websocket config: route a throwaway ZERO through each slot so a + // transition to "no websocket" drops the previous roots. Redundant on + // initial serve (slots default ZERO) — `serve()` gates this call on + // `websocket.is_some()` — but the doc'd contract is "writes all slots + // unconditionally" so a future call site can't leave stale roots behind. + let mut zeros = [JSValue::ZERO; 7]; + let [open, message, close, drain, error, ping, pong] = + match self.config.websocket.as_mut() { + Some(ws) => { + let h = &mut ws.handler; + [ + &mut h.on_open, + &mut h.on_message, + &mut h.on_close, + &mut h.on_drain, + &mut h.on_error, + &mut h.on_ping, + &mut h.on_pong, + ] + } + None => zeros.each_mut(), + }; + wrap_handler_slot(open, server_js, global, Self::js_gc_ws_on_open_set); + wrap_handler_slot(message, server_js, global, Self::js_gc_ws_on_message_set); + wrap_handler_slot(close, server_js, global, Self::js_gc_ws_on_close_set); + wrap_handler_slot(drain, server_js, global, Self::js_gc_ws_on_drain_set); + wrap_handler_slot(error, server_js, global, Self::js_gc_ws_on_error_set); + wrap_handler_slot(ping, server_js, global, Self::js_gc_ws_on_ping_set); + wrap_handler_slot(pong, server_js, global, Self::js_gc_ws_on_pong_set); } } @@ -3634,7 +3640,7 @@ impl AnyServer { /// [`NewServer::js_value_for_dispatch`], closing the dead-but-unswept /// window where a `Weak` may hold a stale address. #[inline] - pub fn js_value_if_strong(&self) -> Option { + pub fn js_value_for_dispatch(&self) -> Option { any_server_dispatch!(self, |s| s.js_value_for_dispatch()) } @@ -3660,7 +3666,7 @@ impl AnyServer { /// /// Skipped while still listening (the idle pass would no-op). Re-entrance /// during the abrupt-stop synchronous drain is handled by the - /// `DEINIT_RUNNING` guard inside `deinit_if_we_can` itself — gating on + /// `deinit_running` guard inside `deinit_if_we_can` itself — gating on /// `TERMINATED` here also blocked the post-`stop(true)` close defer that /// must fire the downgrade when `stop` was called from inside a close /// handler (the socket whose handler ran decrements only after `stop` diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 059150355bd2..aa9204152c0b 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2161,8 +2161,9 @@ where /// configuration (handlers, websocket, routes) with `new_config` and /// re-registers routes on the uws app(s). Ownership of moved-in fields /// transfers to `self.config`; the caller's `new_config` is left in a - /// valid-but-emptied state (`ServerConfig`'s `Drop` then frees whatever - /// was *not* taken — e.g. a websocket block we declined to adopt). + /// valid-but-emptied state and its `Drop` frees whatever was *not* taken. + /// Any `Some(ws)` is adopted unconditionally — `Handler::from_js` already + /// rejected configs with no non-error callback. pub fn on_reload_from_zig(&mut self, new_config: &mut ServerConfig, global: &JSGlobalObject) { httplog!("onReload"); @@ -2182,7 +2183,7 @@ where // async-context re-wrap is unconditional — `with_async_context_if_needed` // is a no-op when no ALS frame is active, so re-wrapping on every // reload keeps the captured frame in sync with the call-time context. - let server_js = self.js_value.try_get().filter(|v| !v.is_empty()); + let server_js = self.js_value_for_dispatch(); if !new_config.on_request.is_empty_or_undefined_or_null() { super::wrap_handler_slot( &mut new_config.on_request, @@ -2218,9 +2219,7 @@ where .set(super::web_socket_server_context::HandlerFlags::SSL, SSL); ws.global_object = bun_ptr::BackRef::new(global); self.config.websocket = Some(ws); - if let Some(server_js) = server_js { - self.write_ws_handler_slots(server_js, global); - } + self.write_ws_handler_slots(server_js, global); } // These get re-applied when we set the static routes again. @@ -2244,10 +2243,8 @@ where let route_list_value = self.set_routes(); if new_config.had_routes_object { - if let Some(server_js_value) = self.js_value.try_get() { - if !server_js_value.is_empty() { - Self::js_gc_route_list_set(server_js_value, global, route_list_value); - } + if let Some(server_js_value) = self.js_value_for_dispatch() { + Self::js_gc_route_list_set(server_js_value, global, route_list_value); } } @@ -2824,10 +2821,10 @@ where let server = unsafe { &mut *server_ptr }; let index = user_route.id; - if server.js_value_for_dispatch().is_none() { + let Some(server_js) = server.js_value_for_dispatch() else { respond_stopped_503(resp); return; - } + }; let should_deinit_context = core::cell::Cell::new(false); let Some(mut prepared) = server.prepare_js_request_context_for::( @@ -2846,8 +2843,7 @@ where // SAFETY: `server_ptr` outlives `prepared`; reborrow to break the // exclusive lifetime tie between `prepared` and `server`. let server = unsafe { &mut *server_ptr }; - let server_request_list = - Self::js_route_list_get_cached(server.js_value_assert_alive()).unwrap(); + let server_request_list = Self::js_route_list_get_cached(server_js).unwrap(); let call_route = if Ctx::IS_H3 { Bun__ServerRouteList__callRouteH3 } else { @@ -2859,7 +2855,7 @@ where global, index, prepared.request_object, - server.js_value_assert_alive(), + server_js, server_request_list, &mut prepared.js_request, std::ptr::from_mut(req).cast::(), @@ -2913,10 +2909,10 @@ where req: &mut Ctx::Req, resp: &mut Ctx::Resp, ) { - if self.js_value_for_dispatch().is_none() { + let Some(js_value) = self.js_value_for_dispatch() else { respond_stopped_503(resp); return; - } + }; let self_ptr: *mut Self = self; let should_deinit_context = core::cell::Cell::new(false); let Some(prepared) = self.prepare_js_request_context_for::( @@ -2936,7 +2932,6 @@ where debug_assert!(!this.config.on_request.is_empty()); let global = this.global_this(); - let js_value = this.js_value_assert_alive(); let on_request_fn = this.config.on_request; let response_value = match on_request_fn.call(global, js_value, &[prepared.js_request, js_value]) { @@ -3211,10 +3206,10 @@ where let server_ptr = server_ref.as_ptr(); let index = this.id; - if server_ref.js_value_for_dispatch().is_none() { + let Some(server_js) = server_ref.js_value_for_dispatch() else { respond_stopped_503(resp); return; - } + }; let should_deinit_context = core::cell::Cell::new(false); // SAFETY: `server_ptr` is the live heap server registered for this route; @@ -3231,8 +3226,6 @@ where }; // SAFETY: `prepared.ctx` is the freshly-allocated RequestContext slot. unsafe { (*prepared.ctx).upgrade_context = Some(upgrade_ctx) }; - // BACKREF: `server_ref` outlives this request (see decl above). - let server_js = server_ref.js_value_assert_alive(); let server_request_list = Self::js_route_list_get_cached(server_js).unwrap(); // S008: `JSGlobalObject` is an `opaque_ffi!` ZST — safe deref. let global = bun_opaque::opaque_deref(server_ref.global_this); @@ -3292,10 +3285,14 @@ where // (mod.rs `app.ws("/*", self_ptr, 0, ..)`); live for the request's // duration. let this = unsafe { &mut *self_ptr }; - if this.js_value_for_dispatch().is_none() { + // Guards both branches below: the `on_request` fallthrough has no + // other gate, and the node:http branch's own re-check (mod.rs: + // `on_node_http_request_with_upgrade_ctx`) is redundant on this path + // but load-bearing for its other caller (`on_node_http_request`). + let Some(server_js) = this.js_value_for_dispatch() else { respond_stopped_503(resp); return; - } + }; if !this.config.on_node_http_request.is_empty() { // NOTE: receiver is `*mut Self` (mod.rs) — the callee re-enters // JS, so a long-lived `&mut self` here would alias on callback. @@ -3359,16 +3356,11 @@ where // We keep the Request object alive for the duration of the request so that we can remove the pointer to the UWS request object. let global = this.global(); - let args = [request_object.to_js(&global), this.js_value_assert_alive()]; + let args = [request_object.to_js(&global), server_js]; let request_value = args[0]; request_value.ensure_still_alive(); - let response_value = - match this - .config - .on_request - .call(&global, this.js_value_assert_alive(), &args) - { + let response_value = match this.config.on_request.call(&global, server_js, &args) { Ok(v) => v, Err(err) => global.take_exception(err), }; From d9e1d97448ec9fd79fc0c35fc0d916829cfaa202 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Sun, 14 Jun 2026 13:57:42 -0700 Subject: [PATCH 36/45] serve: clear ws.handler.server after the abrupt-stop drain, not before The on_close defers reach on_websocket_closed through handler.server, so wiping it before app.close() stranded the live-socket count and the idle pass never saw it drained. Fixup for the previous commit. --- src/runtime/server/mod.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 89f709985412..5cdecebad099 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -1650,7 +1650,6 @@ impl NewServer { } else if !self.flags.contains(ServerFlags::TERMINATED) { if let Some(ws) = self.config.websocket.as_mut() { ws.handler.app = None; - ws.handler.server = None; } self.flags.insert(ServerFlags::TERMINATED); // `app.close()` synchronously drains every open websocket; their @@ -1663,6 +1662,13 @@ impl NewServer { // S012: `NewApp` is a ZST opaque — safe `*mut → &mut` deref. bun_opaque::opaque_deref_mut(self.app.unwrap()).close(); self.deinit_running.set(false); + // Only clear after the drain — `on_close` defers reach + // `on_websocket_closed` through `handler.server`, so wiping it + // earlier would strand the live-socket count and the idle pass + // would never see it drained. + if let Some(ws) = self.config.websocket.as_mut() { + ws.handler.server = None; + } } } From 0d4accc408e6301c85b71f4153e2e0bb1ddede64 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Sun, 14 Jun 2026 13:57:50 -0700 Subject: [PATCH 37/45] test(serve): exercise ws-close auto-downgrade; dedup late keep-alive 503 rig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - drop the redundant serverWeak.deref()?.stop(true) in the ws GC-tracing test so it actually exercises the new on_websocket_closed → deinit_if_we_can auto-downgrade instead of masking it. - extract the shared raw-HTTP/1.1 nextResponse parser + socket plumbing + hold-release-GC protocol from the two "late keep-alive 503" tests into a runLateKeepAlive503 helper parameterized on the server-create snippet. --- test/js/bun/http/bun-server.test.ts | 247 ++++++++++------------------ 1 file changed, 89 insertions(+), 158 deletions(-) diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index 5feff90b8788..40f7d8de92ae 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -622,149 +622,35 @@ test("should be able to await server.stop(true) with keep alive", async () => { expect(async () => await fetch(server.url)).toThrow(); }); -test("late keep-alive request to a route after stop()+GC answers 503", async () => { - // Per-route handlers live in ServerRouteList, which is reachable from JS only - // through the Server wrapper. After a graceful stop() the user may drop the - // wrapper, and a subsequent GC may finalize it before an idle keep-alive - // connection sends one more request. Run in a subprocess so the previous - // js_value_assert_alive() panic on that path surfaces as a non-zero exit - // instead of taking down the test runner. - // - // To reach the 503 guard the wrapper must already be downgraded when the - // late request dispatches. We sequence that by holding the FIRST request - // in-flight (pending_requests > 0) across stop(), pipelining the LATE - // request behind it, then releasing: first completes → pending_requests - // drops to 0 → deinit_if_we_can() downgrades js_value → uws reads the - // pipelined request → on_user_route_request hits the is_strong gate → 503. - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - ` - const { fullGC } = require("bun:jsc"); - - let received = ""; - let sockClosed = false; - let waiter = Promise.withResolvers(); - const nextResponse = () => { - // Wait until a complete HTTP/1.1 response (headers + 2-byte body or - // an empty body for 503) has arrived, then consume it. - return (async () => { - while (true) { - const headerEnd = received.indexOf("\\r\\n\\r\\n"); - if (headerEnd !== -1) { - const head = received.slice(0, headerEnd); - const m = /content-length: (\\d+)/i.exec(head); - const bodyLen = m ? Number(m[1]) : 0; - const total = headerEnd + 4 + bodyLen; - if (received.length >= total) { - const status = head.split("\\r\\n")[0]; - received = received.slice(total); - return status; - } - } - if (sockClosed) return ""; - await waiter.promise; - waiter = Promise.withResolvers(); - } - })(); - }; - - const release = Promise.withResolvers(); - const inflight = Promise.withResolvers(); - let hits = 0; - - await (async () => { - const server = Bun.serve({ - port: 0, - hostname: "127.0.0.1", - routes: { - "/r": async () => { - if (++hits === 1) { - inflight.resolve(); - await release.promise; // keep pending_requests > 0 across stop() - } - return new Response("ok"); - }, - }, - }); - - globalThis.sock = await Bun.connect({ - hostname: "127.0.0.1", - port: server.port, - socket: { - data(socket, data) { - received += data.toString("latin1"); - waiter.resolve(); - }, - close() { sockClosed = true; waiter.resolve(); }, - error() { sockClosed = true; waiter.resolve(); }, - }, - }); - - // First request: handler parks on \`release\`, keeping - // pending_requests > 0 so stop() defers the js_value downgrade. - sock.write("GET /r HTTP/1.1\\r\\nHost: x\\r\\nConnection: keep-alive\\r\\n\\r\\n"); - await inflight.promise; - - // Pipeline the late request behind the held one. uws won't read it - // until the first response is sent, by which time js_value is Weak. - sock.write("GET /r HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\n\\r\\n"); - - // Graceful stop: listener closes; downgrade deferred (request in flight). - server.stop(); - })(); - // The only \`server\` binding is now out of scope. - - // Let the first request complete: pending_requests → 0, - // deinit_if_we_can() downgrades js_value to Weak. uws then reads the - // pipelined request and on_user_route_request sees - // js_value_for_dispatch() == None → 503. Previously: panic. - release.resolve(); - const first = await nextResponse(); - if (!first.includes("200")) throw new Error("first request failed: " + first); - const second = await nextResponse(); - - // Wrapper is now Weak and unreferenced; GC must collect it cleanly. - for (let i = 0; i < 10; i++) { - Bun.gc(true); - fullGC(); - await Bun.sleep(0); - } - console.log(second); - - sock.end(); - process.exit(0); - `, - ], - env: bunEnv, - stdout: "pipe", - stderr: "pipe", - }); - - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stderr).toBe(""); - // Must actually reach the 503 guard — empty would mean the socket was closed - // before dispatch and the guard was never exercised. - expect(stdout.trim()).toMatch(/^HTTP\/1\.1 503\b/); - expect(exitCode).toBe(0); -}); - -test("late keep-alive request to a node:http server after close()+GC answers 503", async () => { - // Same shape as the route test above, but through node:http so the request - // dispatches via on_node_http_request_with_upgrade_ctx — the trampoline that - // was missing the 503 guard until the respond_stopped_503 helper sweep. +// Shared rig for the two "late keep-alive 503" tests below: open a raw TCP +// socket, hold the first request in-flight across stop()/close(), pipeline a +// second request behind it, release, GC, and print the second response's +// status line. The subprocess runs the rig so a (former) panic in the dispatch +// trampoline surfaces as a non-zero exit instead of taking down the runner. +// +// To reach the 503 guard the wrapper must already be downgraded when the late +// request dispatches. We sequence that by holding the FIRST request in-flight +// (pending_requests > 0) across stop(), pipelining the LATE request behind it, +// then releasing: first completes → pending_requests drops to 0 → +// deinit_if_we_can() downgrades js_value → uws reads the pipelined request → +// the trampoline's js_value_for_dispatch() gate fires → 503. +// +// `serverSnippet` must define `port` (the listen port) and `stop()` in scope, +// and may read `release`/`inflight`/`hits` for the hold protocol. +async function runLateKeepAlive503(reqPath: string, serverSnippet: string) { await using proc = Bun.spawn({ cmd: [ bunExe(), "-e", ` - const http = require("node:http"); const { fullGC } = require("bun:jsc"); let received = ""; let sockClosed = false; let waiter = Promise.withResolvers(); + // Wait until a complete HTTP/1.1 response (headers + content-length + // body, or empty for 503) has arrived, then consume + return its + // status line. const nextResponse = async () => { while (true) { const headerEnd = received.indexOf("\\r\\n\\r\\n"); @@ -790,19 +676,11 @@ test("late keep-alive request to a node:http server after close()+GC answers 503 let hits = 0; await (async () => { - const srv = http.createServer(async (req, res) => { - if (++hits === 1) { - inflight.resolve(); - await release.promise; // hold socket non-idle through close() - } - res.writeHead(200, { "content-length": 2 }); - res.end("ok"); - }); - await new Promise(r => srv.listen(0, "127.0.0.1", r)); + ${serverSnippet} globalThis.sock = await Bun.connect({ hostname: "127.0.0.1", - port: srv.address().port, + port, socket: { data(_s, d) { received += d.toString("latin1"); waiter.resolve(); }, close() { sockClosed = true; waiter.resolve(); }, @@ -810,24 +688,28 @@ test("late keep-alive request to a node:http server after close()+GC answers 503 }, }); - sock.write("GET / HTTP/1.1\\r\\nHost: x\\r\\nConnection: keep-alive\\r\\n\\r\\n"); + // First request: handler parks on \`release\`, keeping + // pending_requests > 0 so stop() defers the js_value downgrade. + sock.write("GET ${reqPath} HTTP/1.1\\r\\nHost: x\\r\\nConnection: keep-alive\\r\\n\\r\\n"); await inflight.promise; - // Pipeline the late request while the first is still in-flight. - sock.write("GET / HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\n\\r\\n"); + // Pipeline the late request behind the held one. uws won't read it + // until the first response is sent, by which time js_value is Weak. + sock.write("GET ${reqPath} HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\n\\r\\n"); - // close() → closeIdleConnections() (skips this socket) → stop(). - // Also drops node:http's own reference to the Bun server. - srv.close(); + // Graceful stop: listener closes; downgrade deferred (request in flight). + stop(); })(); + // The only server binding is now out of scope. // First request completes → pending_requests → 0 → js_value downgrades. - // The pipelined request then hits on_node_http_request with the - // wrapper gone → 503. + // The pipelined request then hits the trampoline with the wrapper + // gone → 503. Previously: panic. release.resolve(); const first = await nextResponse(); if (!first.includes("200")) throw new Error("first request failed: " + first); const second = await nextResponse(); + // Wrapper is now Weak and unreferenced; GC must collect it cleanly. for (let i = 0; i < 10; i++) { Bun.gc(true); fullGC(); @@ -846,8 +728,60 @@ test("late keep-alive request to a node:http server after close()+GC answers 503 const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); + // Must actually reach the 503 guard — empty would mean the socket was closed + // before dispatch and the guard was never exercised. expect(stdout.trim()).toMatch(/^HTTP\/1\.1 503\b/); expect(exitCode).toBe(0); +} + +test("late keep-alive request to a route after stop()+GC answers 503", async () => { + // Per-route handlers live in ServerRouteList, which is reachable from JS only + // through the Server wrapper — exercises on_user_route_request's gate. + await runLateKeepAlive503( + "/r", + ` + const server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + routes: { + "/r": async () => { + if (++hits === 1) { + inflight.resolve(); + await release.promise; // keep pending_requests > 0 across stop() + } + return new Response("ok"); + }, + }, + }); + const port = server.port; + const stop = () => server.stop(); + `, + ); +}); + +test("late keep-alive request to a node:http server after close()+GC answers 503", async () => { + // Same shape but through node:http so the request dispatches via + // on_node_http_request_with_upgrade_ctx — the trampoline that was missing + // the 503 guard until the respond_stopped_503 helper sweep. + await runLateKeepAlive503( + "/", + ` + const http = require("node:http"); + const srv = http.createServer(async (req, res) => { + if (++hits === 1) { + inflight.resolve(); + await release.promise; // hold socket non-idle through close() + } + res.writeHead(200, { "content-length": 2 }); + res.end("ok"); + }); + await new Promise(r => srv.listen(0, "127.0.0.1", r)); + const port = srv.address().port; + // close() → closeIdleConnections() (skips this socket) → stop(). + // Also drops node:http's own reference to the Bun server. + const stop = () => srv.close(); + `, + ); }); test("server wrapper survives GC while a websocket is connected after stop()", async () => { @@ -2108,7 +2042,7 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { // adding a GC root ourselves. Assign client directly to the outer var // rather than returning it — returning keeps the async frame's scope // (which contains server) alive via the resolved-value chain in JSC. - let client, serverWeak; + let client; await (async () => { const server = Bun.serve({ port: 0, @@ -2120,7 +2054,6 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { message(ws, m) { ws.send(server.port + ":" + m); }, }, }); - serverWeak = new WeakRef(server); client = new WebSocket(server.url.href.replace("http", "ws")); client.onopen = () => clientOpen.resolve(); client.onmessage = e => echoed.resolve(e.data); @@ -2143,11 +2076,9 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { client.close(); await closed.promise; client = null; - // Graceful stop with an open ws does not auto-finalize when the ws - // closes; force-finish via the WeakRef so the native side drops its - // event-loop ref. The WeakRef adds no root — if tracing failed above, - // deref() would already be undefined. - serverWeak.deref()?.stop(true); + // The last ws closing triggers on_websocket_closed → deinit_if_we_can, + // which downgrades the wrapper without an explicit stop(true) — that's + // the path under test, so no force-finish here. const afterClose = await gcUntilCountAtMost(baseline); console.log(JSON.stringify({ baseline, whileConnected, echo, afterClose })); From b7bfacedfdcc25fd58f1d48340a6316290c0f68b Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 14 Jun 2026 21:01:21 +0000 Subject: [PATCH 38/45] [autofix.ci] apply automated fixes --- src/runtime/server/mod.rs | 32 +++++++++++++++---------------- src/runtime/server/server_body.rs | 6 +++--- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 5cdecebad099..90640c6dd2a1 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -3052,22 +3052,22 @@ impl NewServer { // `websocket.is_some()` — but the doc'd contract is "writes all slots // unconditionally" so a future call site can't leave stale roots behind. let mut zeros = [JSValue::ZERO; 7]; - let [open, message, close, drain, error, ping, pong] = - match self.config.websocket.as_mut() { - Some(ws) => { - let h = &mut ws.handler; - [ - &mut h.on_open, - &mut h.on_message, - &mut h.on_close, - &mut h.on_drain, - &mut h.on_error, - &mut h.on_ping, - &mut h.on_pong, - ] - } - None => zeros.each_mut(), - }; + let [open, message, close, drain, error, ping, pong] = match self.config.websocket.as_mut() + { + Some(ws) => { + let h = &mut ws.handler; + [ + &mut h.on_open, + &mut h.on_message, + &mut h.on_close, + &mut h.on_drain, + &mut h.on_error, + &mut h.on_ping, + &mut h.on_pong, + ] + } + None => zeros.each_mut(), + }; wrap_handler_slot(open, server_js, global, Self::js_gc_ws_on_open_set); wrap_handler_slot(message, server_js, global, Self::js_gc_ws_on_message_set); wrap_handler_slot(close, server_js, global, Self::js_gc_ws_on_close_set); diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index aa9204152c0b..9195b5b3051b 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -3361,9 +3361,9 @@ where request_value.ensure_still_alive(); let response_value = match this.config.on_request.call(&global, server_js, &args) { - Ok(v) => v, - Err(err) => global.take_exception(err), - }; + Ok(v) => v, + Err(err) => global.take_exception(err), + }; let request_object_ptr: *mut Request = request_object; scopeguard::defer! { // uWS request will not live longer than this function From 3385b43f7eb8867528cf6018e7587ff683719404 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Sun, 14 Jun 2026 15:03:43 -0700 Subject: [PATCH 39/45] serve: copy handler.server before uws re-entry; reload writes slots via assert_alive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ws.close/terminate read handler.server after websocket().end()/.close(), which re-enters on_close and may run a user handler that calls stop(true) (clearing handler.server) — copy it first so the count compensation always fires. on_reload_from_zig is reached via a host_fn (wrapper on JS stack) or the hot_map path (removed before downgrade), so the wrapper is alive even when js_value is Weak; using for_dispatch() there skipped the slot writes after stop+reload, leaving the new handlers unrooted in the heap shadow. Also: trim stale WeakRef test prose and the deinit_running Cell doc. --- src/runtime/server/ServerWebSocket.rs | 10 ++++++++-- src/runtime/server/mod.rs | 11 ++++------- src/runtime/server/server_body.rs | 5 ++++- test/js/bun/http/bun-server.test.ts | 7 +++---- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/runtime/server/ServerWebSocket.rs b/src/runtime/server/ServerWebSocket.rs index 3612a78b6f29..6c21c9796222 100644 --- a/src/runtime/server/ServerWebSocket.rs +++ b/src/runtime/server/ServerWebSocket.rs @@ -1376,11 +1376,14 @@ impl ServerWebSocket { break 'brk args.ptr[1].to_slice_or_null(global_this)?; }; + // Copy the server backref BEFORE end(): on_close re-enters and the + // user's close handler may call stop(true), which clears handler.server. + let server = self.handler().server; self.update_flags(|f| f.set_closed(true)); self.websocket().end(code, message_value.slice()); // on_close re-entered with was_closed=true so it skipped the // accounting; balance the count here. - if let Some(server) = self.handler().server { + if let Some(server) = server { server.on_websocket_closed(); } Ok(JSValue::UNDEFINED) @@ -1400,11 +1403,14 @@ impl ServerWebSocket { return Ok(JSValue::UNDEFINED); } + // Copy the server backref BEFORE close(): on_close re-enters and the + // user's close handler may call stop(true), which clears handler.server. + let server = self.handler().server; self.update_flags(|f| f.set_closed(true)); self.websocket().close(); // on_close re-entered with was_closed=true so it skipped the // accounting; balance the count here. - if let Some(server) = self.handler().server { + if let Some(server) = server { server.on_websocket_closed(); } diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 90640c6dd2a1..c8ee1cf2d6d2 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -270,13 +270,10 @@ pub struct NewServer { /// `Cell` because the open/close accounting arrives through shared /// `AnyServer` handles on the JS thread. pub active_websocket_count: core::cell::Cell, - /// Set for the duration of [`NewServer::deinit_if_we_can`]; lets a nested - /// call (reached via a callback the body fires) early-return instead of - /// re-running the downgrade/teardown while the outer frame still holds - /// `&mut self`. A `Cell` rather than a `ServerFlags` bit so the - /// scopeguard's deferred clear holds a `&Cell` (interior mutability) — a - /// raw `*mut ServerFlags` captured before the body forms fresh `&mut self` - /// reborrows would be invalidated under Stacked Borrows. + /// Set across [`NewServer::deinit_if_we_can`] and the synchronous + /// `app.close()` drain in `stop_listening`; lets a nested call (reached + /// via a callback the body fires) early-return instead of re-running the + /// downgrade/teardown while the outer frame still holds `&mut self`. deinit_running: core::cell::Cell, pub request_pool: *mut request_context::RequestContextStackAllocator, /// Null until the H3 listen path runs (`HAS_H3 && config.http3`); never diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 9195b5b3051b..c1a300c56589 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2183,7 +2183,10 @@ where // async-context re-wrap is unconditional — `with_async_context_if_needed` // is a no-op when no ALS frame is active, so re-wrapping on every // reload keeps the captured frame in sync with the call-time context. - let server_js = self.js_value_for_dispatch(); + // `on_reload` is a host_fn — the wrapper is `callframe.this()` on the + // JS stack, alive even if `js_value` was downgraded after stop(). The + // slot writes must reach it so the new handlers are GC-rooted. + let server_js = Some(self.js_value_assert_alive()); if !new_config.on_request.is_empty_or_undefined_or_null() { super::wrap_handler_slot( &mut new_config.on_request, diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index 40f7d8de92ae..eef97c5dc3c3 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -2038,10 +2038,9 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { const closed = Promise.withResolvers(); // Scope server so the only post-stop root is the connected websocket. - // Hold a WeakRef so we can stop(true) after the ws closes without - // adding a GC root ourselves. Assign client directly to the outer var - // rather than returning it — returning keeps the async frame's scope - // (which contains server) alive via the resolved-value chain in JSC. + // Assign client directly to the outer var rather than returning it — + // returning keeps the async frame's scope (which contains server) + // alive via the resolved-value chain in JSC. let client; await (async () => { const server = Bun.serve({ From beaddead11f1dba04049ae4c2428447247b0b0f9 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Sun, 14 Jun 2026 22:51:00 -0700 Subject: [PATCH 40/45] serve: reuse server_js for routeList slot in reload; trim stale assert_alive prose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The handler-slot writes in on_reload_from_zig switched to assert_alive but the routeList slot write below still re-derived via for_dispatch (None on Weak). reload_static_routes keeps for_dispatch — that path is async and can fire post-downgrade. --- src/runtime/server/mod.rs | 6 +++--- src/runtime/server/server_body.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index c8ee1cf2d6d2..4689c3c32493 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -906,7 +906,7 @@ impl NewServer { // Same is-Strong gate as the network trampolines. Unreachable today — // the saved request's `pending_requests` increment blocks the // downgrade — but explicit so a future accounting bug 503s instead of - // panicking in `js_value_assert_alive` below. + // dispatching with a stale wrapper. // SAFETY: `this` is the live server backref for this request. let Some(server_js) = unsafe { &*this }.js_value_for_dispatch() else { server_body::respond_stopped_503(bun_opaque::opaque_deref_mut(resp)); @@ -1093,8 +1093,8 @@ impl NewServer { resp: *mut uws_sys::NewAppResponse, ) { // Idle keep-alive sockets aren't counted in pending_requests, so the - // wrapper can have been collected before this fires. Refuse and close - // rather than panicking in js_value_assert_alive() below. + // wrapper can have downgraded before this fires. Refuse and close + // rather than dispatching with a stale wrapper. // SAFETY: `this` is the live server backref for this request. let Some(js_value) = unsafe { &*this }.js_value_for_dispatch() else { server_body::respond_stopped_503(bun_opaque::opaque_deref_mut(resp)); diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index c1a300c56589..7b90898ec03c 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2246,7 +2246,7 @@ where let route_list_value = self.set_routes(); if new_config.had_routes_object { - if let Some(server_js_value) = self.js_value_for_dispatch() { + if let Some(server_js_value) = server_js { Self::js_gc_route_list_set(server_js_value, global, route_list_value); } } From b7230ced7294446a1a4bb55255409c7b9e593930 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 15 Jun 2026 19:42:53 +0000 Subject: [PATCH 41/45] serve: collapse wrap_handler_slot server_js to plain JSValue Every caller passes Some(live_wrapper); the None arm was dead after beaddead switched reload to js_value_assert_alive(). Drop the Option wrapper and the vestigial 'wrapper already collected' doc clauses. Also shorten the heapStats drain sleep (50ms -> 10ms) and give the subprocess GC-polling tests a 15s budget to match the existing collectContinuously cases; they were brushing the 5s default under debug+ASAN. --- src/runtime/api/BunObject.rs | 8 ++++---- src/runtime/server/mod.rs | 27 ++++++++++++--------------- src/runtime/server/server_body.rs | 8 +++----- test/js/bun/http/bun-server.test.ts | 14 +++++++------- 4 files changed, 26 insertions(+), 31 deletions(-) diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index ad017d0e2f42..4fce346f4732 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1657,19 +1657,19 @@ pub(crate) fn serve(global_object: &JSGlobalObject, callframe: &CallFrame) -> Js // `serve()` stack across `init`/`listen` until this point. crate::server::wrap_handler_slot( &mut server_ref.config.on_request, - Some(obj), + obj, global_object, <$ServerType>::js_gc_on_request_set, ); crate::server::wrap_handler_slot( &mut server_ref.config.on_error, - Some(obj), + obj, global_object, <$ServerType>::js_gc_on_error_set, ); crate::server::wrap_handler_slot( &mut server_ref.config.on_node_http_request, - Some(obj), + obj, global_object, <$ServerType>::js_gc_on_node_http_request_set, ); @@ -1677,7 +1677,7 @@ pub(crate) fn serve(global_object: &JSGlobalObject, callframe: &CallFrame) -> Js // slots default ZERO so `write_ws_handler_slots`'s clear path // would be 7 wasted FFI calls. if server_ref.config.websocket.is_some() { - server_ref.write_ws_handler_slots(Some(obj), global_object); + server_ref.write_ws_handler_slots(obj, global_object); } server_ref.js_value.set_strong(obj, global_object); diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 4689c3c32493..3e00d6558584 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -599,14 +599,15 @@ impl NewServer { /// wrapper's WriteBarrier slot". If `*shadow` is unset (empty/undefined/null), /// normalize it to `ZERO` and clear the slot; otherwise apply the /// async-context wrap and write the wrapped fn into both the slot and -/// `*shadow`. `server_js` may be `None` (wrapper already collected during a -/// late reload), in which case only the shadow is updated. Keeping the -/// is-empty check, the wrap step, and the shadow↔slot pairing in one helper -/// is what stops the serve / reload / ws / clientError sites from drifting. +/// `*shadow`. Every call site already holds a live wrapper (`ptr_to_js` on +/// serve, `callframe.this()` on reload / setOnClientError), so `server_js` +/// is always valid. Keeping the is-empty check, the wrap step, and the +/// shadow↔slot pairing in one helper is what stops the serve / reload / ws / +/// clientError sites from drifting. #[inline] pub(crate) fn wrap_handler_slot( shadow: &mut JSValue, - server_js: Option, + server_js: JSValue, global: &JSGlobalObject, set: fn(JSValue, &JSGlobalObject, JSValue), ) { @@ -615,9 +616,7 @@ pub(crate) fn wrap_handler_slot( } else { shadow.with_async_context_if_needed(global) }; - if let Some(server_js) = server_js { - set(server_js, global, v); - } + set(server_js, global, v); *shadow = v; } @@ -3036,13 +3035,11 @@ impl NewServer { /// WriteBarrier slots, applying the async-context wrap (deferred from /// `Handler::from_js` so the wrapped fn is rooted the moment it exists). /// Writes all slots unconditionally — `JSValue::ZERO` clears, so a reload - /// that omits a callback drops the previous root. `server_js` may be - /// `None` (wrapper already collected during a late reload), in which case - /// the shadows are still wrapped but the slot writes are skipped — same - /// contract as [`wrap_handler_slot`]. Called after `ptr_to_js` in - /// `serve()` and after the websocket-context swap in `on_reload_from_zig`; - /// dispatch keeps reading the shadow. - pub fn write_ws_handler_slots(&mut self, server_js: Option, global: &JSGlobalObject) { + /// that omits a callback drops the previous root. Same contract as + /// [`wrap_handler_slot`]; called after `ptr_to_js` in `serve()` and after + /// the websocket-context swap in `on_reload_from_zig`; dispatch keeps + /// reading the shadow. + pub fn write_ws_handler_slots(&mut self, server_js: JSValue, global: &JSGlobalObject) { // No websocket config: route a throwaway ZERO through each slot so a // transition to "no websocket" drops the previous roots. Redundant on // initial serve (slots default ZERO) — `serve()` gates this call on diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 7b90898ec03c..ee3aa6468ca7 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2186,7 +2186,7 @@ where // `on_reload` is a host_fn — the wrapper is `callframe.this()` on the // JS stack, alive even if `js_value` was downgraded after stop(). The // slot writes must reach it so the new handlers are GC-rooted. - let server_js = Some(self.js_value_assert_alive()); + let server_js = self.js_value_assert_alive(); if !new_config.on_request.is_empty_or_undefined_or_null() { super::wrap_handler_slot( &mut new_config.on_request, @@ -2246,9 +2246,7 @@ where let route_list_value = self.set_routes(); if new_config.had_routes_object { - if let Some(server_js_value) = server_js { - Self::js_gc_route_list_set(server_js_value, global, route_list_value); - } + Self::js_gc_route_list_set(server_js, global, route_list_value); } if self.inspector_server_id.get() != 0 { @@ -3666,7 +3664,7 @@ pub(super) fn server_set_on_client_error_( this.on_clienterror = callback; super::wrap_handler_slot( &mut this.on_clienterror, - Some(server), + server, global, <$T>::js_gc_on_client_error_set, ); diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index eef97c5dc3c3..0caa1c12b891 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -887,7 +887,7 @@ test("server wrapper survives GC while a websocket is connected after stop()", a // becomes collectable again (no leak). expect(afterCloseGC).toBe(baseline); expect(exitCode).toBe(0); -}); +}, 15_000); test("should be able to async upgrade using custom protocol", async () => { const { promise, resolve } = Promise.withResolvers<{ code: number; reason: string } | boolean>(); @@ -1902,7 +1902,7 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { Bun.gc(true); fullGC(); await new Promise(r => setImmediate(r)); - await new Promise(r => setTimeout(r, 50)); + await Bun.sleep(10); } } @@ -1946,7 +1946,7 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { // (after = baseline+1). expect(after).toBe(baseline); expect(exitCode).toBe(0); - }); + }, 15_000); // Control: a handler that does NOT close over server is collected on main // today. This pins that the redesign doesn't regress the non-cycle case. @@ -1965,7 +1965,7 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { for (let i = 0; i < 30 && live() > target; i++) { Bun.gc(true); fullGC(); await new Promise(r => setImmediate(r)); - await new Promise(r => setTimeout(r, 50)); + await Bun.sleep(10); } } await (async () => { @@ -1996,7 +1996,7 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { const { baseline, after } = JSON.parse(stdout.trim()); expect(after).toBe(baseline); expect(exitCode).toBe(0); - }); + }, 15_000); // JSServerWebSocket holds a traced reference to the JSServer wrapper, so the // server (and its ws handlers) stay alive while any websocket is connected, @@ -2098,7 +2098,7 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { expect(echo).toMatch(/^\d+:hi$/); // handler dispatched (server.port captured) expect(afterClose).toBe(baseline); // instance collected, back to prototype floor expect(exitCode).toBe(0); - }); + }, 15_000); // Reload swaps handlers via WriteBarrier .set() — old handlers become // unreachable once nothing else holds them. @@ -2125,7 +2125,7 @@ describe("handler GC tracing (heapStats wrapper-count)", () => { Bun.gc(true); fullGC(); await new Promise(r => setImmediate(r)); - await new Promise(r => setTimeout(r, 50)); + await Bun.sleep(10); } console.log(JSON.stringify({ baseline, beforeReload, afterReload: liveAsync() })); server.stop(true); From ab24590e59eb918a09dcdba1d07c727a19a77ac8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 9 Jul 2026 12:36:21 +0000 Subject: [PATCH 42/45] ci: retrigger From 35f9dc6ff560a3ff268631f162c296d9bc681b1c Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 9 Jul 2026 13:00:57 +0000 Subject: [PATCH 43/45] [autofix.ci] apply automated fixes --- src/runtime/server/server_body.rs | 5 +---- test/js/bun/http/bun-server.test.ts | 10 +++++++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 58e5c0bc56e7..3ed270e90f22 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -3362,10 +3362,7 @@ where // We keep the Request object alive for the duration of the request so that we can remove the pointer to the UWS request object. let global = this.global(); // SAFETY: `request_object_ptr` is live; no other borrow is outstanding. - let args = [ - unsafe { (*request_object_ptr).to_js(&global) }, - server_js, - ]; + let args = [unsafe { (*request_object_ptr).to_js(&global) }, server_js]; let request_value = args[0]; request_value.ensure_still_alive(); diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index b4890a0e1332..c9760186060d 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -1,6 +1,14 @@ import type { Server, ServerWebSocket, Socket } from "bun"; import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isWindows, normalizeBunSnapshot, rejectUnauthorizedScope, tempDirWithFiles, tls } from "harness"; +import { + bunEnv, + bunExe, + isWindows, + normalizeBunSnapshot, + rejectUnauthorizedScope, + tempDirWithFiles, + tls, +} from "harness"; import path from "path"; describe.concurrent("Server", () => { From 3feca20247ce1200ff6e773d7add21b323b24a6c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 9 Jul 2026 13:32:10 +0000 Subject: [PATCH 44/45] serve(ws): re-check is_closed() after reason toString() in ServerWebSocket.close to_slice_or_null on the reason arg can run user toString(), which may re-entrantly call ws.close(). The inner call sets closed=true and runs the on_websocket_closed compensation this PR added; without re-checking, the outer call then decremented the count again (N->N-2 with >=2 sockets). Re-check the guard after coercion so only one decrement runs. Test observes server.pendingWebSockets: 2->1 with the re-check, 2->0 without it (and 2->2 on the released binary, which had no compensation path). --- src/runtime/server/ServerWebSocket.rs | 6 +++ test/js/bun/http/bun-server.test.ts | 65 +++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/src/runtime/server/ServerWebSocket.rs b/src/runtime/server/ServerWebSocket.rs index 33a398faf6b1..413765a905d8 100644 --- a/src/runtime/server/ServerWebSocket.rs +++ b/src/runtime/server/ServerWebSocket.rs @@ -1371,6 +1371,12 @@ impl ServerWebSocket { break 'brk args.ptr[1].to_slice_or_null(global_this)?; }; + // `to_slice_or_null` can run user `toString()`, which may re-entrantly + // `ws.close()` and already decrement the count; re-check the guard. + if self.is_closed() { + return Ok(JSValue::UNDEFINED); + } + // Copy the server backref BEFORE end(): on_close re-enters and the // user's close handler may call stop(true), which clears handler.server. let server = self.handler().server; diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index c9760186060d..dde1cb0b2e1f 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -1870,6 +1870,71 @@ describe("handler liveness across reload/stop", () => { expect(code).toBeGreaterThanOrEqual(1000); }); + test("ws.close() with a reason whose toString() re-enters close() decrements the count once", async () => { + // ServerWebSocket.close coerces the reason arg via toString(), which can + // re-enter ws.close() before the outer call sets the closed flag. The + // re-check after coercion ensures only one on_websocket_closed() runs. + let openCount = 0; + const bothOpen = Promise.withResolvers(); + const firstClosed = Promise.withResolvers(); + let reentered = 0; + let serverSideFirst: unknown; + + using server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch(req, server) { + if (server.upgrade(req)) return; + return new Response(null, { status: 404 }); + }, + websocket: { + open(ws) { + if (openCount === 0) serverSideFirst = ws; + if (++openCount === 2) bothOpen.resolve(); + }, + message(ws, m) { + if (m === "do-close") { + ws.close(1000, { + toString() { + reentered++; + ws.close(); // re-entrant close before outer sets closed=true + return "bye"; + }, + } as unknown as string); + } + }, + close(ws) { + if (ws === serverSideFirst) firstClosed.resolve(); + }, + }, + }); + + const c1 = new WebSocket(`ws://127.0.0.1:${server.port}/`); + const c2 = new WebSocket(`ws://127.0.0.1:${server.port}/`); + const c1Open = Promise.withResolvers(); + const c2Open = Promise.withResolvers(); + const c1Closed = Promise.withResolvers(); + c1.onopen = () => c1Open.resolve(); + c2.onopen = () => c2Open.resolve(); + c1.onerror = e => c1Open.reject(e); + c2.onerror = e => c2Open.reject(e); + c1.onclose = () => c1Closed.resolve(); + await Promise.all([bothOpen.promise, c1Open.promise, c2Open.promise]); + expect(server.pendingWebSockets).toBe(2); + + c1.send("do-close"); + await firstClosed.promise; + await c1Closed.promise; + + // Without the re-check, the outer close() would decrement again: 2→0. + expect({ reentered, pending: server.pendingWebSockets }).toEqual({ reentered: 1, pending: 1 }); + + const c2Closed = Promise.withResolvers(); + c2.onclose = () => c2Closed.resolve(); + c2.close(); + await c2Closed.promise; + }); + test("server.fetch() still dispatches to the handler after stop()", async () => { const server = Bun.serve({ port: 0, From b9275c735be3e18067bc0938ea57e9754ceaec53 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 9 Jul 2026 13:56:26 +0000 Subject: [PATCH 45/45] test(serve): capture re-entrant-close target ws in message(), not by open() order Server-side open() ordering for two same-tick connects is poll-order dependent across platforms; only c1 sends "do-close", so capture its peer there instead. --- test/js/bun/http/bun-server.test.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/js/bun/http/bun-server.test.ts b/test/js/bun/http/bun-server.test.ts index dde1cb0b2e1f..b1419c38fc5b 100644 --- a/test/js/bun/http/bun-server.test.ts +++ b/test/js/bun/http/bun-server.test.ts @@ -1876,9 +1876,9 @@ describe("handler liveness across reload/stop", () => { // re-check after coercion ensures only one on_websocket_closed() runs. let openCount = 0; const bothOpen = Promise.withResolvers(); - const firstClosed = Promise.withResolvers(); + const targetClosed = Promise.withResolvers(); let reentered = 0; - let serverSideFirst: unknown; + let closedTarget: unknown; using server = Bun.serve({ port: 0, @@ -1888,12 +1888,14 @@ describe("handler liveness across reload/stop", () => { return new Response(null, { status: 404 }); }, websocket: { - open(ws) { - if (openCount === 0) serverSideFirst = ws; + open() { if (++openCount === 2) bothOpen.resolve(); }, message(ws, m) { if (m === "do-close") { + // Only c1 sends this; capture c1's server-side peer here rather + // than by open() order, which is not guaranteed across platforms. + closedTarget = ws; ws.close(1000, { toString() { reentered++; @@ -1904,7 +1906,7 @@ describe("handler liveness across reload/stop", () => { } }, close(ws) { - if (ws === serverSideFirst) firstClosed.resolve(); + if (ws === closedTarget) targetClosed.resolve(); }, }, }); @@ -1923,7 +1925,7 @@ describe("handler liveness across reload/stop", () => { expect(server.pendingWebSockets).toBe(2); c1.send("do-close"); - await firstClosed.promise; + await targetClosed.promise; await c1Closed.promise; // Without the re-check, the outer close() would decrement again: 2→0.