From 76afb97d389c4e559cf55c5f441863c3ddcc9463 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 14 Oct 2026 14:32:20 +0000 Subject: [PATCH 1/8] node:http: queue the server 'close' check from the connection's close handler, not from a socket 'close' listener --- src/js/node/_http_server.ts | 61 ++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index 6c66f7904a12..2e8e83dba038 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -138,6 +138,10 @@ function emitCloseServer(self: Server) { if (!relistened) self[kListenerGeneration] = undefined; self.emit("close"); } +// Node emits the 'error' of a plaintext http socket before the server's 'close' tick (net.js Socket.prototype._destroy). Bun emits that 'error' with the socket's 'close' in one later tick, so the check goes behind it. +function emitCloseServerAfterSocketClose(self: Server) { + process.nextTick(emitCloseServer, self); +} function markListenerGenerationClosed(self: Server, generation) { generation.nativeClosed = true; emitCloseServer(self); @@ -1693,6 +1697,8 @@ function getNodeHTTPServerSocket() { #closingHandle; #resetSupported; #closeError: Error | undefined = undefined; + // The server that accepted the connection, like Node's socket._server. The public `server` is rewritten by each http.Server that gets the socket as a 'connection'. + #owner: Server; declare encrypted: boolean; declare resetAndClosing: boolean; declare _writableState: { emitClose: boolean; decodeStrings: boolean }; @@ -1715,6 +1721,7 @@ function getNodeHTTPServerSocket() { this._readableState.emitClose = true; this._writableState.decodeStrings = true; this.server = server; + this.#owner = server; this.#resetSupported = !encrypted && !listenerGeneration?.isUnix; this[kHandle] = handle; this._secureEstablished = !!handle?.secureEstablished; @@ -1824,32 +1831,38 @@ function getNodeHTTPServerSocket() { // Once: a queued response's abort can destroy the socket, and run this, before the native close calls it. if (this.#closeHandled) return; this.#closeHandled = true; + // Like Node's Socket.prototype._destroy: the count goes down before any code of the program runs, and the server's + // 'close' check is queued from this frame, not from a listener on the socket. + const server = this.#owner; + const tracked = server[kTrackedConnections]; + tracked?.delete(this); const errored = this.errored; - // freeParser equivalent: runs before 'close' listeners so they observe the - // released parser (free() invoked, kOnTimeout nulled). - releaseServerParserShim(this); - this[kHandle] = null; - if (closedHandle) { - // Peer FIN: 'end' before 'close', like Node's net.Socket. read(0) emits it when nothing reads the socket. - if (closedHandle.peerEnded) { - this.push(null); - this.read(0); - } - // With no 'error' listener (a socket handed to 'upgrade') the read error would be an uncaught exception. - const closeError = this.listenerCount("error") > 0 ? closedHandle.closeError : undefined; - if (closeError) { - // Node's errnoException(nread, 'read'): "read ECONNRESET". - const er = new ErrnoException(closeError.errno, "read"); - this.#closeError = er.code === closeError.code ? er : closeError; + try { + // freeParser equivalent: runs before 'close' listeners so they observe the + // released parser (free() invoked, kOnTimeout nulled). + releaseServerParserShim(this); + this[kHandle] = null; + if (closedHandle) { + // Peer FIN: 'end' before 'close', like Node's net.Socket. read(0) emits it when nothing reads the socket. + if (closedHandle.peerEnded) { + this.push(null); + this.read(0); + } + // With no 'error' listener (a socket handed to 'upgrade') the read error would be an uncaught exception. + const closeError = this.listenerCount("error") > 0 ? closedHandle.closeError : undefined; + if (closeError) { + // Node's errnoException(nread, 'read'): "read ECONNRESET". + const er = new ErrnoException(closeError.errno, "read"); + this.#closeError = er.code === closeError.code ? er : closeError; + } } - } - const server = this.server; - const tracked = server?.[kTrackedConnections]; - if (tracked) { - tracked.delete(this); - if (tracked.size === 0 && server[kPendingDrainClose]) { - // The server 'close' follows this socket's own 'close' event. - this.once("close", () => process.nextTick(emitCloseServer, server)); + } finally { + // Behind the ticks of the peer's 'end' above, and also when a 'readable' listener throws from read(0). + if (tracked && tracked.size === 0 && server[kPendingDrainClose]) { + process.nextTick( + (errored || this.#closeError) && !this.encrypted ? emitCloseServerAfterSocketClose : emitCloseServer, + server, + ); } } const timer = this[kSocketTimeoutTimer]; From 84f618f175327030beaf2417c1df13b1ed92e8c8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 14 Oct 2026 14:42:27 +0000 Subject: [PATCH 2/8] test: rows for the last connection of a pending server.close(cb); the keep-alive test asserts Node's order --- .../http/node-http-server-close-drain.test.ts | 229 ++++++++++++++++++ test/js/node/http/node-http.test.ts | 14 +- 2 files changed, 239 insertions(+), 4 deletions(-) diff --git a/test/js/node/http/node-http-server-close-drain.test.ts b/test/js/node/http/node-http-server-close-drain.test.ts index 1ddba31ce6ce..91305346ead4 100644 --- a/test/js/node/http/node-http-server-close-drain.test.ts +++ b/test/js/node/http/node-http-server-close-drain.test.ts @@ -1011,3 +1011,232 @@ test("no 'close' is emitted on a re-listened server when an earlier connection e server.closeAllConnections(); } }); + +// server.close(cb) waits for the last connection. Node counts a connection down when its socket is destroyed and +// queues the server's 'close' from there (Socket.prototype._destroy), so nothing the program does with the socket's +// events can hold the server's 'close' back: https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L912-L918 +// In each row the program does something to the last connection. No step waits for a timer: a step follows an +// event or a round trip. Each expected line is what Node v26.3.0 prints for the same script. +function lastConnectionFixture(protocol: "http" | "https", row: string) { + return ` + const http = require("node:http"), https = require("node:https"), net = require("node:net"), tls = require("node:tls"); + const events = []; + process.on("uncaughtException", err => events.push("uncaught " + err.message)); + process.on("exit", () => console.log(events.join(", "))); + const boom = () => { + throw new Error("boom"); + }; + const secure = ${protocol === "https"}; + const server = secure ? https.createServer(${JSON.stringify(tlsCert)}) : http.createServer(); + server.on("close", () => events.push("server close")); + const close = () => server.close(() => events.push("close callback")); + const get = "GET / HTTP/1.1\\r\\nHost: x\\r\\n\\r\\n"; + const upgrade = "GET / HTTP/1.1\\r\\nHost: x\\r\\nConnection: Upgrade\\r\\nUpgrade: x\\r\\n\\r\\n"; + const switching = "HTTP/1.1 101 Switching Protocols\\r\\nConnection: Upgrade\\r\\nUpgrade: x\\r\\n\\r\\n"; + server.listen(0, "127.0.0.1", () => { + const { port } = server.address(); + const open = request => { + const socket = secure + ? tls.connect({ port, host: "127.0.0.1", rejectUnauthorized: false }, () => socket.write(request)) + : net.connect(port, "127.0.0.1", () => socket.write(request)); + socket.on("error", () => {}); + return socket; + }; + ${row} + });`; +} + +// The response is destroyed one round trip after close(), from outside the parse of its connection. +const responseCloseThrows = ` + server.on("request", (req, res) => { + res.on("close", () => { + events.push("response close"); + boom(); + }); + close(); + res.write("x"); + client.once("data", () => res.destroy()); + }); + const client = open(get);`; + +const destroyWithError = ` + server.on("clientError", () => events.push("clientError")); + server.on("request", (req, res) => { + close(); + res.write("x"); + client.once("data", () => req.socket.destroy(new Error("destroyed"))); + }); + const client = open(get);`; + +test.concurrent.each([ + [ + "a response 'close' listener throws", + "http", + responseCloseThrows, + "server close, close callback, response close, uncaught boom", + ], + [ + "a response 'close' listener throws (https)", + "https", + responseCloseThrows, + "server close, close callback, response close, uncaught boom", + ], + [ + "a socket 'close' listener throws", + "http", + `server.on("request", (req, res) => { + req.socket.on("close", boom); + close(); + res.write("x"); + client.once("data", () => res.destroy()); + }); + const client = open(get);`, + "server close, close callback, uncaught boom", + ], + [ + "the 'close' listener of an upgraded socket throws", + "http", + `server.on("upgrade", (req, socket) => { + socket.on("close", boom); + close(); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + const client = open(upgrade);`, + "server close, close callback, uncaught boom", + ], + [ + "the 'close' listener of a socket that sent part of a request head throws", + "http", + `server.once("connection", socket => socket.on("close", boom)); + server.on("request", (req, res) => res.end("ok")); + const partial = open("G"); + partial.on("connect", () => { + // The byte was written first, so the server has read it when it answers this. + const barrier = open("GET / HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\n\\r\\n"); + barrier.resume(); + barrier.on("close", () => { + close(); + partial.destroy(); + }); + });`, + "server close, close callback, uncaught boom", + ], + [ + "server[Symbol.asyncDispose]() waits and a socket 'close' listener throws", + "http", + `server.on("upgrade", (req, socket) => { + socket.on("close", boom); + server[Symbol.asyncDispose]().then(() => events.push("disposed")); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + const client = open(upgrade);`, + "server close, disposed, uncaught boom", + ], + [ + "an 'aborted' listener removes the socket's 'close' listeners", + "http", + `server.on("request", (req, res) => { + req.on("error", () => {}); + req.on("aborted", () => req.socket.removeAllListeners("close")); + close(); + res.write("x"); + client.once("data", () => res.destroy()); + }); + const client = open(get);`, + "server close, close callback", + ], + [ + "the program's socket.emit swallows 'close'", + "http", + `server.on("upgrade", (req, socket) => { + const emit = socket.emit; + socket.emit = function (name) { + if (name === "close") return false; + return emit.apply(this, arguments); + }; + close(); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + const client = open(upgrade);`, + "server close, close callback", + ], + [ + "the 'error' listener of a socket destroyed with an error throws", + "http", + `server.on("upgrade", (req, socket) => { + socket.on("error", boom); + close(); + socket.write(switching); + client.once("data", () => socket.destroy(new Error("destroyed"))); + }); + const client = open(upgrade);`, + "uncaught boom, server close, close callback", + ], + [ + "a tunnel that is idle after the peer's FIN is destroyed", + "http", + `server.on("upgrade", (req, socket) => { + socket.on("end", () => { + close(); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + socket.resume(); + client.end(); + }); + const client = open(upgrade);`, + "server close, close callback", + ], + [ + "a socket 'readable' listener throws at the peer's FIN", + "http", + `server.on("request", (req, res) => { + const socket = req.socket; + socket.on("readable", () => { + if (socket.read() === null) boom(); + }); + close(); + res.end("ok"); + client.once("data", () => client.end()); + client.on("close", () => server.getConnections((err, count) => events.push("connections " + count))); + }); + const client = open(get);`, + "uncaught boom, server close, close callback, connections 0", + ], + [ + "the tunnel socket was re-emitted on a second server", + "http", + `const other = http.createServer(); + server.on("upgrade", (req, socket) => { + socket.on("error", () => {}); + socket.write(switching); + other.emit("connection", socket); + close(); + client.once("data", () => socket.destroy()); + client.on("close", () => server.getConnections((err, count) => events.push("connections " + count))); + }); + const client = open(upgrade);`, + "server close, close callback, connections 0", + ], + // Node emits the socket's 'error' before the server's 'close' on http, and after it on https. + ["the socket is destroyed with an error", "http", destroyWithError, "clientError, server close, close callback"], + [ + "the socket is destroyed with an error (https)", + "https", + destroyWithError, + "server close, close callback, clientError", + ], +] as const)("server.close(cb) calls back when %s", async (_name, protocol, row, expected) => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", lastConnectionFixture(protocol, row)], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr }).toEqual({ stdout: expected + "\n", stderr: "" }); + expect(exitCode).toBe(0); +}); diff --git a/test/js/node/http/node-http.test.ts b/test/js/node/http/node-http.test.ts index 26142da2ae36..ec5f667f596a 100644 --- a/test/js/node/http/node-http.test.ts +++ b/test/js/node/http/node-http.test.ts @@ -5492,7 +5492,13 @@ describe("HTTP server transport shutdown", () => { await release.promise; res.end("done"); }); - server.on("connection", socket => socket.once("close", () => events.push("socket close"))); + const socketClosed = Promise.withResolvers(); + server.on("connection", socket => + socket.once("close", () => { + events.push("socket close"); + socketClosed.resolve(); + }), + ); server.listen(0, "127.0.0.1"); await once(server, "listening"); const { port } = server.address() as AddressInfo; @@ -5533,9 +5539,9 @@ describe("HTTP server transport shutdown", () => { client.destroy(); expect(await closed.promise).toBeUndefined(); - await clientClosed.promise; - expect(events[0]).toBe("socket close"); - expect(events.indexOf("socket close")).toBeLessThan(events.indexOf("close callback")); + await Promise.all([clientClosed.promise, socketClosed.promise]); + // The order Node v26.3.0 prints: net.Server queues its 'close' when the last socket is destroyed, ahead of that socket's own 'close'. + expect(events).toEqual(["close callback", "socket close"]); expect(wire).toContain("Content-Length: 4\r\n"); expect(wire).toEndWith("\r\n\r\ndone"); } finally { From 94becd8358cd5d38d7af3790fa0f2615d81d35bb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 14 Oct 2026 15:24:20 +0000 Subject: [PATCH 3/8] test: two rows compare in any order, the error rows destroy the response --- .../http/node-http-server-close-drain.test.ts | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/test/js/node/http/node-http-server-close-drain.test.ts b/test/js/node/http/node-http-server-close-drain.test.ts index 91305346ead4..45280e335cd0 100644 --- a/test/js/node/http/node-http-server-close-drain.test.ts +++ b/test/js/node/http/node-http-server-close-drain.test.ts @@ -1016,7 +1016,9 @@ test("no 'close' is emitted on a re-listened server when an earlier connection e // queues the server's 'close' from there (Socket.prototype._destroy), so nothing the program does with the socket's // events can hold the server's 'close' back: https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L912-L918 // In each row the program does something to the last connection. No step waits for a timer: a step follows an -// event or a round trip. Each expected line is what Node v26.3.0 prints for the same script. +// event or a round trip. Each expected line is what Node v26.3.0 prints for the same script. An array is that +// line in any order: Node emits a socket's 'close' in a later turn of the loop than its 'error' and than a +// settled promise, Bun in the same one, so only Node puts the server's 'close' in between. function lastConnectionFixture(protocol: "http" | "https", row: string) { return ` const http = require("node:http"), https = require("node:https"), net = require("node:net"), tls = require("node:tls"); @@ -1064,7 +1066,7 @@ const destroyWithError = ` server.on("request", (req, res) => { close(); res.write("x"); - client.once("data", () => req.socket.destroy(new Error("destroyed"))); + client.once("data", () => res.destroy(new Error("destroyed"))); }); const client = open(get);`; @@ -1120,7 +1122,7 @@ test.concurrent.each([ partial.destroy(); }); });`, - "server close, close callback, uncaught boom", + ["server close", "close callback", "uncaught boom"], ], [ "server[Symbol.asyncDispose]() waits and a socket 'close' listener throws", @@ -1132,7 +1134,7 @@ test.concurrent.each([ client.once("data", () => socket.destroy()); }); const client = open(upgrade);`, - "server close, disposed, uncaught boom", + ["server close", "disposed", "uncaught boom"], ], [ "an 'aborted' listener removes the socket's 'close' listeners", @@ -1222,9 +1224,9 @@ test.concurrent.each([ "server close, close callback, connections 0", ], // Node emits the socket's 'error' before the server's 'close' on http, and after it on https. - ["the socket is destroyed with an error", "http", destroyWithError, "clientError, server close, close callback"], + ["the response is destroyed with an error", "http", destroyWithError, "clientError, server close, close callback"], [ - "the socket is destroyed with an error (https)", + "the response is destroyed with an error (https)", "https", destroyWithError, "server close, close callback, clientError", @@ -1237,6 +1239,10 @@ test.concurrent.each([ stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect({ stdout, stderr }).toEqual({ stdout: expected + "\n", stderr: "" }); + if (typeof expected === "string") { + expect({ stdout, stderr }).toEqual({ stdout: expected + "\n", stderr: "" }); + } else { + expect({ events: stdout.trim().split(", ").sort(), stderr }).toEqual({ events: [...expected].sort(), stderr: "" }); + } expect(exitCode).toBe(0); }); From 79f4ce6dd3cd12fb02761db5599c5482c35c83d5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 14 Oct 2026 15:50:27 +0000 Subject: [PATCH 4/8] test: one fixture process for the last-connection rows --- .../http/node-http-server-close-drain.test.ts | 265 +++--------------- ...tp-server-close-last-connection-fixture.js | 212 ++++++++++++++ 2 files changed, 253 insertions(+), 224 deletions(-) create mode 100644 test/js/node/http/node-http-server-close-last-connection-fixture.js diff --git a/test/js/node/http/node-http-server-close-drain.test.ts b/test/js/node/http/node-http-server-close-drain.test.ts index 45280e335cd0..0b507b84a3ad 100644 --- a/test/js/node/http/node-http-server-close-drain.test.ts +++ b/test/js/node/http/node-http-server-close-drain.test.ts @@ -5,6 +5,7 @@ import { createServer } from "node:http"; import { Agent as HttpsAgent, createServer as createHttpsServer, get as httpsGet } from "node:https"; import type { AddressInfo } from "node:net"; import { connect, createServer as createNetServer } from "node:net"; +import path from "node:path"; import { connect as tlsConnect } from "node:tls"; // Node's net.Server#close callback (and the 'close' event) only fires once @@ -1015,234 +1016,50 @@ test("no 'close' is emitted on a re-listened server when an earlier connection e // server.close(cb) waits for the last connection. Node counts a connection down when its socket is destroyed and // queues the server's 'close' from there (Socket.prototype._destroy), so nothing the program does with the socket's // events can hold the server's 'close' back: https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L912-L918 -// In each row the program does something to the last connection. No step waits for a timer: a step follows an -// event or a round trip. Each expected line is what Node v26.3.0 prints for the same script. An array is that -// line in any order: Node emits a socket's 'close' in a later turn of the loop than its 'error' and than a -// settled promise, Bun in the same one, so only Node puts the server's 'close' in between. -function lastConnectionFixture(protocol: "http" | "https", row: string) { - return ` - const http = require("node:http"), https = require("node:https"), net = require("node:net"), tls = require("node:tls"); - const events = []; - process.on("uncaughtException", err => events.push("uncaught " + err.message)); - process.on("exit", () => console.log(events.join(", "))); - const boom = () => { - throw new Error("boom"); - }; - const secure = ${protocol === "https"}; - const server = secure ? https.createServer(${JSON.stringify(tlsCert)}) : http.createServer(); - server.on("close", () => events.push("server close")); - const close = () => server.close(() => events.push("close callback")); - const get = "GET / HTTP/1.1\\r\\nHost: x\\r\\n\\r\\n"; - const upgrade = "GET / HTTP/1.1\\r\\nHost: x\\r\\nConnection: Upgrade\\r\\nUpgrade: x\\r\\n\\r\\n"; - const switching = "HTTP/1.1 101 Switching Protocols\\r\\nConnection: Upgrade\\r\\nUpgrade: x\\r\\n\\r\\n"; - server.listen(0, "127.0.0.1", () => { - const { port } = server.address(); - const open = request => { - const socket = secure - ? tls.connect({ port, host: "127.0.0.1", rejectUnauthorized: false }, () => socket.write(request)) - : net.connect(port, "127.0.0.1", () => socket.write(request)); - socket.on("error", () => {}); - return socket; - }; - ${row} - });`; -} - -// The response is destroyed one round trip after close(), from outside the parse of its connection. -const responseCloseThrows = ` - server.on("request", (req, res) => { - res.on("close", () => { - events.push("response close"); - boom(); - }); - close(); - res.write("x"); - client.once("data", () => res.destroy()); - }); - const client = open(get);`; - -const destroyWithError = ` - server.on("clientError", () => events.push("clientError")); - server.on("request", (req, res) => { - close(); - res.write("x"); - client.once("data", () => res.destroy(new Error("destroyed"))); - }); - const client = open(get);`; - -test.concurrent.each([ - [ - "a response 'close' listener throws", - "http", - responseCloseThrows, - "server close, close callback, response close, uncaught boom", - ], - [ - "a response 'close' listener throws (https)", - "https", - responseCloseThrows, - "server close, close callback, response close, uncaught boom", - ], - [ - "a socket 'close' listener throws", - "http", - `server.on("request", (req, res) => { - req.socket.on("close", boom); - close(); - res.write("x"); - client.once("data", () => res.destroy()); - }); - const client = open(get);`, - "server close, close callback, uncaught boom", - ], - [ - "the 'close' listener of an upgraded socket throws", - "http", - `server.on("upgrade", (req, socket) => { - socket.on("close", boom); - close(); - socket.write(switching); - client.once("data", () => socket.destroy()); - }); - const client = open(upgrade);`, - "server close, close callback, uncaught boom", - ], - [ - "the 'close' listener of a socket that sent part of a request head throws", - "http", - `server.once("connection", socket => socket.on("close", boom)); - server.on("request", (req, res) => res.end("ok")); - const partial = open("G"); - partial.on("connect", () => { - // The byte was written first, so the server has read it when it answers this. - const barrier = open("GET / HTTP/1.1\\r\\nHost: x\\r\\nConnection: close\\r\\n\\r\\n"); - barrier.resume(); - barrier.on("close", () => { - close(); - partial.destroy(); - }); - });`, - ["server close", "close callback", "uncaught boom"], - ], - [ - "server[Symbol.asyncDispose]() waits and a socket 'close' listener throws", - "http", - `server.on("upgrade", (req, socket) => { - socket.on("close", boom); - server[Symbol.asyncDispose]().then(() => events.push("disposed")); - socket.write(switching); - client.once("data", () => socket.destroy()); - }); - const client = open(upgrade);`, - ["server close", "disposed", "uncaught boom"], - ], - [ - "an 'aborted' listener removes the socket's 'close' listeners", - "http", - `server.on("request", (req, res) => { - req.on("error", () => {}); - req.on("aborted", () => req.socket.removeAllListeners("close")); - close(); - res.write("x"); - client.once("data", () => res.destroy()); - }); - const client = open(get);`, - "server close, close callback", - ], - [ - "the program's socket.emit swallows 'close'", - "http", - `server.on("upgrade", (req, socket) => { - const emit = socket.emit; - socket.emit = function (name) { - if (name === "close") return false; - return emit.apply(this, arguments); - }; - close(); - socket.write(switching); - client.once("data", () => socket.destroy()); - }); - const client = open(upgrade);`, - "server close, close callback", - ], - [ - "the 'error' listener of a socket destroyed with an error throws", - "http", - `server.on("upgrade", (req, socket) => { - socket.on("error", boom); - close(); - socket.write(switching); - client.once("data", () => socket.destroy(new Error("destroyed"))); - }); - const client = open(upgrade);`, - "uncaught boom, server close, close callback", - ], - [ - "a tunnel that is idle after the peer's FIN is destroyed", - "http", - `server.on("upgrade", (req, socket) => { - socket.on("end", () => { - close(); - socket.write(switching); - client.once("data", () => socket.destroy()); - }); - socket.resume(); - client.end(); - }); - const client = open(upgrade);`, - "server close, close callback", - ], - [ - "a socket 'readable' listener throws at the peer's FIN", - "http", - `server.on("request", (req, res) => { - const socket = req.socket; - socket.on("readable", () => { - if (socket.read() === null) boom(); - }); - close(); - res.end("ok"); - client.once("data", () => client.end()); - client.on("close", () => server.getConnections((err, count) => events.push("connections " + count))); - }); - const client = open(get);`, - "uncaught boom, server close, close callback, connections 0", - ], - [ - "the tunnel socket was re-emitted on a second server", - "http", - `const other = http.createServer(); - server.on("upgrade", (req, socket) => { - socket.on("error", () => {}); - socket.write(switching); - other.emit("connection", socket); - close(); - client.once("data", () => socket.destroy()); - client.on("close", () => server.getConnections((err, count) => events.push("connections " + count))); - }); - const client = open(upgrade);`, - "server close, close callback, connections 0", - ], - // Node emits the socket's 'error' before the server's 'close' on http, and after it on https. - ["the response is destroyed with an error", "http", destroyWithError, "clientError, server close, close callback"], - [ - "the response is destroyed with an error (https)", - "https", - destroyWithError, - "server close, close callback, clientError", - ], -] as const)("server.close(cb) calls back when %s", async (_name, protocol, row, expected) => { +// These are the lines that Node v26.3.0 prints for the fixture. +test("server.close(cb) calls back, whatever the program does to the events of the last connection", async () => { await using proc = Bun.spawn({ - cmd: [bunExe(), "-e", lastConnectionFixture(protocol, row)], + cmd: [bunExe(), path.join(import.meta.dir, "node-http-server-close-last-connection-fixture.js")], env: bunEnv, stdout: "pipe", stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - if (typeof expected === "string") { - expect({ stdout, stderr }).toEqual({ stdout: expected + "\n", stderr: "" }); - } else { - expect({ events: stdout.trim().split(", ").sort(), stderr }).toEqual({ events: [...expected].sort(), stderr: "" }); - } + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ + "a response 'close' listener throws": ["server close", "close callback", "response close", "uncaught boom"], + "a response 'close' listener throws (https)": ["server close", "close callback", "response close", "uncaught boom"], + "a socket 'close' listener throws": ["server close", "close callback", "uncaught boom"], + "the 'close' listener of an upgraded socket throws": ["server close", "close callback", "uncaught boom"], + // Sorted by the fixture. + "the 'close' listener of a socket that sent one byte throws": ["close callback", "server close", "uncaught boom"], + "server[Symbol.asyncDispose]() waits and a socket 'close' listener throws": [ + "disposed", + "server close", + "uncaught boom", + ], + "an 'aborted' listener removes the socket's 'close' listeners": ["server close", "close callback"], + "the program's socket.emit swallows 'close'": ["server close", "close callback"], + "the 'error' listener of a socket destroyed with an error throws": [ + "uncaught boom", + "server close", + "close callback", + ], + "a tunnel that is idle after the peer's FIN is destroyed": ["server close", "close callback"], + "a socket 'readable' listener throws at the peer's FIN": [ + "uncaught boom", + "server close", + "close callback", + "connections 0", + ], + "the tunnel socket was emitted as a 'connection' of a second server": [ + "server close", + "close callback", + "connections 0", + ], + "the response is destroyed with an error": ["clientError", "server close", "close callback"], + "the response is destroyed with an error (https)": ["server close", "close callback", "clientError"], + }); expect(exitCode).toBe(0); -}); + // One debug build process for all rows: it loads node:http and node:https, which takes seconds there. +}, 30_000); diff --git a/test/js/node/http/node-http-server-close-last-connection-fixture.js b/test/js/node/http/node-http-server-close-last-connection-fixture.js new file mode 100644 index 000000000000..8002c7cbec98 --- /dev/null +++ b/test/js/node/http/node-http-server-close-last-connection-fixture.js @@ -0,0 +1,212 @@ +// Each row has its own server. server.close() waits for the last connection, and the program does something to +// that connection. No step waits for a timer: a step follows an event or a round trip. +// Prints one JSON object: the name of each row and its events in order. Node.js runs this file too. +const http = require("node:http"); +const https = require("node:https"); +const net = require("node:net"); +const tls = require("node:tls"); +const fs = require("node:fs"); +const path = require("node:path"); + +const keys = path.join(__dirname, "..", "test", "fixtures", "keys"); +const tlsOptions = { + key: fs.readFileSync(path.join(keys, "agent1-key.pem")), + cert: fs.readFileSync(path.join(keys, "agent1-cert.pem")), +}; +const get = "GET / HTTP/1.1\r\nHost: x\r\n\r\n"; +const upgrade = "GET / HTTP/1.1\r\nHost: x\r\nConnection: Upgrade\r\nUpgrade: x\r\n\r\n"; +const switching = "HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: x\r\n\r\n"; + +const results = {}; +const anyOrder = []; +process.on("uncaughtException", err => { + if (!err.events) throw err; + err.events.push("uncaught " + err.message); +}); +process.on("exit", () => { + for (const events of anyOrder) events.sort(); + console.log(JSON.stringify(results)); +}); + +function row(name, protocol, run) { + const events = (results[name] = []); + const secure = protocol === "https"; + const server = secure ? https.createServer(tlsOptions) : http.createServer(); + server.on("close", () => events.push("server close")); + server.listen(0, "127.0.0.1", () => { + const { port } = server.address(); + run({ + server, + events, + close: () => server.close(() => events.push("close callback")), + boom() { + throw Object.assign(new Error("boom"), { events }); + }, + open(request) { + const socket = secure + ? tls.connect({ port, host: "127.0.0.1", rejectUnauthorized: false }, () => socket.write(request)) + : net.connect(port, "127.0.0.1", () => socket.write(request)); + socket.on("error", () => {}); + return socket; + }, + }); + }); + return events; +} + +// The response is destroyed one round trip after close(), from outside the parse of its connection. +function responseCloseThrows({ server, events, close, boom, open }) { + server.on("request", (req, res) => { + res.on("close", () => { + events.push("response close"); + boom(); + }); + close(); + res.write("x"); + client.once("data", () => res.destroy()); + }); + const client = open(get); +} +row("a response 'close' listener throws", "http", responseCloseThrows); +row("a response 'close' listener throws (https)", "https", responseCloseThrows); + +row("a socket 'close' listener throws", "http", ({ server, close, boom, open }) => { + server.on("request", (req, res) => { + req.socket.on("close", boom); + close(); + res.write("x"); + client.once("data", () => res.destroy()); + }); + const client = open(get); +}); + +row("the 'close' listener of an upgraded socket throws", "http", ({ server, close, boom, open }) => { + server.on("upgrade", (req, socket) => { + socket.on("close", boom); + close(); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + const client = open(upgrade); +}); + +// This close carries an error (an incomplete request). Node emits the socket's 'close' in a later turn of the loop +// than its 'error', with the server's 'close' in between. Bun emits both in one tick. +anyOrder.push( + row("the 'close' listener of a socket that sent one byte throws", "http", ({ server, close, boom, open }) => { + server.once("connection", socket => socket.on("close", boom)); + server.on("request", (req, res) => res.end("ok")); + const partial = open("G"); + partial.on("connect", () => { + // The byte was written first, so the server has read it when it answers this. + const barrier = open("GET / HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); + barrier.resume(); + barrier.on("close", () => { + close(); + partial.destroy(); + }); + }); + }), +); + +// Node emits the socket's 'close' in a later turn of the loop than the server's, so the promise settles in between. +anyOrder.push( + row("server[Symbol.asyncDispose]() waits and a socket 'close' listener throws", "http", context => { + const { server, events, boom, open } = context; + server.on("upgrade", (req, socket) => { + socket.on("close", boom); + server[Symbol.asyncDispose]().then(() => events.push("disposed")); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + const client = open(upgrade); + }), +); + +row("an 'aborted' listener removes the socket's 'close' listeners", "http", ({ server, close, open }) => { + server.on("request", (req, res) => { + req.on("error", () => {}); + req.on("aborted", () => req.socket.removeAllListeners("close")); + close(); + res.write("x"); + client.once("data", () => res.destroy()); + }); + const client = open(get); +}); + +row("the program's socket.emit swallows 'close'", "http", ({ server, close, open }) => { + server.on("upgrade", (req, socket) => { + const emit = socket.emit; + socket.emit = function (name) { + if (name === "close") return false; + return emit.apply(this, arguments); + }; + close(); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + const client = open(upgrade); +}); + +row("the 'error' listener of a socket destroyed with an error throws", "http", ({ server, close, boom, open }) => { + server.on("upgrade", (req, socket) => { + socket.on("error", boom); + close(); + socket.write(switching); + client.once("data", () => socket.destroy(new Error("destroyed"))); + }); + const client = open(upgrade); +}); + +row("a tunnel that is idle after the peer's FIN is destroyed", "http", ({ server, close, open }) => { + server.on("upgrade", (req, socket) => { + socket.on("end", () => { + close(); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + socket.resume(); + client.end(); + }); + const client = open(upgrade); +}); + +row("a socket 'readable' listener throws at the peer's FIN", "http", ({ server, events, close, boom, open }) => { + server.on("request", (req, res) => { + const socket = req.socket; + socket.on("readable", () => { + if (socket.read() === null) boom(); + }); + close(); + res.end("ok"); + client.once("data", () => client.end()); + client.on("close", () => server.getConnections((err, count) => events.push("connections " + count))); + }); + const client = open(get); +}); + +row("the tunnel socket was emitted as a 'connection' of a second server", "http", ({ server, events, close, open }) => { + const other = http.createServer(); + server.on("upgrade", (req, socket) => { + socket.on("error", () => {}); + socket.write(switching); + other.emit("connection", socket); + close(); + client.once("data", () => socket.destroy()); + client.on("close", () => server.getConnections((err, count) => events.push("connections " + count))); + }); + const client = open(upgrade); +}); + +// Node emits the socket's 'error' before the server's 'close' on http, and after it on https. +function destroyWithError({ server, events, close, open }) { + server.on("clientError", () => events.push("clientError")); + server.on("request", (req, res) => { + close(); + res.write("x"); + client.once("data", () => res.destroy(new Error("destroyed"))); + }); + const client = open(get); +} +row("the response is destroyed with an error", "http", destroyWithError); +row("the response is destroyed with an error (https)", "https", destroyWithError); From 669614853ae0004be28ac997a76536fe0b1d281c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 15 Oct 2026 14:07:58 +0000 Subject: [PATCH 5/8] node:http: cut the change in place: the server's check replaces the listener, the count stays where it was --- src/js/node/_http_server.ts | 79 ++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 37 deletions(-) diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index 2e8e83dba038..d9be57a50c12 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -121,26 +121,35 @@ const DateNow = Date.now; let cluster; -function emitCloseServer(self: Server) { +// Whether the pending close() is complete. If it is, its 'close' is due and the pending state is cleared. +function takePendingClose(self: Server) { // A native close promise belongs to one listener; Node's Server 'close' spans overlapping listeners and handed-off sockets (net.js _emitCloseIfDrained). - if (!self[kPendingDrainClose]) return; + if (!self[kPendingDrainClose]) return false; const pendingGenerations = self[kPendingCloseGenerations]; let drainedAtClose = true; for (const generation of pendingGenerations) { - if (!generation.nativeClosed) return; + if (!generation.nativeClosed) return false; drainedAtClose &&= generation.drainedAtClose; } const relistened = !!self[serverSymbol]; // Node queues 'close' inside a close() that leaves no connection open. A listen() after it, or a connection to that listener, cannot hold the event back. - if ((relistened || self[kTrackedConnections].size > 0) && !drainedAtClose) return; + if ((relistened || self[kTrackedConnections].size > 0) && !drainedAtClose) return false; self[kPendingDrainClose] = false; pendingGenerations.clear(); if (!relistened) self[kListenerGeneration] = undefined; + return true; +} +function emitCloseServer(self: Server) { + if (takePendingClose(self)) self.emit("close"); +} +function emitServerClose(self: Server) { self.emit("close"); } -// Node emits the 'error' of a plaintext http socket before the server's 'close' tick (net.js Socket.prototype._destroy). Bun emits that 'error' with the socket's 'close' in one later tick, so the check goes behind it. -function emitCloseServerAfterSocketClose(self: Server) { - process.nextTick(emitCloseServer, self); +// For the last connection of a plaintext server, closed with an error. Node emits that socket's 'error' ahead of the +// server's 'close' (https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L906-L918), so the event waits one tick more. +// The decision does not wait: a listen() from a listener of that socket does not hold the 'close' back. +function emitCloseServerAfterSocketError(self: Server) { + if (takePendingClose(self)) process.nextTick(emitServerClose, self); } function markListenerGenerationClosed(self: Server, generation) { generation.nativeClosed = true; @@ -1697,8 +1706,6 @@ function getNodeHTTPServerSocket() { #closingHandle; #resetSupported; #closeError: Error | undefined = undefined; - // The server that accepted the connection, like Node's socket._server. The public `server` is rewritten by each http.Server that gets the socket as a 'connection'. - #owner: Server; declare encrypted: boolean; declare resetAndClosing: boolean; declare _writableState: { emitClose: boolean; decodeStrings: boolean }; @@ -1721,7 +1728,6 @@ function getNodeHTTPServerSocket() { this._readableState.emitClose = true; this._writableState.decodeStrings = true; this.server = server; - this.#owner = server; this.#resetSupported = !encrypted && !listenerGeneration?.isUnix; this[kHandle] = handle; this._secureEstablished = !!handle?.secureEstablished; @@ -1831,36 +1837,35 @@ function getNodeHTTPServerSocket() { // Once: a queued response's abort can destroy the socket, and run this, before the native close calls it. if (this.#closeHandled) return; this.#closeHandled = true; - // Like Node's Socket.prototype._destroy: the count goes down before any code of the program runs, and the server's - // 'close' check is queued from this frame, not from a listener on the socket. - const server = this.#owner; - const tracked = server[kTrackedConnections]; - tracked?.delete(this); const errored = this.errored; - try { - // freeParser equivalent: runs before 'close' listeners so they observe the - // released parser (free() invoked, kOnTimeout nulled). - releaseServerParserShim(this); - this[kHandle] = null; - if (closedHandle) { - // Peer FIN: 'end' before 'close', like Node's net.Socket. read(0) emits it when nothing reads the socket. - if (closedHandle.peerEnded) { - this.push(null); - this.read(0); - } - // With no 'error' listener (a socket handed to 'upgrade') the read error would be an uncaught exception. - const closeError = this.listenerCount("error") > 0 ? closedHandle.closeError : undefined; - if (closeError) { - // Node's errnoException(nread, 'read'): "read ECONNRESET". - const er = new ErrnoException(closeError.errno, "read"); - this.#closeError = er.code === closeError.code ? er : closeError; - } + // freeParser equivalent: runs before 'close' listeners so they observe the + // released parser (free() invoked, kOnTimeout nulled). + releaseServerParserShim(this); + this[kHandle] = null; + if (closedHandle) { + // Peer FIN: 'end' before 'close', like Node's net.Socket. read(0) emits it when nothing reads the socket. + if (closedHandle.peerEnded) { + this.push(null); + this.read(0); } - } finally { - // Behind the ticks of the peer's 'end' above, and also when a 'readable' listener throws from read(0). - if (tracked && tracked.size === 0 && server[kPendingDrainClose]) { + // With no 'error' listener (a socket handed to 'upgrade') the read error would be an uncaught exception. + const closeError = this.listenerCount("error") > 0 ? closedHandle.closeError : undefined; + if (closeError) { + // Node's errnoException(nread, 'read'): "read ECONNRESET". + const er = new ErrnoException(closeError.errno, "read"); + this.#closeError = er.code === closeError.code ? er : closeError; + } + } + const server = this.server; + const tracked = server?.[kTrackedConnections]; + if (tracked) { + tracked.delete(this); + if (tracked.size === 0 && server[kPendingDrainClose]) { + // Queued from this frame, like Node's Socket.prototype._destroy, not from a listener on the socket. + // Over TLS Node counts the raw socket, which TLSWrap.prototype.close destroys first, so there the + // 'error' does not go ahead: https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L684-L687 process.nextTick( - (errored || this.#closeError) && !this.encrypted ? emitCloseServerAfterSocketClose : emitCloseServer, + (errored || this.#closeError) && !this.encrypted ? emitCloseServerAfterSocketError : emitCloseServer, server, ); } From 2f3ce9eb831847ae0b87078e4d5f0c604f06eeed Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 15 Oct 2026 14:21:30 +0000 Subject: [PATCH 6/8] test: 26 rows for the last connection, pinned to what Bun prints, with Node's line where it differs --- .../http/node-http-server-close-drain.test.ts | 140 +++++++---- ...tp-server-close-last-connection-fixture.js | 229 ++++++++++++------ 2 files changed, 251 insertions(+), 118 deletions(-) diff --git a/test/js/node/http/node-http-server-close-drain.test.ts b/test/js/node/http/node-http-server-close-drain.test.ts index 0b507b84a3ad..cb8f30ab8c68 100644 --- a/test/js/node/http/node-http-server-close-drain.test.ts +++ b/test/js/node/http/node-http-server-close-drain.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isWindows, tls as tlsCert } from "harness"; +import { bunEnv, bunExe, isASAN, isCI, isDebug, isWindows, tls as tlsCert } from "harness"; import { once } from "node:events"; import { createServer } from "node:http"; import { Agent as HttpsAgent, createServer as createHttpsServer, get as httpsGet } from "node:https"; @@ -1014,52 +1014,92 @@ test("no 'close' is emitted on a re-listened server when an earlier connection e }); // server.close(cb) waits for the last connection. Node counts a connection down when its socket is destroyed and -// queues the server's 'close' from there (Socket.prototype._destroy), so nothing the program does with the socket's -// events can hold the server's 'close' back: https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L912-L918 -// These are the lines that Node v26.3.0 prints for the fixture. -test("server.close(cb) calls back, whatever the program does to the events of the last connection", async () => { - await using proc = Bun.spawn({ - cmd: [bunExe(), path.join(import.meta.dir, "node-http-server-close-last-connection-fixture.js")], - env: bunEnv, - stdout: "pipe", - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stderr).toBe(""); - expect(JSON.parse(stdout)).toEqual({ - "a response 'close' listener throws": ["server close", "close callback", "response close", "uncaught boom"], - "a response 'close' listener throws (https)": ["server close", "close callback", "response close", "uncaught boom"], - "a socket 'close' listener throws": ["server close", "close callback", "uncaught boom"], - "the 'close' listener of an upgraded socket throws": ["server close", "close callback", "uncaught boom"], - // Sorted by the fixture. - "the 'close' listener of a socket that sent one byte throws": ["close callback", "server close", "uncaught boom"], - "server[Symbol.asyncDispose]() waits and a socket 'close' listener throws": [ - "disposed", - "server close", - "uncaught boom", - ], - "an 'aborted' listener removes the socket's 'close' listeners": ["server close", "close callback"], - "the program's socket.emit swallows 'close'": ["server close", "close callback"], - "the 'error' listener of a socket destroyed with an error throws": [ - "uncaught boom", - "server close", - "close callback", - ], - "a tunnel that is idle after the peer's FIN is destroyed": ["server close", "close callback"], - "a socket 'readable' listener throws at the peer's FIN": [ - "uncaught boom", - "server close", - "close callback", - "connections 0", - ], - "the tunnel socket was emitted as a 'connection' of a second server": [ - "server close", - "close callback", - "connections 0", - ], - "the response is destroyed with an error": ["clientError", "server close", "close callback"], - "the response is destroyed with an error (https)": ["server close", "close callback", "clientError"], - }); - expect(exitCode).toBe(0); - // One debug build process for all rows: it loads node:http and node:https, which takes seconds there. -}, 30_000); +// queues the server's 'close' from there, so nothing the program does with the socket's events can hold the +// server's 'close' back: https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L906-L918 +// Node v26.3.0 prints the same line for each row, except where a comment gives its line. +test( + "server.close(cb) calls back, whatever the program does to the events of the last connection", + async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), path.join(import.meta.dir, "node-http-server-close-last-connection-fixture.js")], + // The fixture ends itself before the test can time out, so that a row that hangs shows in the rows it prints. + env: { ...bunEnv, DEADLINE_MS: isCI || isASAN || isDebug ? "60000" : "4000" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ + "a response 'close' listener throws": ["server close", "close callback", "response close", "uncaught boom"], + "a response 'close' listener throws (https)": [ + "server close", + "close callback", + "response close", + "uncaught boom", + ], + "a socket 'close' listener throws": ["server close", "close callback", "uncaught boom"], + "the 'close' listener of an upgraded socket throws": ["server close", "close callback", "uncaught boom"], + // Node: server close, close callback, uncaught boom. This close carries an error, and Node emits the socket's + // 'close' in a later turn of the loop than its 'error'. + "the 'close' listener of a socket that sent one byte throws": ["uncaught boom", "server close", "close callback"], + // Node: server close, disposed, uncaught boom. Node emits the socket's 'close' in a later turn of the loop. + "server[Symbol.asyncDispose]() waits and a socket 'close' listener throws": [ + "server close", + "uncaught boom", + "disposed", + ], + "an 'aborted' listener removes the socket's 'close' listeners": ["server close", "close callback"], + "the program's socket.emit swallows 'close'": ["server close", "close callback"], + "the 'error' listener of a socket destroyed with an error throws": [ + "uncaught boom", + "server close", + "close callback", + ], + "closeAllConnections() closes the idle last connection": ["server close", "close callback", "uncaught boom"], + "closeIdleConnections() closes the idle last connection": ["server close", "close callback", "uncaught boom"], + "the peer closes the idle last connection": ["server close", "close callback", "uncaught boom"], + // Node: server close, close callback, uncaught boom. Here the listener's own close is delivered after this + // socket's close, and the server's 'close' waits for both. + "closeIdleConnections() and then socket.destroy() close the idle last connection": [ + "uncaught boom", + "server close", + "close callback", + ], + "a socket 'end' listener removes every listener of the socket": ["server close", "close callback"], + "a socket 'error' listener removes the 'close' listeners at the peer's reset": [ + "socket error ECONNRESET", + "server close", + "close callback", + ], + "the peer ends the idle last connection": ["socket end", "server close", "close callback", "socket close"], + "the peer ends the idle last connection (https)": [ + "socket end", + "server close", + "close callback", + "socket close", + ], + // Node: server close, close callback, socket close. The sweep of close() closes this connection before the + // listener's own close is delivered. + "close() itself closes the idle last connection": ["socket close", "server close", "close callback"], + "a tunnel that is idle after the peer's FIN is destroyed": ["server close", "close callback"], + "the response is destroyed with an error": ["clientError", "server close", "close callback"], + "the response is destroyed with an error (https)": ["server close", "close callback", "clientError"], + "the peer resets a connection that sent part of a request head": [ + "clientError ECONNRESET", + "server close", + "close callback", + ], + "the peer resets a connection that sent part of a request head (https)": [ + "server close", + "close callback", + "clientError ECONNRESET", + ], + "a 'clientError' listener listens again": ["server close", "close callback", "listening again"], + "the socket's 'error' listener listens again": ["server close", "close callback", "listening again"], + "the socket's 'close' listener listens again": ["server close", "close callback", "listening again"], + }); + expect(exitCode).toBe(0); + }, + // CI has its own time for each test. A local debug or ASAN build needs several seconds to start the fixture. + (isASAN || isDebug) && !isCI ? 90_000 : undefined, +); diff --git a/test/js/node/http/node-http-server-close-last-connection-fixture.js b/test/js/node/http/node-http-server-close-last-connection-fixture.js index 8002c7cbec98..0734175a789f 100644 --- a/test/js/node/http/node-http-server-close-last-connection-fixture.js +++ b/test/js/node/http/node-http-server-close-last-connection-fixture.js @@ -1,5 +1,5 @@ -// Each row has its own server. server.close() waits for the last connection, and the program does something to -// that connection. No step waits for a timer: a step follows an event or a round trip. +// Each row has its own server. server.close() waits for the last connection of that server, and the program does +// something to that connection. A step follows an event or a round trip, never a timer. // Prints one JSON object: the name of each row and its events in order. Node.js runs this file too. const http = require("node:http"); const https = require("node:https"); @@ -18,40 +18,57 @@ const upgrade = "GET / HTTP/1.1\r\nHost: x\r\nConnection: Upgrade\r\nUpgrade: x\ const switching = "HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: x\r\n\r\n"; const results = {}; -const anyOrder = []; process.on("uncaughtException", err => { if (!err.events) throw err; err.events.push("uncaught " + err.message); }); -process.on("exit", () => { - for (const events of anyOrder) events.sort(); - console.log(JSON.stringify(results)); -}); +process.on("exit", () => console.log(JSON.stringify(results))); +// A row that never ends must not hold the test until its timeout: the rows printed so far show which one it is. +if (process.env.DEADLINE_MS) setTimeout(() => process.exit(1), +process.env.DEADLINE_MS).unref(); function row(name, protocol, run) { const events = (results[name] = []); const secure = protocol === "https"; const server = secure ? https.createServer(tlsOptions) : http.createServer(); + // No idle timer of the server closes a connection of a row. + server.keepAliveTimeout = 0; server.on("close", () => events.push("server close")); server.listen(0, "127.0.0.1", () => { const { port } = server.address(); + const open = request => { + const socket = secure + ? tls.connect({ port, host: "127.0.0.1", rejectUnauthorized: false }, () => socket.write(request)) + : net.connect(port, "127.0.0.1", () => socket.write(request)); + socket.on("error", () => {}); + return socket; + }; + // Calls then(raw socket) when the server has read a part of a request head from that connection. + const partial = (head, then) => { + const raw = net.connect(port, "127.0.0.1", () => { + const peer = secure ? tls.connect({ socket: raw, rejectUnauthorized: false }, () => send(peer)) : send(raw); + peer.on("error", () => {}); + }); + raw.on("error", () => {}); + const send = peer => { + peer.write(head); + // The bytes were written first, so the server has read them when it answers this. + const barrier = open("GET / HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); + barrier.resume(); + barrier.on("close", () => then(raw)); + return peer; + }; + }; run({ server, events, + open, + partial, close: () => server.close(() => events.push("close callback")), boom() { throw Object.assign(new Error("boom"), { events }); }, - open(request) { - const socket = secure - ? tls.connect({ port, host: "127.0.0.1", rejectUnauthorized: false }, () => socket.write(request)) - : net.connect(port, "127.0.0.1", () => socket.write(request)); - socket.on("error", () => {}); - return socket; - }, }); }); - return events; } // The response is destroyed one round trip after close(), from outside the parse of its connection. @@ -90,38 +107,25 @@ row("the 'close' listener of an upgraded socket throws", "http", ({ server, clos const client = open(upgrade); }); -// This close carries an error (an incomplete request). Node emits the socket's 'close' in a later turn of the loop -// than its 'error', with the server's 'close' in between. Bun emits both in one tick. -anyOrder.push( - row("the 'close' listener of a socket that sent one byte throws", "http", ({ server, close, boom, open }) => { - server.once("connection", socket => socket.on("close", boom)); - server.on("request", (req, res) => res.end("ok")); - const partial = open("G"); - partial.on("connect", () => { - // The byte was written first, so the server has read it when it answers this. - const barrier = open("GET / HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); - barrier.resume(); - barrier.on("close", () => { - close(); - partial.destroy(); - }); - }); - }), -); +row("the 'close' listener of a socket that sent one byte throws", "http", ({ server, close, boom, partial }) => { + server.once("connection", socket => socket.on("close", boom)); + server.on("request", (req, res) => res.end("ok")); + partial("G", socket => { + close(); + socket.destroy(); + }); +}); -// Node emits the socket's 'close' in a later turn of the loop than the server's, so the promise settles in between. -anyOrder.push( - row("server[Symbol.asyncDispose]() waits and a socket 'close' listener throws", "http", context => { - const { server, events, boom, open } = context; - server.on("upgrade", (req, socket) => { - socket.on("close", boom); - server[Symbol.asyncDispose]().then(() => events.push("disposed")); - socket.write(switching); - client.once("data", () => socket.destroy()); - }); - const client = open(upgrade); - }), -); +row("server[Symbol.asyncDispose]() waits and a socket 'close' listener throws", "http", context => { + const { server, events, boom, open } = context; + server.on("upgrade", (req, socket) => { + socket.on("close", boom); + server[Symbol.asyncDispose]().then(() => events.push("disposed")); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + const client = open(upgrade); +}); row("an 'aborted' listener removes the socket's 'close' listeners", "http", ({ server, close, open }) => { server.on("request", (req, res) => { @@ -158,47 +162,104 @@ row("the 'error' listener of a socket destroyed with an error throws", "http", ( const client = open(upgrade); }); -row("a tunnel that is idle after the peer's FIN is destroyed", "http", ({ server, close, open }) => { - server.on("upgrade", (req, socket) => { - socket.on("end", () => { +// The last connection is idle after its response. Each row closes it in another way, and its 'close' listener throws. +function idleConnection(closeIt) { + return ({ server, close, boom, open }) => { + server.on("request", (req, res) => { + req.socket.on("close", boom); close(); - socket.write(switching); - client.once("data", () => socket.destroy()); + res.end("ok"); + client.once("data", () => closeIt(server, client, req.socket)); }); - socket.resume(); - client.end(); + const client = open(get); + }; +} +row( + "closeAllConnections() closes the idle last connection", + "http", + idleConnection(server => server.closeAllConnections()), +); +row( + "closeIdleConnections() closes the idle last connection", + "http", + idleConnection(server => server.closeIdleConnections()), +); +row( + "the peer closes the idle last connection", + "http", + idleConnection((server, client) => client.end()), +); +// The native handle is closed and its close is not delivered yet when destroy() runs. +row( + "closeIdleConnections() and then socket.destroy() close the idle last connection", + "http", + idleConnection((server, client, socket) => { + server.closeIdleConnections(); + socket.destroy(); + }), +); + +row("a socket 'end' listener removes every listener of the socket", "http", ({ server, close, open }) => { + server.on("request", (req, res) => { + req.socket.on("end", () => req.socket.removeAllListeners()); + close(); + res.end("ok"); + client.once("data", () => client.end()); }); - const client = open(upgrade); + const client = open(get); }); -row("a socket 'readable' listener throws at the peer's FIN", "http", ({ server, events, close, boom, open }) => { +row("a socket 'error' listener removes the 'close' listeners at the peer's reset", "http", context => { + const { server, events, close, open } = context; server.on("request", (req, res) => { - const socket = req.socket; - socket.on("readable", () => { - if (socket.read() === null) boom(); + req.socket.on("error", err => { + events.push("socket error " + err.code); + req.socket.removeAllListeners("close"); }); close(); res.end("ok"); + client.once("data", () => client.resetAndDestroy()); + }); + const client = open(get); +}); + +// The order of the server's 'close' and the events of the last socket. +function endBeforeServerClose({ server, events, close, open }) { + server.on("request", (req, res) => { + req.socket.on("end", () => events.push("socket end")); + req.socket.on("close", () => events.push("socket close")); + close(); + res.end("ok"); client.once("data", () => client.end()); - client.on("close", () => server.getConnections((err, count) => events.push("connections " + count))); + }); + const client = open(get); +} +row("the peer ends the idle last connection", "http", endBeforeServerClose); +row("the peer ends the idle last connection (https)", "https", endBeforeServerClose); + +row("close() itself closes the idle last connection", "http", ({ server, events, close, open }) => { + server.on("request", (req, res) => { + req.socket.on("close", () => events.push("socket close")); + res.end("ok"); + client.once("data", close); }); const client = open(get); }); -row("the tunnel socket was emitted as a 'connection' of a second server", "http", ({ server, events, close, open }) => { - const other = http.createServer(); +row("a tunnel that is idle after the peer's FIN is destroyed", "http", ({ server, close, open }) => { server.on("upgrade", (req, socket) => { - socket.on("error", () => {}); - socket.write(switching); - other.emit("connection", socket); - close(); - client.once("data", () => socket.destroy()); - client.on("close", () => server.getConnections((err, count) => events.push("connections " + count))); + socket.on("end", () => { + close(); + socket.write(switching); + client.once("data", () => socket.destroy()); + }); + socket.resume(); + client.end(); }); const client = open(upgrade); }); -// Node emits the socket's 'error' before the server's 'close' on http, and after it on https. +// A close that carries an error. On http Node emits the socket's 'error' before the server's 'close', on https after it. function destroyWithError({ server, events, close, open }) { server.on("clientError", () => events.push("clientError")); server.on("request", (req, res) => { @@ -210,3 +271,35 @@ function destroyWithError({ server, events, close, open }) { } row("the response is destroyed with an error", "http", destroyWithError); row("the response is destroyed with an error (https)", "https", destroyWithError); + +function peerReset({ server, events, close, partial }) { + server.on("request", (req, res) => res.end("ok")); + server.on("clientError", (err, socket) => { + events.push("clientError " + err.code); + socket.destroy(); + }); + partial("GET / HT", socket => { + close(); + socket.resetAndDestroy(); + }); +} +row("the peer resets a connection that sent part of a request head", "http", peerReset); +row("the peer resets a connection that sent part of a request head (https)", "https", peerReset); + +// The close carries an error, and a listener that runs on the way listens again. +function listenAgain(listenFrom) { + return ({ server, events, close, open }) => { + const again = () => server.listen(0, "127.0.0.1", () => events.push("listening again") && server.unref()); + server.on("clientError", () => listenFrom === "clientError" && again()); + server.on("request", (req, res) => { + if (listenFrom !== "clientError") req.socket.on(listenFrom, again); + close(); + res.write("x"); + client.once("data", () => res.destroy(new Error("destroyed"))); + }); + const client = open(get); + }; +} +row("a 'clientError' listener listens again", "http", listenAgain("clientError")); +row("the socket's 'error' listener listens again", "http", listenAgain("error")); +row("the socket's 'close' listener listens again", "http", listenAgain("close")); From 568532aa46307aa3a3fb1697cb06cfef509c7f56 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 15 Oct 2026 14:48:22 +0000 Subject: [PATCH 7/8] node:http: one-line comments with the Node permalinks --- src/js/node/_http_server.ts | 8 ++------ .../http/node-http-server-close-drain.test.ts | 15 +++++---------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index d9be57a50c12..14a443bcfec6 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -145,9 +145,7 @@ function emitCloseServer(self: Server) { function emitServerClose(self: Server) { self.emit("close"); } -// For the last connection of a plaintext server, closed with an error. Node emits that socket's 'error' ahead of the -// server's 'close' (https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L906-L918), so the event waits one tick more. -// The decision does not wait: a listen() from a listener of that socket does not hold the 'close' back. +// Node emits a plaintext socket's 'error' ahead of the server's 'close': https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L906-L918 function emitCloseServerAfterSocketError(self: Server) { if (takePendingClose(self)) process.nextTick(emitServerClose, self); } @@ -1861,9 +1859,7 @@ function getNodeHTTPServerSocket() { if (tracked) { tracked.delete(this); if (tracked.size === 0 && server[kPendingDrainClose]) { - // Queued from this frame, like Node's Socket.prototype._destroy, not from a listener on the socket. - // Over TLS Node counts the raw socket, which TLSWrap.prototype.close destroys first, so there the - // 'error' does not go ahead: https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L684-L687 + // Not a listener on the socket. Over TLS Node destroys the counted raw socket before the 'error': https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L684-L687 process.nextTick( (errored || this.#closeError) && !this.encrypted ? emitCloseServerAfterSocketError : emitCloseServer, server, diff --git a/test/js/node/http/node-http-server-close-drain.test.ts b/test/js/node/http/node-http-server-close-drain.test.ts index cb8f30ab8c68..6da408c2ac7e 100644 --- a/test/js/node/http/node-http-server-close-drain.test.ts +++ b/test/js/node/http/node-http-server-close-drain.test.ts @@ -1013,9 +1013,7 @@ test("no 'close' is emitted on a re-listened server when an earlier connection e } }); -// server.close(cb) waits for the last connection. Node counts a connection down when its socket is destroyed and -// queues the server's 'close' from there, so nothing the program does with the socket's events can hold the -// server's 'close' back: https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L906-L918 +// Node queues the server's 'close' when the last socket is destroyed, not from that socket's events: https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L906-L918 // Node v26.3.0 prints the same line for each row, except where a comment gives its line. test( "server.close(cb) calls back, whatever the program does to the events of the last connection", @@ -1039,10 +1037,9 @@ test( ], "a socket 'close' listener throws": ["server close", "close callback", "uncaught boom"], "the 'close' listener of an upgraded socket throws": ["server close", "close callback", "uncaught boom"], - // Node: server close, close callback, uncaught boom. This close carries an error, and Node emits the socket's - // 'close' in a later turn of the loop than its 'error'. + // Node: server close, close callback, uncaught boom. Node emits this socket's 'close' a turn of the loop after its 'error'. "the 'close' listener of a socket that sent one byte throws": ["uncaught boom", "server close", "close callback"], - // Node: server close, disposed, uncaught boom. Node emits the socket's 'close' in a later turn of the loop. + // Node: server close, disposed, uncaught boom. Node emits the socket's 'close' a turn of the loop later. "server[Symbol.asyncDispose]() waits and a socket 'close' listener throws": [ "server close", "uncaught boom", @@ -1058,8 +1055,7 @@ test( "closeAllConnections() closes the idle last connection": ["server close", "close callback", "uncaught boom"], "closeIdleConnections() closes the idle last connection": ["server close", "close callback", "uncaught boom"], "the peer closes the idle last connection": ["server close", "close callback", "uncaught boom"], - // Node: server close, close callback, uncaught boom. Here the listener's own close is delivered after this - // socket's close, and the server's 'close' waits for both. + // Node: server close, close callback, uncaught boom. Here the listener's own close is delivered after this socket's. "closeIdleConnections() and then socket.destroy() close the idle last connection": [ "uncaught boom", "server close", @@ -1078,8 +1074,7 @@ test( "close callback", "socket close", ], - // Node: server close, close callback, socket close. The sweep of close() closes this connection before the - // listener's own close is delivered. + // Node: server close, close callback, socket close. Here the listener's own close is delivered after this socket's. "close() itself closes the idle last connection": ["socket close", "server close", "close callback"], "a tunnel that is idle after the peer's FIN is destroyed": ["server close", "close callback"], "the response is destroyed with an error": ["clientError", "server close", "close callback"], From 3dd502d5a4059cd154bfa198140ca05459723824 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 19 Oct 2026 14:58:38 +0000 Subject: [PATCH 8/8] test: the fixture has no deadline of its own --- test/js/node/http/node-http-server-close-drain.test.ts | 3 +-- .../http/node-http-server-close-last-connection-fixture.js | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/test/js/node/http/node-http-server-close-drain.test.ts b/test/js/node/http/node-http-server-close-drain.test.ts index 6da408c2ac7e..bfac715b650e 100644 --- a/test/js/node/http/node-http-server-close-drain.test.ts +++ b/test/js/node/http/node-http-server-close-drain.test.ts @@ -1020,8 +1020,7 @@ test( async () => { await using proc = Bun.spawn({ cmd: [bunExe(), path.join(import.meta.dir, "node-http-server-close-last-connection-fixture.js")], - // The fixture ends itself before the test can time out, so that a row that hangs shows in the rows it prints. - env: { ...bunEnv, DEADLINE_MS: isCI || isASAN || isDebug ? "60000" : "4000" }, + env: bunEnv, stdout: "pipe", stderr: "pipe", }); diff --git a/test/js/node/http/node-http-server-close-last-connection-fixture.js b/test/js/node/http/node-http-server-close-last-connection-fixture.js index 0734175a789f..d6d2f5d95f49 100644 --- a/test/js/node/http/node-http-server-close-last-connection-fixture.js +++ b/test/js/node/http/node-http-server-close-last-connection-fixture.js @@ -23,8 +23,6 @@ process.on("uncaughtException", err => { err.events.push("uncaught " + err.message); }); process.on("exit", () => console.log(JSON.stringify(results))); -// A row that never ends must not hold the test until its timeout: the rows printed so far show which one it is. -if (process.env.DEADLINE_MS) setTimeout(() => process.exit(1), +process.env.DEADLINE_MS).unref(); function row(name, protocol, run) { const events = (results[name] = []);