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
15 changes: 8 additions & 7 deletions src/js/node/_http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
};
Comment thread
robobun marked this conversation as resolved.
Comment thread
claude[bot] marked this conversation as resolved.

Server.prototype.getConnections = function (callback) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Original file line number Diff line number Diff line change
Expand Up @@ -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");
1 change: 1 addition & 0 deletions test/js/first_party/ws/ws.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,7 @@ it("Server should be able to send empty pings", async () => {
return await promise;
} finally {
httpServer.closeAllConnections();
httpServer.close();
}
}
{
Expand Down
119 changes: 119 additions & 0 deletions test/js/node/http/node-http-close-all-connections.test.ts
Original file line number Diff line number Diff line change
@@ -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<Error | undefined>();
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<void>();
// 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<Error | undefined>();
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);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
1 change: 1 addition & 0 deletions test/js/node/http/node-http-with-ws.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ test.concurrent("should not crash when closing sockets after upgrade", async ()
http_socket?.destroy();
});
server.closeAllConnections();
server.close();
resolve();
}, 10);
}
Expand Down
2 changes: 2 additions & 0 deletions test/js/node/http/node-http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
});

Expand Down
2 changes: 2 additions & 0 deletions test/js/web/fetch/client-fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ test("pre aborted with readable request body", async () => {
).rejects.toThrow();
} finally {
server.closeAllConnections();
server.close();
}
});

Expand Down Expand Up @@ -559,6 +560,7 @@ test("fetching with Request object - issue #1527", async () => {
expect(await fetch(request)).resolves.pass();
} finally {
server.closeAllConnections();
server.close();
}
});

Expand Down
1 change: 1 addition & 0 deletions test/js/web/fetch/fetch.stream.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ describe.concurrent("fetch() with streaming", () => {
expect(true).toBe(true);
} finally {
server?.closeAllConnections();
server?.close();
}
});
}
Expand Down
Loading