diff --git a/packages/bun-types/bun.d.ts b/packages/bun-types/bun.d.ts index d8c9402e2871..a36bd91fc614 100644 --- a/packages/bun-types/bun.d.ts +++ b/packages/bun-types/bun.d.ts @@ -6739,6 +6739,18 @@ declare module "bun" { * @default false */ allowHalfOpen?: boolean; + /** + * Uniquely identify this listener for hot reloading. + * + * When Bun is started with the `--hot` flag, `Bun.listen()` calls that + * resolve to the same `id` reuse the existing listening socket (swapping + * handlers in place) instead of re-binding, which would fail with + * `EADDRINUSE`. If not provided, an id is derived from `hostname`, + * `port`, and `tls`. Pass `null` to opt out of hot-reload reuse. + * + * When Bun is not started with `--hot`, this value is currently unused. + */ + id?: string | null; } interface TCPSocketConnectOptions extends SocketOptions { @@ -6782,6 +6794,21 @@ declare module "bun" { tls?: TLSOptions | boolean; } + interface UnixSocketListenOptions extends UnixSocketOptions { + /** + * Uniquely identify this listener for hot reloading. + * + * When Bun is started with the `--hot` flag, `Bun.listen()` calls that + * resolve to the same `id` reuse the existing listening socket (swapping + * handlers in place) instead of re-binding. If not provided, an id is + * derived from `unix` and `tls`. Pass `null` to opt out of hot-reload + * reuse. + * + * When Bun is not started with `--hot`, this value is currently unused. + */ + id?: string | null; + } + interface FdSocketOptions extends SocketOptions { /** * TLS configuration with which to create the socket @@ -6817,7 +6844,7 @@ declare module "bun" { * * @category HTTP & Networking */ - function listen(options: UnixSocketOptions): UnixSocketListener; + function listen(options: UnixSocketListenOptions): UnixSocketListener; /** * @category HTTP & Networking diff --git a/src/jsc/rare_data.rs b/src/jsc/rare_data.rs index 482f7698d542..00299d274a03 100644 --- a/src/jsc/rare_data.rs +++ b/src/jsc/rare_data.rs @@ -84,15 +84,15 @@ impl HotMap { self._map.get(key).copied() } - /// Untyped insert — typed `insert` lives in `bun_runtime` where the - /// `TaggedPointerUnion` payload list is named. - pub fn insert_raw(&mut self, key: &[u8], entry: HotMapEntry) { + /// Returns `false` and keeps the existing entry if `key` is already registered. + pub fn insert_raw(&mut self, key: &[u8], entry: HotMapEntry) -> bool { let gop = bun_core::handle_oom(self._map.get_or_put(key)); if gop.found_existing { - panic!("HotMap already contains key"); + return false; } // `get_or_put` already boxed the key; the map owns its keys. *gop.value_ptr = entry; + true } pub fn remove(&mut self, key: &[u8]) { diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 25189b61d259..35c5f3ac30cb 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1599,13 +1599,14 @@ fn serve(global_object: &JSGlobalObject, callframe: &CallFrame) -> JsResult(), - }, - ); + let entry = HotMapEntry { + tag: $tag as u8, + ptr: server.cast::<()>(), + }; + // Key held by a `Bun.listen` entry: stay unregistered so `stop()` leaves it alone. + if !hot.insert_raw(&server_ref.config.id, entry) { + server_ref.config.allow_hot = false; + } } } diff --git a/src/runtime/socket/Handlers.rs b/src/runtime/socket/Handlers.rs index 05a15ed2c91f..c87ce51c521d 100644 --- a/src/runtime/socket/Handlers.rs +++ b/src/runtime/socket/Handlers.rs @@ -411,6 +411,13 @@ impl Handlers { self.cell.set_callbacks(global_object, &wrapped); self.binary_type.set(reloaded.binary_type); } + + /// [`apply_reload`](Self::apply_reload) for callbacks `from_generated` already context-wrapped. + pub(crate) fn copy_callbacks_from(&self, global_object: &JSGlobalObject, source: &Handlers) { + self.cell + .set_callbacks(global_object, &source.cell.callbacks()); + self.binary_type.set(source.binary_type.get()); + } } /// One in-flight dispatch into JS. Holds an `Rc` so the callbacks it is about diff --git a/src/runtime/socket/JSSocketHandlers.rs b/src/runtime/socket/JSSocketHandlers.rs index a306ba0fdf35..1456b27e75c0 100644 --- a/src/runtime/socket/JSSocketHandlers.rs +++ b/src/runtime/socket/JSSocketHandlers.rs @@ -136,6 +136,14 @@ impl JSSocketHandlers { Bun__SocketHandlers__setCallbacks(global, self.0, callbacks.as_ptr()); } + /// Inverse of [`set_callbacks`](Self::set_callbacks): unset fields read as `JSValue::ZERO`. + pub(crate) fn callbacks(self) -> [JSValue; CALLBACK_COUNT] { + core::array::from_fn(|i| { + let v = Bun__SocketHandlers__getField(self.0, i as u32); + if v.is_undefined() { JSValue::ZERO } else { v } + }) + } + /// Drops the `open` callback: a client socket clears it after its first TLS /// handshake so renegotiations do not fire it again. #[inline] diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index d36d771db941..c11766558e97 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -67,6 +67,9 @@ fn with_ssl_ctx_cache( // `to_js(self)` impl does would invalidate that link). use crate::generated_classes::js_Listener; +/// `HotMapEntry.tag` for listeners; `crate::server::AnyServerTag` owns 0..=3. +const HOT_MAP_TAG_LISTENER: u8 = 4; + // R-2 (host-fn re-entrancy): every JS-exposed method takes `&self`; per-field // interior mutability via `Cell` (Copy) / `JsCell` (non-Copy). The codegen // shim still emits `this: &mut Listener` — `&mut T` auto-derefs to `&T` @@ -93,6 +96,8 @@ pub struct Listener { /// Reference to this listener's JS wrapper. Strong while it is listening or /// has connections, downgraded to weak once idle so GC can reclaim it. pub this_value: JsCell, + /// `--hot` registry key; non-empty iff registered in `VirtualMachine::hot_map()`. + pub(crate) hot_id: JsCell>, } #[derive(Clone, Copy, Default)] @@ -189,6 +194,55 @@ impl Listener { let ssl_enabled = socket_config.ssl.is_some(); let socket_flags = socket_config.socket_flags(); + // `--hot` reuses the listener a previous evaluation bound (as `Bun.serve` does); `id: null`/`""` opts out. + let hot_id: Box<[u8]> = match opts.get(global, "id")? { + None => compute_hot_id(socket_config.hostname_or_unix.slice(), port, ssl_enabled), + Some(id) if id.is_null() => Box::default(), + Some(id) => { + let slice = id.to_slice(global)?; + let user = slice.slice(); + if user.is_empty() { + Box::default() + } else { + // Prefixed so a user id can never alias a `Bun.serve` key. + let mut buf = Vec::with_capacity(user.len() + 9); + buf.extend_from_slice(b"[listen]-"); + buf.extend_from_slice(user); + buf.into_boxed_slice() + } + } + }; + if !hot_id.is_empty() { + if let Some(hot) = global.bun_vm().as_mut().hot_map() { + if let Some(entry) = hot.get_entry(&hot_id) { + if entry.tag == HOT_MAP_TAG_LISTENER { + // SAFETY: tag matched; `register_for_hot_reload` inserted a + // `*mut Listener` that `do_stop`/`deinit` remove before freeing. + let existing: &Listener = unsafe { &*entry.ptr.cast::() }; + let this_ref = existing.this_value.get(); + if this_ref.is_strong() { + if let Some(this_value) = this_ref.try_get() { + existing + .handlers + .copy_callbacks_from(global, &socket_config.handlers); + let default_data = socket_config.default_data; + existing.strong_data.with_mut(|s| { + if default_data.is_empty() { + s.deinit(); + } else { + s.set(global, default_data); + } + }); + return Ok(this_value); + } + } + // A weak `JSValue` may already be dead; release the port instead. + Listener::do_stop(existing, false); + } + } + } + } + #[cfg(windows)] if port.is_none() { // we check if the path is a named pipe otherwise we try to connect using AF_UNIX @@ -234,6 +288,7 @@ impl Listener { secure_ctx: Cell::new(None), strong_data: JsCell::new(Strong::empty()), this_value: JsCell::new(JsRef::empty()), + hot_id: JsCell::new(Box::default()), })); // SAFETY: just allocated, non-null; every field touched below // is `Cell`/`JsCell` or `&self`, so a shared borrow suffices. @@ -326,6 +381,7 @@ impl Listener { (), )); } + Listener::register_for_hot_reload(this, hot_id); return Ok(this_value); } } @@ -362,6 +418,7 @@ impl Listener { secure_ctx: Cell::new(None), strong_data: JsCell::new(Strong::empty()), this_value: JsCell::new(JsRef::empty()), + hot_id: JsCell::new(Box::default()), })); // SAFETY: just allocated, non-null; every field touched through this // borrow is `Cell`/`JsCell` or `&self`. The one plain-field write @@ -598,6 +655,16 @@ impl Listener { )); } + Listener::register_for_hot_reload(this, hot_id); + if !ssl_enabled { + // S008: `ListenSocket` is an `opaque_ffi!` ZST — safe deref. + let fd = bun_opaque::opaque_deref_mut(listen_socket).fd(); + global + .bun_vm() + .as_mut() + .add_listening_socket_for_watch_mode(fd); + } + Ok(this_value) } @@ -844,6 +911,7 @@ impl Listener { } fn do_stop(this: &Self, force_close: bool) { + Self::unregister_for_hot_reload(this); if matches!(this.listener.get(), ListenerType::None) { return; } @@ -854,8 +922,17 @@ impl Listener { ))); } - if matches!(listener, ListenerType::Uws(_)) { + if let ListenerType::Uws(socket) = listener { Self::unlink_unix_socket_path(this); + if !this.ssl { + // S008: `ListenSocket` is an `opaque_ffi!` ZST — safe deref. + let fd = bun_opaque::opaque_deref_mut(socket).fd(); + this.handlers + .global_object + .bun_vm() + .as_mut() + .remove_listening_socket_for_watch_mode(fd); + } } // The listener's poll_ref tracks the listening socket only; accepted @@ -906,6 +983,15 @@ impl Listener { match listener { ListenerType::Uws(socket) => { Self::unlink_unix_socket_path(&self); + if !self.ssl { + // S008: `ListenSocket` is an `opaque_ffi!` ZST — safe deref. + let fd = bun_opaque::opaque_deref_mut(socket).fd(); + self.handlers + .global_object + .bun_vm() + .as_mut() + .remove_listening_socket_for_watch_mode(fd); + } // S008: `ListenSocket` is an `opaque_ffi!` ZST — safe deref. bun_opaque::opaque_deref_mut(socket).close(); } @@ -926,6 +1012,35 @@ impl Listener { Self::deinit(Box::into_raw(self)); } + /// No-op outside `--hot` (`hot_map()` is `None`) or when the key is taken. + fn register_for_hot_reload(this: *mut Self, hot_id: Box<[u8]>) { + if hot_id.is_empty() { + return; + } + // SAFETY: `this` was just allocated by `listen()`; no `&mut` outstanding. + let this_ref = unsafe { &*this }; + let vm = this_ref.handlers.global_object.bun_vm().as_mut(); + let Some(hot) = vm.hot_map() else { return }; + let entry = bun_jsc::rare_data::HotMapEntry { + tag: HOT_MAP_TAG_LISTENER, + ptr: this.cast::<()>(), + }; + if hot.insert_raw(&hot_id, entry) { + this_ref.hot_id.set(hot_id); + } + } + + fn unregister_for_hot_reload(this: &Self) { + let hot_id = this.hot_id.with_mut(core::mem::take); + if hot_id.is_empty() { + return; + } + let vm = this.handlers.global_object.bun_vm().as_mut(); + if let Some(hot) = vm.hot_map() { + hot.remove(&hot_id); + } + } + /// Match Node.js/libuv: unlink the unix socket file before closing the listening fd. /// Unlinking after close would race with another process creating a socket at the same path. fn unlink_unix_socket_path(this: &Self) { @@ -947,6 +1062,7 @@ impl Listener { // and `close_all()` can fire JS `close` handlers that re-derive // `&Listener` — no `&mut` may span that. let this_ref = unsafe { &*this }; + Self::unregister_for_hot_reload(this_ref); this_ref.this_value.with_mut(|r| r.finalize()); this_ref.strong_data.with_mut(|s| s.deinit()); this_ref.poll_ref.with_mut(|p| p.unref(bun_io::js_vm_ctx())); @@ -1721,6 +1837,26 @@ pub(crate) fn js_add_server_name(global: &JSGlobalObject, frame: &CallFrame) -> Err(global.throw(format_args!("Expected a Listener instance"))) } +/// Keyed on the *requested* address so `port: 0` is stable across reloads; prefix keeps it disjoint from `ServerConfig::compute_id`. +fn compute_hot_id(hostname_or_unix: &[u8], port: Option, ssl: bool) -> Box<[u8]> { + use std::io::Write as _; + // fd-based listeners have no address to key on. + if hostname_or_unix.is_empty() && port.is_none() { + return Box::default(); + } + let mut buf: Vec = Vec::with_capacity(hostname_or_unix.len() + 24); + let _ = buf.write_all(if ssl { b"[tls]-" } else { b"[tcp]-" }); + match port { + Some(p) => { + let _ = write!(&mut buf, "tcp:{}:{}", bstr::BStr::new(hostname_or_unix), p); + } + None => { + let _ = write!(&mut buf, "unix:{}", bstr::BStr::new(hostname_or_unix)); + } + } + buf.into_boxed_slice() +} + #[cfg(windows)] fn is_valid_pipe_name(pipe_name: &[u8]) -> bool { // check for valid pipe names diff --git a/test/cli/hot/hot.test.ts b/test/cli/hot/hot.test.ts index 8ab6f31dd9e6..7db5351eea65 100644 --- a/test/cli/hot/hot.test.ts +++ b/test/cli/hot/hot.test.ts @@ -1,7 +1,7 @@ import { spawn } from "bun"; -import { beforeEach, expect, it } from "bun:test"; +import { beforeEach, describe, expect, it } from "bun:test"; import { copyFileSync, cpSync, readFileSync, renameSync, rmSync, unlinkSync, writeFileSync } from "fs"; -import { bunEnv, bunExe, isDebug, isWindows, tmpdirSync, waitForFileToExist } from "harness"; +import { bunEnv, bunExe, isDebug, isWindows, tempDir, tmpdirSync, waitForFileToExist } from "harness"; import { join } from "path"; const timeout = isDebug ? Infinity : 10_000; @@ -776,3 +776,159 @@ ${Buffer.alloc(counter * 2, " ").toString()}throw new Error(${counter});`, }, longTimeout, ); + +// https://github.com/oven-sh/bun/issues/26036 +// Under --hot, re-evaluating the entry module re-runs Bun.listen()/Bun.serve() +// with the same address. The previous listener must be reused (handlers +// swapped in place) rather than re-binding, which would fail EADDRINUSE. +describe("should reuse the listening socket on hot reload", () => { + for (const [name, listen, extract] of [ + [ + "Bun.listen", + `const server = Bun.listen({ + hostname: "127.0.0.1", + port: 0, + socket: { + open(s) { s.write("v" + globalThis.reloadCount + "\\n"); s.flush(); }, + data() {}, + }, + });`, + async (port: number) => { + const { promise, resolve, reject } = Promise.withResolvers(); + let text = ""; + const sock = await Bun.connect({ + hostname: "127.0.0.1", + port, + socket: { + data(s, data) { + // TCP is a byte stream; accumulate until the newline + // terminator instead of assuming a single-chunk delivery. + text += Buffer.from(data).toString(); + const nl = text.indexOf("\n"); + if (nl === -1) return; + resolve(text.slice(0, nl)); + s.end(); + }, + close: () => reject(new Error("socket closed before a full line arrived")), + error: (_s, e) => reject(e), + connectError: (_s, e) => reject(e), + }, + }); + const result = await promise; + sock.end(); + return result; + }, + ], + [ + "Bun.serve", + `const server = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch() { return new Response("v" + globalThis.reloadCount); }, + });`, + async (port: number) => { + const res = await fetch(`http://127.0.0.1:${port}/`); + return await res.text(); + }, + ], + ] as const) { + it( + name, + async () => { + // The hot-reload registry keys on the *requested* address, so + // `port: 0` matches itself across reloads and the child keeps the + // same resolved port. We read that port back from the first event + // rather than reserving one in the parent (which would be a TOCTOU + // race with other processes on the CI box). + const source = (n: number) => ` +globalThis.reloadCount = ${n}; +${listen} +console.log(JSON.stringify({ listening: true, port: server.port, reload: globalThis.reloadCount })); +`; + using dir = tempDir("hot-listen-reuse", { + "index.ts": source(1), + }); + const entry = join(String(dir), "index.ts"); + + await using runner = spawn({ + cmd: [bunExe(), "--hot", "run", entry], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + }); + + let stderr = ""; + const stderrDone = (async () => { + for await (const chunk of runner.stderr) { + stderr += new TextDecoder().decode(chunk); + // Without the fix the second evaluation fails EADDRINUSE and + // stdout never produces another event; bail instead of hanging + // until the timeout. + if (stderr.includes("EADDRINUSE") || stderr.includes("Failed to")) { + runner.kill(); + } + } + })().catch(() => {}); + + const events: Array<{ listening: boolean; port: number; reload: number }> = []; + const responses: string[] = []; + let buf = ""; + const target = 3; + let port = 0; + + try { + for await (const chunk of runner.stdout) { + buf += new TextDecoder().decode(chunk); + const lines = buf.split("\n"); + buf = lines.pop() ?? ""; + let advanced = false; + for (const line of lines) { + if (!line.startsWith("{")) continue; + const ev = JSON.parse(line); + // File watchers can fire more than once for a single write + // (truncate+write); ignore repeats of the current generation + // so the strict-sequence assertions below aren't at the mercy + // of platform watcher coalescing. + if (events.length > 0 && events[events.length - 1].reload === ev.reload) continue; + events.push(ev); + port ||= ev.port; + advanced = true; + } + if (!advanced) continue; + if (events.length >= target) { + responses.push(await extract(port)); + runner.kill(); + break; + } + // Verify the new handlers are actually wired up (not just that + // listen() didn't throw), then trigger the next reload. + responses.push(await extract(port)); + writeFileSync(entry, source(events.length + 1)); + } + } catch (e) { + // runner.kill() from the stderr reader aborts this iterator; only + // swallow that — let real errors from JSON.parse / extract / + // writeFileSync surface directly. + if (!runner.killed) throw e; + } + + runner.kill(); + await runner.exited; + await stderrDone; + + expect(stderr).not.toContain("EADDRINUSE"); + expect(stderr).not.toContain("Failed to listen"); + expect(stderr).not.toContain("Failed to start server"); + expect(events).toEqual([ + { listening: true, port, reload: 1 }, + { listening: true, port, reload: 2 }, + { listening: true, port, reload: 3 }, + ]); + expect(responses).toEqual(["v1", "v2", "v3"]); + }, + timeout, + ); + } +});