Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/bun-uws/src/HttpContextData.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,10 @@ struct alignas(16) HttpContextData {
void clearRoutes() {
this->router = HttpRouter<RouterData>{};
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. */
}
Comment thread
robobun marked this conversation as resolved.

public:
Expand Down
4 changes: 2 additions & 2 deletions src/js/node/_http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
47 changes: 30 additions & 17 deletions src/runtime/server/server_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3710,32 +3710,45 @@ 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,
server,
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::<c_void>());
}
// 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::<c_void>());
// S008: `NewApp<SSL>` is a ZST opaque — safe `*mut → &mut` deref.
bun_opaque::opaque_deref_mut(app)
.filter(thunk, core::ptr::from_mut::<$T>(this).cast::<c_void>());
}
// S008: `NewApp<SSL>` is a ZST opaque — safe `*mut → &mut` deref.
bun_opaque::opaque_deref_mut(app)
.filter(thunk, core::ptr::from_mut::<$T>(this).cast::<c_void>());
}
return Ok(JSValue::UNDEFINED);
}
Expand Down
89 changes: 88 additions & 1 deletion test/js/bun/http/serve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 () => {
Comment thread
robobun marked this conversation as resolved.
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
Expand Down
Loading