Release Bun.serve handler Strongs once the server is idle (native↔JS cycle leak)

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::<HTMLBundleRoute>::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::<HTMLBundleRoute>::ref_(html) };
                     break 'brk route_bundle::Data::Html(route_bundle::Html {

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<Arc<StaticRoute>> 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.

src/runtime/server/ServerWebSocket.rs

@@ -388,8 +388,12 @@
 
         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 @@
         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,13 +646,20 @@
         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();
 
         // Downgrade + signal

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<VirtualMachine>,
     pub global_object: bun_ptr::BackRef<JSGlobalObject>,
-    /// 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<usize>,
+    /// 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<super::AnyServer>,
 
     /// 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(),
         };
 

src/runtime/server/mod.rs

@@ -220,6 +220,7 @@
         const DEINIT_SCHEDULED            = 1 << 0;
         const TERMINATED                  = 1 << 1;
         const HAS_HANDLED_ALL_CLOSED_PROMISE = 1 << 2;
+        const HANDLERS_RELEASED           = 1 << 3;
     }
 }
 
@@ -265,6 +266,11 @@
     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<u32>,
     pub request_pool: *mut request_context::RequestContextStackAllocator<Self, SSL, DEBUG, false>,
     /// Null until the H3 listen path runs (`HAS_H3 && config.http3`); never
     /// allocated when `!SSL`. Kept as a raw nullable pointer rather than a
@@ -1026,6 +1032,20 @@
         req: &mut uws_sys::Request,
         resp: *mut uws_sys::NewAppResponse<SSL>,
     ) {
+        // `deinit_if_we_can` releases the handler Strongs once the server is
+        // fully idle after a graceful `stop()`. A surviving keep-alive
+        // connection can still deliver one more request after that; there is
+        // nothing left to dispatch it to, so 503 and close the connection
+        // instead of calling a released handler. Every uws dispatch
+        // trampoline carries this same guard.
+        // SAFETY: `this` is the live server backref registered as the uws
+        // userdata; no other borrow derived from it is alive here.
+        if unsafe { (*this).config.on_request.is_none() } {
+            // S012: `NewAppResponse<SSL>` is a ZST opaque — safe deref.
+            server_body::reject_stopped(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,
@@ -1040,13 +1060,8 @@
 
         // 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);
-        debug_assert!(!on_request.is_empty());
+        // `Some` is guaranteed by the early return above.
+        let on_request = server.config.on_request.as_ref().unwrap().get();
 
         let global = server.global_this();
         let js_value = server.js_value_assert_alive();
@@ -1077,6 +1092,15 @@
         let server = user_route.server.cast_mut();
         let index = user_route.id;
 
+        // Same late-503 path as `on_request` (see the comment there); the JS
+        // wrapper that owns the route list may already be collected.
+        // SAFETY: `server` is the live backref stored in `user_route`.
+        if unsafe { (*server).flags.contains(ServerFlags::HANDLERS_RELEASED) } {
+            // S012: `NewAppResponse<SSL>` is a ZST opaque — safe deref.
+            server_body::reject_stopped(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(
             server,
@@ -1160,6 +1184,14 @@
         use bun_http_jsc::method_jsc::MethodJsc as _;
         use node_http_response::Flags as NhrFlags;
 
+        // Same late-503 path as `on_request` (see the comment there).
+        // SAFETY: `this` is the live server backref registered as the uws
+        // userdata; no other borrow derived from it is alive here.
+        if unsafe { (*this).config.on_node_http_request.is_none() } {
+            server_body::reject_stopped(resp);
+            return;
+        }
+
         // SAFETY: `this` is the live server backref registered as the uws
         // userdata; only one borrow derived from it is alive at a time.
         unsafe { (*this).on_pending_request() };
@@ -1205,12 +1237,8 @@
             },
             None => JSValue::UNDEFINED,
         };
-        let callback = this_ref
-            .config
-            .on_node_http_request
-            .as_ref()
-            .map(|s| s.get())
-            .unwrap_or(JSValue::ZERO);
+        // `Some` is guaranteed by the early return above.
+        let callback = this_ref.config.on_node_http_request.as_ref().unwrap().get();
         // 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
@@ -1462,10 +1490,19 @@
     }
 
     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 {
@@ -1664,9 +1701,33 @@
             );
         }
         if self.pending_requests == 0 && !self.has_listener() && !self.has_active_web_sockets() {
+            // No request or upgrade can reach the user's handler callbacks once
+            // the listener is gone, the in-flight count is zero, and there are
+            // no live websockets. Release their `Strong` handles now: a handler
+            // defined in a scope that closes over the JS `Server` value
+            // otherwise forms a native↔JS cycle (box → Strong(handler) →
+            // lexical environment → Server wrapper → m_ctx → box) that the GC
+            // cannot see through, so the wrapper never finalizes and the box is
+            // never freed.
+            //
+            // The `Option<Strong>` / `StrongOptional` releases are idempotent
+            // and run on every idle pass. The websocket unprotect is counted
+            // per value, so HANDLERS_RELEASED gates it to exactly once. The
+            // flag is terminal: a released server can never dispatch again
+            // (every uws trampoline rejects on the flag or the cleared
+            // handler, and `on_reload_from_zig` short-circuits before
+            // installing anything new).
+            self.config.on_request = None;
+            self.config.on_node_http_request = None;
+            self.config.on_error = None;
+            self.on_clienterror.deinit();
             if let Some(ws) = self.config.websocket.as_mut() {
                 ws.handler.app = None;
+                if !self.flags.contains(ServerFlags::HANDLERS_RELEASED) {
+                    ws.handler.unprotect();
+                }
             }
+            self.flags.insert(ServerFlags::HANDLERS_RELEASED);
             self.unref();
 
             // Detach DevServer. This is needed because there are aggressive
@@ -1905,6 +1966,7 @@
             h3_alt_svc: Box::<[u8]>::default(),
             js_value: jsc::JsRef::empty(),
             pending_requests: 0,
+            active_websocket_count: core::cell::Cell::new(0),
             request_pool: <Self as ServerPools<SSL, DEBUG>>::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 +2109,7 @@
             websocket.global_object =
                 bun_ptr::BackRef::new(bun_opaque::opaque_deref(self.global_this));
             websocket.handler.app = Some(std::ptr::from_mut(app).cast::<c_void>());
+            websocket.handler.server = Some(any_server);
             websocket
                 .handler
                 .flags
@@ -3453,6 +3516,26 @@
         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 handler release
+    /// (and deferred deinit) that was held back by the open sockets fires.
+    ///
+    /// Skipped 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.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;