diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index 6a01d6883869..10d5003173fc 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -362,15 +362,16 @@ Server.prototype.unref = function () { }; Server.prototype.closeAllConnections = function () { - const server = this[serverSymbol]; - if (!server) { + // Node.js destroys every tracked connection and leaves the listen socket + // alone, so the server keeps accepting. It is also the forced half of + // close() + closeAllConnections(), which runs once the listener is gone. + const connections = this[kTrackedConnections]; + if (!connections) { return; } - this[serverSymbol] = undefined; - clearInterval(this[kConnectionsCheckingInterval]); - this.listening = false; - - server.stop(true); + for (const socket of connections) { + socket.destroy(); + } }; Server.prototype.getConnections = function (callback) { diff --git a/test/js/bun/test/parallel/test-http-server.listening-should-work.ts b/test/js/bun/test/parallel/test-http-server.listening-should-work.ts index 8f9e565558b8..872e9d1cd87e 100644 --- a/test/js/bun/test/parallel/test-http-server.listening-should-work.ts +++ b/test/js/bun/test/parallel/test-http-server.listening-should-work.ts @@ -6,5 +6,9 @@ const { expect } = createTest(import.meta.path); const server = http.createServer(); await once(server.listen(0), "listening"); expect(server.listening).toBe(true); +// closeAllConnections() destroys the connections, it does not stop listening. server.closeAllConnections(); +expect(server.listening).toBe(true); +server.close(); expect(server.listening).toBe(false); +await once(server, "close"); diff --git a/test/js/bun/test/parallel/test-http-timeout-destruction-should-be-visible-using-kConnectionsCheckingInterval.ts b/test/js/bun/test/parallel/test-http-timeout-destruction-should-be-visible-using-kConnectionsCheckingInterval.ts index 70ac11ae6895..4c73ba57a6ea 100644 --- a/test/js/bun/test/parallel/test-http-timeout-destruction-should-be-visible-using-kConnectionsCheckingInterval.ts +++ b/test/js/bun/test/parallel/test-http-timeout-destruction-should-be-visible-using-kConnectionsCheckingInterval.ts @@ -6,5 +6,10 @@ const { expect } = createTest(import.meta.path); const { kConnectionsCheckingInterval } = require("_http_server"); const server = http.createServer(); await once(server.listen(0), "listening"); +expect(server[kConnectionsCheckingInterval]._destroyed).toBe(false); +// Only close() tears the interval down; closeAllConnections() keeps listening. server.closeAllConnections(); +expect(server[kConnectionsCheckingInterval]._destroyed).toBe(false); +server.close(); expect(server[kConnectionsCheckingInterval]._destroyed).toBe(true); +await once(server, "close"); diff --git a/test/js/first_party/ws/ws.test.ts b/test/js/first_party/ws/ws.test.ts index eed842c0d261..61abcd6a3be8 100644 --- a/test/js/first_party/ws/ws.test.ts +++ b/test/js/first_party/ws/ws.test.ts @@ -769,6 +769,7 @@ it("Server should be able to send empty pings", async () => { return await promise; } finally { httpServer.closeAllConnections(); + httpServer.close(); } } { diff --git a/test/js/node/http/node-http-close-all-connections.test.ts b/test/js/node/http/node-http-close-all-connections.test.ts new file mode 100644 index 000000000000..4ac55881e25b --- /dev/null +++ b/test/js/node/http/node-http-close-all-connections.test.ts @@ -0,0 +1,119 @@ +// These tests also pass on Node.js. `server.closeAllConnections()` destroys the +// tracked connections and leaves the listen socket alone, so the server keeps +// accepting traffic and a later `close(cb)` still completes normally. +import { expect, test } from "bun:test"; +import { once } from "node:events"; +import { createServer, type Server } from "node:http"; +import { connect, type AddressInfo, type Socket } from "node:net"; + +async function listenAndConnect(server: Server) { + const connected = once(server, "connection"); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const { port } = server.address() as AddressInfo; + + const client = connect(port, "127.0.0.1"); + client.on("error", () => {}); + await once(client, "connect"); + client.write("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n"); + + const [serverSocket] = await connected; + return { port, client, serverSocket }; +} + +test("closeAllConnections() destroys the connections but keeps the server listening", async () => { + const server = createServer((req, res) => res.end("ok")); + let closeEvents = 0; + server.on("close", () => closeEvents++); + try { + const { port, client, serverSocket } = await listenAndConnect(server); + // Read the response so the connection is idle but still kept alive. + await once(client, "data"); + + // Node destroys the socket object itself, so anything tracking sockets via + // 'connection' + socket.on('close') sees them leave. + const serverSocketClosed = once(serverSocket, "close"); + const clientClosed = once(client, "close"); + server.closeAllConnections(); + + expect(serverSocket.destroyed).toBe(true); + await serverSocketClosed; + await clientClosed; + + expect(server.listening).toBe(true); + expect(closeEvents).toBe(0); + + const res = await fetch(`http://127.0.0.1:${port}/`); + expect(await res.text()).toBe("ok"); + expect(res.status).toBe(200); + + const { promise, resolve } = Promise.withResolvers(); + server.close(resolve); + expect(await promise).toBeUndefined(); + expect(server.listening).toBe(false); + expect(closeEvents).toBe(1); + } finally { + server.closeAllConnections(); + if (server.listening) server.close(); + } +}); + +test("closeAllConnections() destroys in-flight connections after close()", async () => { + const { promise: requestReceived, resolve: onRequest } = Promise.withResolvers(); + // Never respond: the connection stays in-flight, so close() alone cannot finish. + const server = createServer(() => onRequest()); + try { + const { client, serverSocket } = await listenAndConnect(server); + await requestReceived; + + const { promise: serverClosed, resolve: onClosed } = Promise.withResolvers(); + const clientClosed = once(client, "close"); + server.close(onClosed); + server.closeAllConnections(); + + expect(serverSocket.destroyed).toBe(true); + await clientClosed; + expect(await serverClosed).toBeUndefined(); + expect(server.listening).toBe(false); + } finally { + if (server.listening) server.close(); + } +}); + +test("closeAllConnections() destroys every tracked connection", async () => { + const server = createServer((req, res) => res.end("ok")); + const serverSockets: Socket[] = []; + server.on("connection", socket => serverSockets.push(socket)); + try { + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const { port } = server.address() as AddressInfo; + + const clients: Socket[] = []; + for (let i = 0; i < 4; i++) { + const client = connect(port, "127.0.0.1"); + client.on("error", () => {}); + await once(client, "connect"); + client.write("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n"); + await once(client, "data"); + clients.push(client); + } + expect(serverSockets).toHaveLength(4); + + const allClosed = Promise.all(clients.map(client => once(client, "close"))); + server.closeAllConnections(); + expect(serverSockets.map(socket => socket.destroyed)).toEqual([true, true, true, true]); + + await allClosed; + expect(server.listening).toBe(true); + } finally { + server.closeAllConnections(); + if (server.listening) server.close(); + } +}); + +test("closeAllConnections() on a server that never listened does nothing", () => { + const server = createServer(() => {}); + expect(() => server.closeAllConnections()).not.toThrow(); + expect(server.listening).toBe(false); +}); diff --git a/test/js/node/http/node-http-with-ws.test.ts b/test/js/node/http/node-http-with-ws.test.ts index a3ef8cac6a29..b64787a6b9a4 100644 --- a/test/js/node/http/node-http-with-ws.test.ts +++ b/test/js/node/http/node-http-with-ws.test.ts @@ -94,6 +94,7 @@ test.concurrent("should not crash when closing sockets after upgrade", async () http_socket?.destroy(); }); server.closeAllConnections(); + server.close(); resolve(); }, 10); } diff --git a/test/js/node/http/node-http.test.ts b/test/js/node/http/node-http.test.ts index 261e2007baa2..8e3b32f50ce6 100644 --- a/test/js/node/http/node-http.test.ts +++ b/test/js/node/http/node-http.test.ts @@ -1542,6 +1542,8 @@ describe("HTTP Server Security Tests - Advanced", () => { // Close the server if it's still running if (server.listening) { server.closeAllConnections(); + server.close(); + await once(server, "close"); } }); diff --git a/test/js/web/fetch/client-fetch.test.ts b/test/js/web/fetch/client-fetch.test.ts index 37cf159bbc09..2b90d8a2c14c 100644 --- a/test/js/web/fetch/client-fetch.test.ts +++ b/test/js/web/fetch/client-fetch.test.ts @@ -85,6 +85,7 @@ test("pre aborted with readable request body", async () => { ).rejects.toThrow(); } finally { server.closeAllConnections(); + server.close(); } }); @@ -559,6 +560,7 @@ test("fetching with Request object - issue #1527", async () => { expect(await fetch(request)).resolves.pass(); } finally { server.closeAllConnections(); + server.close(); } }); diff --git a/test/js/web/fetch/fetch.stream.test.ts b/test/js/web/fetch/fetch.stream.test.ts index a5ff4cd38938..8a5f03340992 100644 --- a/test/js/web/fetch/fetch.stream.test.ts +++ b/test/js/web/fetch/fetch.stream.test.ts @@ -246,6 +246,7 @@ describe.concurrent("fetch() with streaming", () => { expect(true).toBe(true); } finally { server?.closeAllConnections(); + server?.close(); } }); }