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

src/runtime/bake/DevServer.rs

@@ -1247,6 +1247,8 @@ 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_` in `get_or_put_route_bundle`.
+                unsafe { bun_ptr::RefCount::<HTMLBundleRoute>::deref(html.html_bundle) };
             }
         }
 
@@ -5376,8 +5378,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,6 @@ 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()
+// Drop covers the `Framework` Strongs and `bundled_html_text`;
+// `client_bundle`, `cached_response`, and `html_bundle` hold intrusive refs
+// released in `DevServer`'s `Drop`.

src/runtime/server/ServerWebSocket.rs

@@ -388,8 +388,10 @@ 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);
+        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 +428,25 @@ 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
                 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 {
+                // The re-entrant `on_close` above skipped its accounting
+                // because the closed flag was already set.
+                if let Some(server) = server {
+                    server.on_websocket_closed();
+                }
+            }
         }
     }
 
@@ -633,11 +643,16 @@ impl ServerWebSocket {
         bun_output::scoped_log!(WebSocketServer, "onClose");
         // TODO: Can this called inside finalize?
         let handler = self.handler();
+        // Copied out: the deferred call may form `&mut NewServer`, which owns
+        // the handler storage.
+        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();
@@ -1349,8 +1364,13 @@ impl ServerWebSocket {
             break 'brk args.ptr[1].to_slice_or_null(global_this)?;
         };
 
+        let server = self.handler().server;
         self.update_flags(|f| f.set_closed(true));
         self.websocket().end(code, message_value.slice());
+        // The re-entrant `on_close` skipped its accounting (closed was set).
+        if let Some(server) = server {
+            server.on_websocket_closed();
+        }
         Ok(JSValue::UNDEFINED)
     }
 
@@ -1368,8 +1388,13 @@ impl ServerWebSocket {
             return Ok(JSValue::UNDEFINED);
         }
 
+        let server = self.handler().server;
         self.update_flags(|f| f.set_closed(true));
         self.websocket().close();
+        // The re-entrant `on_close` skipped its accounting (closed was set).
+        if let Some(server) = server {
+            server.on_websocket_closed();
+        }
 
         Ok(JSValue::UNDEFINED)
     }

src/runtime/server/WebSocketServerContext.rs

@@ -34,10 +34,8 @@ 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>,
+    /// Backref to the owning `NewServer`; set in `set_routes` alongside `app`.
+    pub server: Option<super::AnyServer>,
 
     /// used by publish()
     pub flags: HandlerFlags,
@@ -68,19 +66,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 +104,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(),
         };
 
@@ -172,18 +157,26 @@ impl Handler {
         self.on_pong.protect();
     }
 
-    pub fn unprotect(&self) {
+    pub fn unprotect(&mut 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();
+        // Zero the slots so in-flight dispatches (e.g. a `message` handler
+        // that closed the last socket of a stopped server) see empty values
+        // instead of unrooted cells.
+        for field in [
+            &mut self.on_open,
+            &mut self.on_message,
+            &mut self.on_close,
+            &mut self.on_drain,
+            &mut self.on_error,
+            &mut self.on_ping,
+            &mut self.on_pong,
+        ] {
+            field.unprotect();
+            *field = JSValue::ZERO;
+        }
     }
 }
 
@@ -206,7 +199,7 @@ impl WebSocketServerContext {
         self.handler.protect();
     }
 
-    pub fn unprotect(&self) {
+    pub fn unprotect(&mut self) {
         self.handler.unprotect();
     }
 }

src/runtime/server/mod.rs

@@ -220,6 +220,9 @@ bitflags::bitflags! {
         const DEINIT_SCHEDULED            = 1 << 0;
         const TERMINATED                  = 1 << 1;
         const HAS_HANDLED_ALL_CLOSED_PROMISE = 1 << 2;
+        const HANDLERS_RELEASED           = 1 << 3;
+        /// Transient: held across the abrupt-stop `app.close()` drain.
+        const WEBSOCKETS_DRAINING         = 1 << 4;
     }
 }
 
@@ -265,6 +268,9 @@ pub struct NewServer<const SSL: bool, const DEBUG: bool> {
     pub base_url_string_for_joining: Box<[u8]>,
     pub config: ServerConfig,
     pub pending_requests: usize,
+    /// Live `ServerWebSocket` count; `Cell` because updates arrive through
+    /// shared `AnyServer` handles.
+    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,16 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
         req: &mut uws_sys::Request,
         resp: *mut uws_sys::NewAppResponse<SSL>,
     ) {
+        // A keep-alive connection can deliver a request after the idle pass
+        // released the handlers; close it instead of calling a freed handler.
+        // 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 +1056,7 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
 
         // 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());
+        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 +1087,14 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
         let server = user_route.server.cast_mut();
         let index = user_route.id;
 
+        // Same late-503 path as `on_request`.
+        // 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 +1178,14 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
         use bun_http_jsc::method_jsc::MethodJsc as _;
         use node_http_response::Flags as NhrFlags;
 
+        // Same late-503 path as `on_request`.
+        // 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 +1231,7 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
             },
             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.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 +1483,19 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
     }
 
     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 {
@@ -1577,8 +1607,12 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
                 ws.handler.app = None;
             }
             self.flags.insert(ServerFlags::TERMINATED);
+            // Keep `on_websocket_closed` from re-entering `deinit_if_we_can`
+            // while `app.close()` drains sockets under this `&mut self`.
+            self.flags.insert(ServerFlags::WEBSOCKETS_DRAINING);
             // S012: `NewApp<SSL>` is a ZST opaque — safe `*mut → &mut` deref.
             bun_opaque::opaque_deref_mut(self.app.unwrap()).close();
+            self.flags.remove(ServerFlags::WEBSOCKETS_DRAINING);
         }
     }
 
@@ -1664,9 +1698,21 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
             );
         }
         if self.pending_requests == 0 && !self.has_listener() && !self.has_active_web_sockets() {
+            // Release the handler refs: a handler closing over the JS `Server`
+            // forms a cycle (box → Strong → closure env → wrapper → box) the
+            // GC cannot see through. The counted websocket unprotect runs once
+            // via the terminal HANDLERS_RELEASED flag.
+            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 +1951,7 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
             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 +2094,7 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
             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 +3501,23 @@ 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; draining the last socket of a
+    /// stopped server runs the idle pass that was held back by the open
+    /// sockets. Skipped during the synchronous abrupt-stop drain, which
+    /// holds `&mut self` (`stop()` follows with its own pass).
+    pub(crate) fn on_websocket_closed(&self) {
+        let drained = any_server_dispatch!(self, |s| {
+            s.note_websocket_closed() && !s.flags.contains(ServerFlags::WEBSOCKETS_DRAINING)
+        });
+        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;