diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index 89b6db00bacd..00fcd70b028f 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -494,17 +494,29 @@ Server.prototype.unref = function () { return this; }; +// Node.js's ConnectionsList (what closeAllConnections/closeIdleConnections walk) +// is parser-keyed, and freeParser() removes a connection once its 'connect' or +// 'upgrade' request has been received in full: at the handoff for CONNECT and +// body-less upgrades, after the body for an upgrade that carries one. +// releaseServerParserShim() nulls socket.parser at the handoff in every case, +// so a handed-off socket still counts as listed while that body is arriving +// (hasIncompleteRequest stays set until the message completes). +function isOutsideConnectionsList(socket) { + return socket.parser == null && !socket[kHandle]?.hasIncompleteRequest; +} + Server.prototype.closeAllConnections = function () { closeAllHttp1Connections(this); - const server = this[serverSymbol]; - if (!server) { + const connections = this[kTrackedConnections]; + if (!connections) { return; } - this[serverSymbol] = undefined; - clearInterval(this[kConnectionsCheckingInterval]); - this.listening = false; - - server.stop(true); + for (const socket of connections) { + if (isOutsideConnectionsList(socket)) { + continue; + } + socket.destroy(); + } }; Server.prototype.getConnections = function (callback) { @@ -519,8 +531,33 @@ Server.prototype.getConnections = function (callback) { Server.prototype.closeIdleConnections = function () { closeIdleHttp1Connections(this); - const server = this[serverSymbol]; - server?.closeIdleConnections(); + const connections = this[kTrackedConnections]; + if (!connections) { + return; + } + for (const socket of connections) { + if (isOutsideConnectionsList(socket)) { + continue; + } + const message = socket._httpMessage; + if (message && !message.finished) { + continue; + } + // Deliberately unlike Node.js, which destroys the connection here and + // drops the queued responses: a pipelined queue counts as in flight, as it + // does for the native idle sweep that close() runs. + if (socket[kPipelinedResponses]?.length) { + continue; + } + // Node.js's ConnectionsList.idle() also skips parsers whose + // last_message_start_ is set, i.e. a request head or body is still being + // received (a connection that has not sent its first request yet counts + // too); hasIncompleteRequest is the native handle's view of the same state. + if (socket[kHandle]?.hasIncompleteRequest) { + continue; + } + socket.destroy(); + } }; Server.prototype.close = function (optionalCallback?) { diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp index 441e75d4fbe4..3cac55c4b93d 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp @@ -300,6 +300,27 @@ bool JSNodeHTTPServerSocket::isRequestTimedOut(uint64_t headersTimeoutMs, uint64 return isRequestTimedOutImpl(socket, headersTimeoutMs, requestTimeoutMs); } +template +static bool hasIncompleteRequestImpl(us_socket_t* socket) +{ + auto* httpResponseData = reinterpret_cast*>(us_socket_ext(socket)); + if (httpResponseData->isConnectRequest) { + return false; + } + return httpResponseData->lastMessageStartMs != 0; +} + +bool JSNodeHTTPServerSocket::hasIncompleteRequest() const +{ + if (!socket || upgraded || us_socket_is_closed(socket)) { + return false; + } + if (is_ssl) { + return hasIncompleteRequestImpl(socket); + } + return hasIncompleteRequestImpl(socket); +} + bool JSNodeHTTPServerSocket::isAuthorized() const { // is secure means that tls was established successfully diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.h b/src/jsc/bindings/node/JSNodeHTTPServerSocket.h index 42795b49c421..fd5ef59b87a4 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.h +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.h @@ -88,6 +88,10 @@ class JSNodeHTTPServerSocket : public JSC::JSDestructibleObject { * (both in milliseconds; 0 disables the respective check). */ bool isRequestTimedOut(uint64_t headersTimeoutMs, uint64_t requestTimeoutMs) const; + /* node:http server compat: whether a request message is currently being + * received on this connection (Node's parser.last_message_start_ != 0). */ + bool hasIncompleteRequest() const; + /* node:http server compat - HTTP/1.1 pipelining. Responses for requests * that were parsed while an earlier response on this connection was still * in flight are queued here (in arrival order) and become the connection's diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp b/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp index 59e3be6835e9..fecea7031c7e 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp @@ -23,6 +23,7 @@ using namespace WebCore; JSC_DECLARE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterOnClose); JSC_DECLARE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterOnDrain); JSC_DECLARE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterClosed); +JSC_DECLARE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterHasIncompleteRequest); JSC_DECLARE_CUSTOM_SETTER(jsNodeHttpServerSocketSetterOnClose); JSC_DECLARE_CUSTOM_SETTER(jsNodeHttpServerSocketSetterOnDrain); JSC_DECLARE_CUSTOM_SETTER(jsNodeHttpServerSocketSetterOnData); @@ -60,6 +61,7 @@ static const JSC::HashTableValue JSNodeHTTPServerSocketPrototypeTableValues[] = { "ondata"_s, static_cast(JSC::PropertyAttribute::CustomAccessor), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterOnData, jsNodeHttpServerSocketSetterOnData } }, { "bytesWritten"_s, static_cast(JSC::PropertyAttribute::CustomAccessor), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterBytesWritten, noOpSetter } }, { "closed"_s, static_cast(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterClosed, noOpSetter } }, + { "hasIncompleteRequest"_s, static_cast(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterHasIncompleteRequest, noOpSetter } }, { "response"_s, static_cast(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterResponse, noOpSetter } }, { "duplex"_s, static_cast(JSC::PropertyAttribute::CustomAccessor), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterDuplex, jsNodeHttpServerSocketSetterDuplex } }, { "remoteAddress"_s, static_cast(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterRemoteAddress, noOpSetter } }, @@ -513,6 +515,15 @@ JSC_DEFINE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterClosed, (JSC::JSGlobalObjec return JSValue::encode(JSC::jsBoolean(thisObject->isClosed())); } +JSC_DEFINE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterHasIncompleteRequest, (JSC::JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, JSC::PropertyName propertyName)) +{ + auto* thisObject = dynamicDowncast(JSC::JSValue::decode(thisValue)); + if (!thisObject) [[unlikely]] { + return JSValue::encode(JSC::jsUndefined()); + } + return JSValue::encode(JSC::jsBoolean(thisObject->hasIncompleteRequest())); +} + JSC_DEFINE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterBytesWritten, (JSC::JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, JSC::PropertyName propertyName)) { auto* thisObject = dynamicDowncast(JSC::JSValue::decode(thisValue)); diff --git a/test/js/bun/http/node-http-halfclose-midupload.test.ts b/test/js/bun/http/node-http-halfclose-midupload.test.ts index 0df21738886d..19fa7c770725 100644 --- a/test/js/bun/http/node-http-halfclose-midupload.test.ts +++ b/test/js/bun/http/node-http-halfclose-midupload.test.ts @@ -67,8 +67,6 @@ async function runTeardownStages(bind: string | undefined, url: (port: number) = try { expect(await withTimeout(writeResult.promise, "write-after-end")).toBeTrue(); await withTimeout(fetchSettled, "fetch-settled"); - // Not server.closeAllConnections(): bun's implementation also stops the - // server, which makes the disposal/close below reject. socket?.destroy(); await withTimeout(connectionClosed.promise, "connection-closed"); const serverClosed = new Promise((resolve, reject) => server.close(err => (err ? reject(err) : resolve()))); 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..79cf00386b59 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 389d280be903..5df42dfe4c9b 100644 --- a/test/js/first_party/ws/ws.test.ts +++ b/test/js/first_party/ws/ws.test.ts @@ -771,6 +771,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-server-close-connections.test.ts b/test/js/node/http/node-http-server-close-connections.test.ts new file mode 100644 index 000000000000..8368af5697d9 --- /dev/null +++ b/test/js/node/http/node-http-server-close-connections.test.ts @@ -0,0 +1,458 @@ +// server.closeIdleConnections() / server.closeAllConnections() must keep +// working after server.close() has run: that is the canonical graceful-drain +// pattern (close(); wait; closeIdleConnections()) and the force path used by +// http-terminator. These tests also pass on Node.js v26, except the subprocess +// test at the end and the one pipelining test that says otherwise. +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; +import { once } from "node:events"; +import { createServer, type Server, type ServerResponse } from "node:http"; +import { connect, type AddressInfo, type Socket } from "node:net"; + +async function listen(server: Server) { + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + return (server.address() as AddressInfo).port; +} + +async function openConnection(server: Server, port: number) { + const gotConnection = once(server, "connection"); + const client = connect(port, "127.0.0.1"); + client.on("error", () => {}); + client.on("data", () => {}); + await once(client, "connect"); + return { client, gotConnection }; +} + +function waitClose(client: Socket) { + // once() rejects on 'error'; the client may see ECONNRESET on a forced + // close, which for this test still means "the connection was reaped". + return new Promise(resolve => client.once("close", () => resolve())); +} + +describe.each(["closeIdleConnections", "closeAllConnections"] as const)("%s", method => { + test("does not touch a socket handed to the 'upgrade' listener", async () => { + const server = createServer(); + let upgraded!: Socket; + server.on("upgrade", (req, sock) => { + upgraded = sock; + sock.write("HTTP/1.1 101 Switching Protocols\r\nConnection: Upgrade\r\nUpgrade: x\r\n\r\n"); + }); + try { + const port = await listen(server); + const { client } = await openConnection(server, port); + const gotResponse = once(client, "data"); + client.write("GET / HTTP/1.1\r\nHost: x\r\nConnection: Upgrade\r\nUpgrade: x\r\n\r\n"); + await gotResponse; + + // Node.js's ConnectionsList is parser-keyed; a body-less upgrade request + // is complete when it is handed off, so freeParser() has removed the + // entry by the time 'upgrade' is emitted and neither call reaches it. + server[method](); + expect(upgraded.destroyed).toBe(false); + + upgraded.destroy(); + client.destroy(); + await new Promise(r => server.close(() => r())); + } finally { + server.closeAllConnections(); + if (server.listening) server.close(); + } + }); + + test("treats an upgrade request whose body is still arriving like Node.js", async () => { + const server = createServer(); + const { promise: upgradeEmitted, resolve: onUpgrade } = Promise.withResolvers(); + server.on("upgrade", () => onUpgrade()); + try { + const port = await listen(server); + const { client, gotConnection } = await openConnection(server, port); + // 3 of the 10 body bytes: Node.js (v26, which delivers upgrade bodies) + // only frees the parser once the request is complete, so until then the + // connection is still listed. closeAllConnections() destroys it; + // closeIdleConnections() skips it because a message is being received. + client.write("GET / HTTP/1.1\r\nHost: x\r\nConnection: Upgrade\r\nUpgrade: x\r\nContent-Length: 10\r\n\r\nabc"); + const [serverSocket] = await gotConnection; + await upgradeEmitted; + + server[method](); + expect(serverSocket.destroyed).toBe(method === "closeAllConnections"); + + serverSocket.destroy(); + client.destroy(); + await new Promise(r => server.close(() => r())); + } finally { + server.closeAllConnections(); + if (server.listening) server.close(); + } + }); + + test("reaps a connection that went idle after close()", async () => { + let finishResponse!: () => void; + const responseGate = new Promise(r => (finishResponse = r)); + const { promise: requestReceived, resolve: onRequest } = Promise.withResolvers(); + const { promise: responded, resolve: onResponded } = Promise.withResolvers(); + const server = createServer(async (req, res) => { + onRequest(); + await responseGate; + res.on("finish", () => onResponded()); + res.end("ok"); + }); + server.keepAliveTimeout = 60_000; + try { + const port = await listen(server); + const { client, gotConnection } = await openConnection(server, port); + const clientClosed = waitClose(client); + + // Request is in flight (handler running) when close() runs, so close() + // on its own leaves this connection open. + client.write("GET / HTTP/1.1\r\nHost: x\r\nConnection: keep-alive\r\n\r\n"); + const [serverSocket] = await gotConnection; + await requestReceived; + await new Promise(r => setImmediate(r)); + server.close(); + + // Let the response finish: the connection is now idle but still open + // (kept alive). + finishResponse(); + await responded; + expect(serverSocket.destroyed).toBe(false); + + // The post-close call must reap it. + server[method](); + expect(serverSocket.destroyed).toBe(true); + await clientClosed; + client.destroy(); + } finally { + server.closeAllConnections(); + if (server.listening) server.close(); + } + }); +}); + +describe("closeIdleConnections", () => { + test("skips connections with an incomplete request head", async () => { + const server = createServer((req, res) => res.end("ok")); + server.keepAliveTimeout = 60_000; + server.headersTimeout = 0; + try { + const port = await listen(server); + + // Fresh accept, zero bytes: Node.js initializes last_message_start_ on + // parser creation as DoS protection, so this is not idle. + const { client: fresh, gotConnection: freshConn } = await openConnection(server, port); + const [freshServerSocket] = await freshConn; + + // Partial request head: last_message_start_ is non-zero, so not idle. + const { client: partial, gotConnection: partialConn } = await openConnection(server, port); + const [partialServerSocket] = await partialConn; + partial.write("GET / HTTP/1.1"); + + // Completed cycle, now keep-alive idle: this one is reaped. + const { client: idle, gotConnection: idleConn } = await openConnection(server, port); + const idleResponse = once(idle, "data"); + idle.write("GET / HTTP/1.1\r\nHost: x\r\nConnection: keep-alive\r\n\r\n"); + const [idleServerSocket] = await idleConn; + await idleResponse; + + server.closeIdleConnections(); + + expect({ + fresh: freshServerSocket.destroyed, + partial: partialServerSocket.destroyed, + idle: idleServerSocket.destroyed, + }).toEqual({ fresh: false, partial: false, idle: true }); + + fresh.destroy(); + partial.destroy(); + idle.destroy(); + server.closeAllConnections(); + await new Promise(r => server.close(() => r())); + } finally { + server.closeAllConnections(); + if (server.listening) server.close(); + } + }); + + test("skips a connection whose request body is still arriving, reaps it once received", async () => { + // Responds before the body has arrived; the connection stays keep-alive and + // the rest of the body is read and discarded. + const server = createServer((req, res) => res.end("ok")); + server.keepAliveTimeout = 60_000; + try { + const port = await listen(server); + const { client, gotConnection } = await openConnection(server, port); + const response = once(client, "data"); + client.write("POST / HTTP/1.1\r\nHost: x\r\nContent-Length: 6\r\n\r\nabc"); + const [serverSocket] = await gotConnection; + await response; + + // The response is finished, but the request message is not: still busy. + server.closeIdleConnections(); + expect(serverSocket.destroyed).toBe(false); + + // Nothing in JS observes the discarded bytes arriving, so the sweep itself + // is the observable: it keeps skipping the connection until they have. + client.write("def"); + while (!serverSocket.destroyed) { + await new Promise(r => setImmediate(r)); + server.closeIdleConnections(); + } + await waitClose(client); + } finally { + server.closeAllConnections(); + if (server.listening) server.close(); + } + }); + + test("keeps a connection with pipelined responses still queued (unlike Node.js)", async () => { + const responses: ServerResponse[] = []; + const { promise: bothDispatched, resolve: onBothDispatched } = Promise.withResolvers(); + const server = createServer((req, res) => { + responses.push(res); + if (responses.length === 2) onBothDispatched(); + }); + server.keepAliveTimeout = 60_000; + try { + const port = await listen(server); + const { client, gotConnection } = await openConnection(server, port); + let received = ""; + const { promise: gotBothBodies, resolve: onBothBodies } = Promise.withResolvers(); + client.on("data", chunk => { + received += chunk; + if (received.includes("body-a") && received.includes("body-b")) onBothBodies(); + }); + client.write("GET /a HTTP/1.1\r\nHost: x\r\n\r\nGET /b HTTP/1.1\r\nHost: x\r\n\r\n"); + const [serverSocket] = await gotConnection; + await bothDispatched; + const [resA, resB] = responses; + + // Synchronously after the first response finishes, the second one is + // still queued behind it. Node.js v26 destroys the connection at this + // point and never delivers the second response; Bun treats the queue as + // in flight, like its native idle sweep does. + resA.end("body-a"); + server.closeIdleConnections(); + expect(serverSocket.destroyed).toBe(false); + + resB.end("body-b"); + await gotBothBodies; + + server.closeIdleConnections(); + expect(serverSocket.destroyed).toBe(true); + await waitClose(client); + } finally { + server.closeAllConnections(); + if (server.listening) server.close(); + } + }); + + test("skips in-flight connections and reaps idle ones", async () => { + const inflightResponses: ServerResponse[] = []; + const server = createServer((req, res) => { + if (req.url === "/inflight") { + inflightResponses.push(res); + return; // never respond + } + res.end("ok"); + }); + server.keepAliveTimeout = 60_000; + try { + const port = await listen(server); + + const { client: idle, gotConnection: idleConn } = await openConnection(server, port); + const idleResponse = once(idle, "data"); + const idleClosed = waitClose(idle); + idle.write("GET /idle HTTP/1.1\r\nHost: x\r\nConnection: keep-alive\r\n\r\n"); + const [idleServerSocket] = await idleConn; + await idleResponse; + + const { client: busy, gotConnection: busyConn } = await openConnection(server, port); + const busyClosed = waitClose(busy); + busy.write("GET /inflight HTTP/1.1\r\nHost: x\r\nConnection: keep-alive\r\n\r\n"); + const [busyServerSocket] = await busyConn; + while (inflightResponses.length === 0) await new Promise(r => setImmediate(r)); + + server.closeIdleConnections(); + + expect(idleServerSocket.destroyed).toBe(true); + expect(busyServerSocket.destroyed).toBe(false); + await idleClosed; + + server.closeAllConnections(); + await busyClosed; + idle.destroy(); + busy.destroy(); + await new Promise(r => server.close(() => r())); + } finally { + server.closeAllConnections(); + if (server.listening) server.close(); + } + }); +}); + +describe("closeAllConnections", () => { + test("after close(), destroys in-flight connections so the close callback runs", 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 port = await listen(server); + const { client, gotConnection } = await openConnection(server, port); + const clientClosed = waitClose(client); + client.write("GET / HTTP/1.1\r\nHost: x\r\n\r\n"); + const [serverSocket] = await gotConnection; + await requestReceived; + + const { promise: closed, resolve: onClosed } = Promise.withResolvers(); + server.close(onClosed); + server.closeAllConnections(); + + expect(serverSocket.destroyed).toBe(true); + await clientClosed; + expect(await closed).toBeUndefined(); + client.destroy(); + } finally { + server.closeAllConnections(); + if (server.listening) server.close(); + } + }); + + test("does not stop the listen socket", async () => { + const server = createServer((req, res) => res.end("ok")); + let closeEvents = 0; + server.on("close", () => closeEvents++); + try { + const port = await listen(server); + const { client } = await openConnection(server, port); + const firstResponse = once(client, "data"); + client.write("GET / HTTP/1.1\r\nHost: x\r\nConnection: keep-alive\r\n\r\n"); + await firstResponse; + + const clientClosed = waitClose(client); + server.closeAllConnections(); + await clientClosed; + + // The listener is untouched: still listening, no 'close' event, and a + // fresh request is served. + 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("is a no-op when no connections are established", async () => { + const server = createServer((req, res) => res.end("ok")); + try { + const port = await listen(server); + + // No client has connected yet: must not throw and must not affect the listener. + expect(() => server.closeAllConnections()).not.toThrow(); + expect(server.listening).toBe(true); + + const res = await fetch(`http://127.0.0.1:${port}/`); + expect(await res.text()).toBe("ok"); + expect(res.status).toBe(200); + } finally { + server.closeAllConnections(); + if (server.listening) server.close(); + } + }); + + test("destroys every tracked connection", async () => { + const server = createServer((req, res) => res.end("ok")); + const serverSockets: Socket[] = []; + server.on("connection", socket => serverSockets.push(socket)); + try { + const port = await listen(server); + + const clients: Socket[] = []; + for (let i = 0; i < 4; i++) { + const { client } = await openConnection(server, port); + const response = once(client, "data"); + client.write("GET / HTTP/1.1\r\nHost: x\r\nConnection: keep-alive\r\n\r\n"); + await response; + clients.push(client); + } + expect(serverSockets).toHaveLength(4); + + const allClosed = Promise.all(clients.map(waitClose)); + server.closeAllConnections(); + + // Like Node, the socket objects themselves are destroyed synchronously, so + // the usual 'connection' + socket.on('close') bookkeeping sees them leave. + expect(serverSockets.map(socket => socket.destroyed)).toEqual([true, true, true, true]); + await allClosed; + expect(server.listening).toBe(true); + + const { promise, resolve } = Promise.withResolvers(); + server.close(resolve); + expect(await promise).toBeUndefined(); + } finally { + server.closeAllConnections(); + if (server.listening) server.close(); + } + }); + + test("is a no-op on a server that never listened", () => { + const server = createServer(); + expect(() => server.closeAllConnections()).not.toThrow(); + expect(() => server.closeIdleConnections()).not.toThrow(); + }); +}); + +// https://github.com/oven-sh/bun/issues/30501: @azure/msal-node tears its +// loopback redirect server down with exactly this sequence. The browser's +// connection is still in flight at that point, so only closeAllConnections() +// can reclaim it; when it was a no-op after close(), the process hung. +test("close(); closeAllConnections(); unref() with an in-flight request lets the process exit", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const http = require("node:http"); + const net = require("node:net"); + const server = http.createServer(() => { + // Never respond; tear down while the request is in flight. + server.close(); + server.closeAllConnections(); + server.unref(); + console.log("teardown done"); + setTimeout(() => { + console.log("still alive after teardown"); + process.exit(7); + }, 3000).unref(); + }); + server.listen(0, "127.0.0.1", () => { + const client = net.connect(server.address().port, "127.0.0.1", () => { + client.write("GET / HTTP/1.1\\r\\nHost: x\\r\\nConnection: keep-alive\\r\\n\\r\\n"); + }); + client.on("error", () => {}); + }); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout).toBe("teardown done\n"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + // Debug builds spend a few seconds just loading node:http in the child, and + // the child's own 3s watchdog needs to get its message out when this breaks. +}, 15_000); 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 b9edb1f12756..ee81d97f8e29 100644 --- a/test/js/node/http/node-http.test.ts +++ b/test/js/node/http/node-http.test.ts @@ -1543,6 +1543,7 @@ describe("HTTP Server Security Tests - Advanced", () => { // Close the server if it's still running if (server.listening) { server.closeAllConnections(); + server.close(); } }); diff --git a/test/js/web/fetch/client-fetch.test.ts b/test/js/web/fetch/client-fetch.test.ts index 26443a1d7598..d5831297efe3 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 fda8e9098669..e15493de4865 100644 --- a/test/js/web/fetch/fetch.stream.test.ts +++ b/test/js/web/fetch/fetch.stream.test.ts @@ -243,6 +243,7 @@ describe.concurrent("fetch() with streaming", () => { expect(true).toBe(true); } finally { server?.closeAllConnections(); + server?.close(); } }); }