diff --git a/packages/bun-uws/src/HttpContextData.h b/packages/bun-uws/src/HttpContextData.h index 0abbd62c0689..18a6153f64b8 100644 --- a/packages/bun-uws/src/HttpContextData.h +++ b/packages/bun-uws/src/HttpContextData.h @@ -85,7 +85,10 @@ struct alignas(16) HttpContextData { void clearRoutes() { this->router = HttpRouter{}; this->currentRouter = &router; - filterHandlers.clear(); + /* filterHandlers are connection-level (open/close) callbacks, not routes; + * Bun's only consumer (the node:http 'connection' thunk) is registered + * once for the server's lifetime and left in place here like + * onSocketClosed / onClientError above. */ } public: diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index 282248b5675b..565c2bd66b8d 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -1187,8 +1187,8 @@ function httpAllowHalfOpenGet(this: Server) { // Node reads `server.httpAllowHalfOpen` when the peer's FIN arrives (socketOnEnd), so // assigning it after listen() has to reach the native listener too. Push the flags -// alone: setServerCustomOptions() would also re-register the connection filter, which -// appends rather than replaces and can reallocate the vector uWS is iterating. +// alone: setServerCustomOptions() would also re-bind the clientError/connection +// callbacks, which a flag-only update has no reason to touch. function httpAllowHalfOpenSet(this: Server, value) { const previous = !!this[kHttpAllowHalfOpen]; this[kHttpAllowHalfOpen] = value; diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 3111061e1be4..10f17abcde83 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -3710,6 +3710,15 @@ pub(super) fn server_set_on_connection_( // SAFETY: as_ returned a non-null *mut to a live server. let this = unsafe { &mut *this }; if let Some(app) = this.app { + // The uWS filter is registered once per native server. + // Under `bun --hot` a node:http module re-eval reaches + // this point again on the same server (hot_map returned + // the existing one) and `filter()` appends, so without + // this gate `filterHandlers` would grow by one per + // reload. The thunk reads `self.on_connection` at call + // time, so updating the slot is enough on subsequent + // calls. + let first = this.on_connection.is_empty(); this.on_connection = callback; super::wrap_handler_slot( &mut this.on_connection, @@ -3717,25 +3726,29 @@ pub(super) fn server_set_on_connection_( global, <$T>::js_gc_on_connection_set, ); - // uws filters fire with `1` when an HTTP connection is opened - // (for TLS, when its handshake completes) and `-1` on close; - // only the open notification is forwarded to JS. - extern "C" fn thunk( - socket: *mut uws_sys::us_socket_t, - opened: i32, - user_data: *mut c_void, - ) { - if opened != 1 { - return; + if first { + // uws filters fire with `1` when an HTTP connection is + // opened (for TLS, when its handshake completes) and + // `-1` on close; only the open notification is + // forwarded to JS. + extern "C" fn thunk( + socket: *mut uws_sys::us_socket_t, + opened: i32, + user_data: *mut c_void, + ) { + if opened != 1 { + return; + } + // SAFETY: user_data is the `*mut Self` registered + // below; socket is a live uWS socket for this + // server's group. + let this = unsafe { &mut *user_data.cast::<$T>() }; + this.on_connection_callback(socket.cast::()); } - // SAFETY: user_data is the `*mut Self` registered below; - // socket is a live uWS socket for this server's group. - let this = unsafe { &mut *user_data.cast::<$T>() }; - this.on_connection_callback(socket.cast::()); + // S008: `NewApp` is a ZST opaque — safe `*mut → &mut` deref. + bun_opaque::opaque_deref_mut(app) + .filter(thunk, core::ptr::from_mut::<$T>(this).cast::()); } - // S008: `NewApp` is a ZST opaque — safe `*mut → &mut` deref. - bun_opaque::opaque_deref_mut(app) - .filter(thunk, core::ptr::from_mut::<$T>(this).cast::()); } return Ok(JSValue::UNDEFINED); } diff --git a/test/js/bun/http/serve.test.ts b/test/js/bun/http/serve.test.ts index d6534b96eae8..fc44b67bd98e 100644 --- a/test/js/bun/http/serve.test.ts +++ b/test/js/bun/http/serve.test.ts @@ -20,7 +20,9 @@ import { join, resolve } from "path"; // import app_jsx from "./app.jsx"; import { heapStats } from "bun:jsc"; import { spawn } from "child_process"; -import net from "node:net"; +import { once } from "node:events"; +import { createServer as createHttpServer } from "node:http"; +import net, { type AddressInfo } from "node:net"; import { networkInterfaces } from "node:os"; import nodeTls from "node:tls"; import { tmpdir } from "os"; @@ -1268,6 +1270,91 @@ it("reload() of a node:http-backed server is not treated as a mode switch", asyn expect(exitCode).toBe(0); }); +// reload() clears the uWS route table; the connection-open filter that backs +// node:http's 'connection' event is not a route and must survive that. `bun --hot` +// on a node:http app takes this reload path on every file change. +it("reload() of a node:http-backed server keeps the 'connection' event firing", async () => { + let connections = 0; + const httpServer = createHttpServer((req, res) => res.end("ok")); + httpServer.on("connection", () => connections++); + httpServer.listen(0, "127.0.0.1"); + await once(httpServer, "listening"); + try { + const { port } = httpServer.address() as AddressInfo; + const hit = async () => { + const socket = net.connect(port, "127.0.0.1"); + socket.on("error", () => {}); + await once(socket, "connect"); + socket.write("GET / HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); + // Drain the response so 'end' (and then 'close') is delivered. + socket.resume(); + await once(socket, "close"); + }; + + await hit(); + expect(connections).toBe(1); + + const native = (httpServer as any)[Symbol.for("::bunternal::")] as Server; + native.reload({ fetch: () => new Response("x") }); + + await hit(); + expect(connections).toBe(2); + } finally { + httpServer.closeAllConnections(); + httpServer.close(); + } +}); + +// Under --hot the module re-evaluates and listen() runs again against the same +// native server from the hot map, re-invoking Server__setOnConnection. The JS +// onServerConnection callback dedupes on socketHandle.duplex, so this asserts +// the user-visible contract (exactly one 'connection' per accept) holds across +// hot reloads, not the native filter-vector size. +it("--hot reload of a node:http server fires 'connection' once per socket", async () => { + using dir = tempDir("hot-node-http-connection", { + "server.mjs": ` + import { createServer } from "node:http"; + import { once } from "node:events"; + import { connect } from "node:net"; + import { writeFileSync, readFileSync } from "node:fs"; + + globalThis.__reloads ??= 0; + const n = globalThis.__reloads++; + + let fired = 0; + const server = createServer((req, res) => res.end("ok")); + server.on("connection", () => fired++); + await once(server.listen(0, "127.0.0.1"), "listening"); + + const s = connect(server.address().port, "127.0.0.1"); + s.on("error", () => {}); + await once(s, "connect"); + s.write("GET / HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\n\\r\\n"); + s.resume(); + await once(s, "close"); + + console.log("[reload " + n + "] " + fired); + if (n < 3) { + const self = new URL(import.meta.url); + writeFileSync(self, readFileSync(self, "utf8")); + } else { + process.exit(fired === 1 ? 0 : 1); + } + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "--hot", "server.mjs"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr.replaceAll(/^DEBUG:.*\n/gm, "")).toBe(""); + expect(stdout).toBe("[reload 0] 1\n[reload 1] 1\n[reload 2] 1\n[reload 3] 1\n"); + expect(exitCode).toBe(0); +}, 30_000); + it("reload() cannot turn a Bun.serve server into a node:http server", async () => { // The server's kind is fixed when listen() sizes its connections' native // per-socket block; a reload that smuggles in the node:http handler used by