diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 1a64301a51ee..09f976b58a53 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1650,6 +1650,37 @@ 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 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. + crate::server::wrap_handler_slot( + &mut server_ref.config.on_request, + obj, + global_object, + <$ServerType>::js_gc_on_request_set, + ); + crate::server::wrap_handler_slot( + &mut server_ref.config.on_error, + obj, + global_object, + <$ServerType>::js_gc_on_error_set, + ); + crate::server::wrap_handler_slot( + &mut server_ref.config.on_node_http_request, + 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.js_value.set_strong(obj, global_object); if global_object.bun_vm().test_isolation_enabled { diff --git a/src/runtime/bake/DevServer.rs b/src/runtime/bake/DevServer.rs index 9a52eb4b0805..029b94f8826d 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) }; } } @@ -5252,13 +5256,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() @@ -5389,8 +5393,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. diff --git a/src/runtime/server/RequestContext.rs b/src/runtime/server/RequestContext.rs index 34e3589b2221..8dd9f3c7c16d 100644 --- a/src/runtime/server/RequestContext.rs +++ b/src/runtime/server/RequestContext.rs @@ -3499,12 +3499,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 95d4ccf20d11..a049ec7b8379 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), @@ -1313,8 +1316,10 @@ 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)); + // 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); @@ -1326,8 +1331,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_; } if let Some(on_request_) = arg.get_truthy(global, "fetch")? { @@ -1335,10 +1339,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_; } 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/ServerWebSocket.rs b/src/runtime/server/ServerWebSocket.rs index d1858f15e7ba..413765a905d8 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 @@ -356,6 +356,12 @@ impl ServerWebSocket { .this_value .set(JsRef::init_strong(this_value, global_object)); js::data_set_cached(this_value, global_object, data_value); + // 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_for_dispatch()) { + js::server_set_cached(this_value, global_object, server_js); + } this } @@ -383,8 +389,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() { @@ -421,17 +431,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(); + } + } } } @@ -523,7 +542,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 @@ -535,7 +555,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(); @@ -628,11 +648,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(); @@ -668,7 +695,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(); @@ -697,10 +727,7 @@ 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); handler.run_error_callback(vm, global_object, err); @@ -1344,8 +1371,22 @@ 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; 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) = server { + server.on_websocket_closed(); + } Ok(JSValue::UNDEFINED) } @@ -1363,8 +1404,16 @@ 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) = server { + server.on_websocket_closed(); + } Ok(JSValue::UNDEFINED) } diff --git a/src/runtime/server/WebSocketServerContext.rs b/src/runtime/server/WebSocketServerContext.rs index 2548f670af85..68edea0d13f5 100644 --- a/src/runtime/server/WebSocketServerContext.rs +++ b/src/runtime/server/WebSocketServerContext.rs @@ -29,15 +29,20 @@ pub struct Handler { pub on_pong: JSValue, pub app: Option<*mut c_void>, + /// 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, - /// 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, /// used by publish() pub flags: HandlerFlags, @@ -68,19 +73,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, @@ -117,9 +109,9 @@ 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), - active_connections: core::cell::Cell::new(0), flags: HandlerFlags::empty(), }; @@ -143,9 +135,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; @@ -161,30 +154,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 { @@ -201,14 +170,6 @@ impl WebSocketServerContext { ..Default::default() } } - - pub fn protect(&self) { - self.handler.protect(); - } - - pub fn unprotect(&self) { - self.handler.unprotect(); - } } bun_core::comptime_string_map! { @@ -428,6 +389,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 1b0f1fb48c43..0ab152ee30c2 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -262,11 +262,27 @@ 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, + /// 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 /// 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, + /// 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 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, @@ -288,7 +304,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, } @@ -302,7 +320,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. @@ -562,6 +580,44 @@ impl NewServer { self.js_value.try_get().expect("js_value alive") } + /// 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::Strong(_) => self.js_value.try_get(), + _ => 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`. 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: JSValue, + 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) + }; + set(server_js, global, v); + *shadow = v; +} + +impl NewServer { /// Per-monomorphization static. /// Rust statics cannot be const-generic; routed through a /// `&'static AtomicBool` so the four (SSL,DEBUG) instantiations share one @@ -832,6 +888,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 + // 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)); + return; + }; let prepared: PreparedRequest = match &req { SavedRequestUnion::Stack(r) => { // reshaped for borrowck — decouple the inner @@ -878,7 +943,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), }; @@ -1012,6 +1077,14 @@ 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 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)); + return; + }; let should_deinit_context = core::cell::Cell::new(false); let Some(prepared) = Self::prepare_js_request_context( this, @@ -1026,16 +1099,10 @@ 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(); - 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, @@ -1063,6 +1130,12 @@ impl NewServer { let server = user_route.server.cast_mut(); let index = user_route.id; + // SAFETY: `server` is the live backref stored in `user_route`. + 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( server, @@ -1081,7 +1154,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, || { @@ -1148,6 +1220,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 @@ -1185,12 +1262,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 @@ -1398,24 +1470,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`). @@ -1442,10 +1498,25 @@ 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 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 } pub fn has_active_web_sockets(&self) -> bool { @@ -1557,15 +1628,27 @@ 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 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.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; + } } } 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 @@ -1584,6 +1667,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.deinit_running.get() { + return; + } + 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={}", self.pending_requests, @@ -1644,8 +1748,12 @@ 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; + ws.handler.server = None; } self.unref(); @@ -1665,6 +1773,7 @@ impl NewServer { self.schedule_deinit(); } } + self.deinit_running.set(false); } pub fn schedule_deinit(&mut self) { @@ -1846,9 +1955,9 @@ impl NewServer { } } - // owned-field cleanup (all_closed_promise / user_routes / - // config / on_clienterror / 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. @@ -1899,6 +2008,8 @@ impl NewServer { h3_alt_svc: Box::<[u8]>::default(), 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 @@ -1913,7 +2024,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), })); @@ -2041,6 +2152,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 @@ -2357,8 +2469,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. @@ -2680,7 +2792,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); } @@ -2879,16 +2991,98 @@ 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, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong + ); } pub(super) mod https { - bun_jsc::codegen_cached_accessors!("HTTPSServer"; routeList); + bun_jsc::codegen_cached_accessors!( + "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); + bun_jsc::codegen_cached_accessors!( + "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); + bun_jsc::codegen_cached_accessors!( + "DebugHTTPSServer"; routeList, onRequest, onError, onNodeHTTPRequest, onClientError, wsOnOpen, wsOnMessage, wsOnClose, wsOnDrain, wsOnError, wsOnPing, wsOnPong + ); + } +} + +// 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), + (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), + } + } + }; +} + +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!( + 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 + /// `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. 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 + // `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); } } @@ -3455,6 +3649,15 @@ impl AnyServer { any_server_dispatch!(self, |s| &s.config) } + /// 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. + #[inline] + pub fn js_value_for_dispatch(&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(), @@ -3467,6 +3670,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). 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()); + 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; diff --git a/src/runtime/server/server.classes.ts b/src/runtime/server/server.classes.ts index 08cc34caf0c4..8528cf76b031 100644 --- a/src/runtime/server/server.classes.ts +++ b/src/runtime/server/server.classes.ts @@ -90,7 +90,20 @@ function generate(name) { finalize: true, construct: true, noConstructor: true, - values: ["routeList"], + values: [ + "routeList", + "onRequest", + "onError", + "onNodeHTTPRequest", + "onClientError", + "wsOnOpen", + "wsOnMessage", + "wsOnClose", + "wsOnDrain", + "wsOnError", + "wsOnPing", + "wsOnPong", + ], }); } export default [ @@ -335,7 +348,7 @@ export default [ finalize: true, construct: true, klass: {}, - values: ["socket"], + values: ["server"], }), define({ diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index adcbbdf1ee19..3ed270e90f22 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; @@ -330,6 +330,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); @@ -337,6 +338,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) @@ -376,6 +378,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) @@ -402,6 +405,19 @@ 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 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(!R::IS_H3); +} + pub(super) type ServerRequestContext = NewRequestContext, SSL, DEBUG, false>; pub(super) type ServerH3RequestContext = @@ -2148,8 +2164,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"); @@ -2162,47 +2179,53 @@ 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(); - } - // 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(); + // `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. + // `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 = 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, + server_js, + global, + Self::js_gc_on_request_set, + ); + self.config.on_request = new_config.on_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(); + 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() { + 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() { - 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(); - } + ws.global_object = bun_ptr::BackRef::new(global); + self.config.websocket = Some(ws); + self.write_ws_handler_slots(server_js, global); } // These get re-applied when we set the static routes again. @@ -2226,11 +2249,7 @@ 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); - } - } + Self::js_gc_route_list_set(server_js, global, route_list_value); } if self.inspector_server_id.get() != 0 { @@ -2257,10 +2276,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) @@ -2303,7 +2320,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, @@ -2442,9 +2459,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) }; @@ -2698,12 +2715,11 @@ 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(); + if self.all_closed_promise.has_value() { + return self.all_closed_promise.value(); } - *prom = jsc::JSPromiseStrong::init(global); - prom.value() + self.all_closed_promise = jsc::JSPromiseStrong::init(global); + self.all_closed_promise.value() } // `notify_inspector_server_stopped` lives in the unbounded impl block @@ -2713,7 +2729,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); @@ -2810,6 +2826,11 @@ where let server = unsafe { &mut *server_ptr }; let index = user_route.id; + 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::( req, @@ -2827,8 +2848,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 { @@ -2840,7 +2860,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::(), @@ -2894,6 +2914,10 @@ where req: &mut Ctx::Req, resp: &mut Ctx::Resp, ) { + 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::( @@ -2910,16 +2934,10 @@ 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, @@ -3191,6 +3209,11 @@ where let server_ptr = server_ref.as_ptr(); let index = this.id; + 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; // `req`/`resp` are the live uWS handles passed to the route handler. @@ -3206,8 +3229,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); @@ -3267,13 +3288,21 @@ where // (mod.rs `app.ws("/*", self_ptr, 0, ..)`); live for the request's // duration. let this = unsafe { &mut *self_ptr }; - if this.config.on_node_http_request.is_some() { + // 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. 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"); @@ -3333,18 +3362,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(); // SAFETY: `request_object_ptr` is live; no other borrow is outstanding. - let args = [ - unsafe { (*request_object_ptr).to_js(&global) }, - this.js_value_assert_alive(), - ]; + let args = [unsafe { (*request_object_ptr).to_js(&global) }, server_js]; let request_value = args[0]; request_value.ensure_still_alive(); - let response_value = match this.config.on_request.as_ref().unwrap().get().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), }; @@ -3496,9 +3518,13 @@ where error_code: u8, raw_packet: &[u8], ) { - let Some(callback) = self.on_clienterror.get() else { + if self.js_value_for_dispatch().is_none() { return; - }; + } + let callback = self.on_clienterror; + if callback.is_empty() { + return; + } { let is_ssl = SSL; let global = self.global(); @@ -3643,8 +3669,13 @@ 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; + super::wrap_handler_slot( + &mut this.on_clienterror, + 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( 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 4be490e0b093..b1419c38fc5b 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, rejectUnauthorizedScope, tempDirWithFiles, tls } from "harness"; +import { + bunEnv, + bunExe, + isWindows, + normalizeBunSnapshot, + rejectUnauthorizedScope, + tempDirWithFiles, + tls, +} from "harness"; import path from "path"; describe.concurrent("Server", () => { @@ -623,6 +631,273 @@ test("should be able to await server.stop(true) with keep alive", async () => { expect(async () => await fetch(server.url)).toThrow(); }); +// 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 { 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"); + 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 () => { + ${serverSnippet} + + globalThis.sock = await Bun.connect({ + hostname: "127.0.0.1", + port, + socket: { + data(_s, d) { received += d.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 ${reqPath} 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 ${reqPath} HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\n\\r\\n"); + + // 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 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(); + 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 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 () => { + // 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); + }; + + 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 () => { + 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 < 30; i++) { + Bun.gc(true); + fullGC(); + await new Promise(r => setImmediate(r)); + await Bun.sleep(10); + } + const afterStopGC = serverCount(); + + const closed = Promise.withResolvers(); + ws.onclose = () => closed.resolve(); + ws.close(); + await closed.promise; + + await drain(baseline); + 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. 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).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>(); using server = Bun.serve({ @@ -1480,3 +1755,632 @@ 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("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 targetClosed = Promise.withResolvers(); + let reentered = 0; + let closedTarget: 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() { + 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++; + ws.close(); // re-entrant close before outer sets closed=true + return "bye"; + }, + } as unknown as string); + } + }, + close(ws) { + if (ws === closedTarget) targetClosed.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 targetClosed.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, + 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 = () => { + 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 Bun.sleep(10); + } + } + + // 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(); + + 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. + await drain(baseline); + console.log(JSON.stringify({ baseline, 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 { 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); + }, 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. + 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 = () => { + 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 Bun.sleep(10); + } + } + 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, + fetch: () => new Response("ok"), + }); + await fetch(server.url, { keepalive: false }); + server.stop(true); + })(); + await drain(baseline); + console.log(JSON.stringify({ baseline, 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 { 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, + // 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 = () => { + const c = heapStats().objectTypeCounts; + return (c.DebugHTTPServer ?? 0) + (c.HTTPServer ?? 0); + }; + + async function gcUntilCountAtMost(max) { + 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(); + } + + // 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(); + const closed = Promise.withResolvers(); + + // Scope server so the only post-stop root is the connected websocket. + // 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({ + 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); }, + }, + }); + 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; + // 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 })); + `, + ], + 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, 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).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. + 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 < 30 && liveAsync() > baseline; i++) { + Bun.gc(true); + fullGC(); + await new Promise(r => setImmediate(r)); + await Bun.sleep(10); + } + 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); + }); + + // 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 < 30 && liveAsync() > baseline; i++) { + Bun.gc(true); + fullGC(); + await new Promise(r => setImmediate(r)); + 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({ + 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")); + // 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 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 closed; + 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); +});