node:http: defer server 'close' until every tracked connection has ended

src/js/node/_http_server.ts

@@ -86,6 +86,7 @@ const OutgoingMessagePrototype = OutgoingMessage.prototype;
 const { kIncomingMessage } = require("node:_http_common");
 const kConnectionsCheckingInterval = Symbol("http.server.connectionsCheckingInterval");
 const kTrackedConnections = Symbol("http.server.trackedConnections");
+const kPendingDrainClose = Symbol("http.server.pendingDrainClose");
 const kHttpAllowHalfOpen = Symbol("http.server.httpAllowHalfOpen");
 
 // node.http trace events ('http.server.request' b/e). The agent module is
@@ -120,6 +121,16 @@ const DateNow = Date.now;
 let cluster;
 
 function emitCloseServer(self: Server) {
+  // Like Node.js's net.Server#_emitCloseIfDrained: 'close' waits for every
+  // accepted connection to end. The native all-closed promise resolves on
+  // pending_requests == 0, so a keep-alive connection that was serving a
+  // request when close() ran is still open when that promise resolves.
+  const connections = self[kTrackedConnections];
+  if (connections && connections.size > 0) {
+    self[kPendingDrainClose] = true;
+    return;
+  }
+  self[kPendingDrainClose] = false;
   callCloseCallback(self);
   self.emit("close");
 }
@@ -309,6 +320,7 @@ function Server(options, callback): void {
   defineHttpAllowHalfOpen(this);
   this[kInternalSocketData] = undefined;
   this[kTrackedConnections] = new Set();
+  this[kPendingDrainClose] = false;
   this[tlsSymbol] = null;
   this.noDelay = true;
   if (typeof options === "function") {
@@ -1667,7 +1679,14 @@ const NodeHTTPServerSocket = class Socket extends NetSocket {
     // released parser (free() invoked, kOnTimeout nulled).
     releaseServerParserShim(this);
     this[kHandle] = null;
-    this.server?.[kTrackedConnections]?.delete(this);
+    const server = this.server;
+    const tracked = server?.[kTrackedConnections];
+    if (tracked) {
+      tracked.delete(this);
+      if (tracked.size === 0 && server[kPendingDrainClose]) {
+        process.nextTick(emitCloseServer, server);
+      }
+    }
     const timer = this[kSocketTimeoutTimer];
     if (timer) {
       clearTimeout(timer);

test/js/node/http/node-http-server-close-drain.test.ts

@@ -0,0 +1,104 @@
+import { test, expect } from "bun:test";
+import { once } from "node:events";
+import { createServer } from "node:http";
+import type { AddressInfo } from "node:net";
+import { connect } from "node:net";
+
+// Node's net.Server#close callback (and the 'close' event) only fires once
+// every accepted connection has ended. A connection that was mid-request
+// when close() ran stays open after the response is delivered, so the
+// callback must be withheld until that connection closes.
+test("server.close(cb) does not fire while a keep-alive connection is still open", async () => {
+  const inHandler = Promise.withResolvers<void>();
+  let releaseResponse!: () => void;
+  const paths: string[] = [];
+  const server = createServer((req, res) => {
+    paths.push(req.url as string);
+    if (paths.length === 1) {
+      inHandler.resolve();
+      releaseResponse = () => res.end("resp:" + req.url);
+    } else {
+      res.end("resp:" + req.url);
+    }
+  });
+  server.keepAliveTimeout = 60000;
+  server.listen(0, "127.0.0.1");
+  await once(server, "listening");
+  const { port } = server.address() as AddressInfo;
+
+  const socket = connect(port, "127.0.0.1");
+  try {
+    await once(socket, "connect");
+    let body = "";
+    socket.on("data", chunk => (body += chunk));
+    socket.on("error", () => {});
+
+    // First request: handler is entered but response held until after close().
+    socket.write("GET /first HTTP/1.1\r\nHost: x\r\n\r\n");
+    await inHandler.promise;
+
+    let closeEventFired = false;
+    server.once("close", () => (closeEventFired = true));
+    const closed = Promise.withResolvers<void>();
+    let closeCbFired = false;
+    server.close(() => {
+      closeCbFired = true;
+      closed.resolve();
+    });
+
+    // Handler finishes: response is delivered, connection stays open.
+    // Yield a few event-loop turns after the bytes arrive so the server's
+    // "all requests done" task chain has run before the callback is checked.
+    releaseResponse();
+    while (!body.includes("resp:/first")) await once(socket, "data");
+    for (let i = 0; i < 4; i++) await new Promise<void>(r => setImmediate(r));
+    expect(closeCbFired).toBe(false);
+    expect(closeEventFired).toBe(false);
+
+    // A second request on the same connection is still served (matching Node),
+    // and the close callback must still be withheld afterwards.
+    socket.write("GET /second HTTP/1.1\r\nHost: x\r\n\r\n");
+    while (!body.includes("resp:/second")) await once(socket, "data");
+    for (let i = 0; i < 4; i++) await new Promise<void>(r => setImmediate(r));
+    expect(closeCbFired).toBe(false);
+    expect(closeEventFired).toBe(false);
+    expect(paths).toEqual(["/first", "/second"]);
+
+    // Closing the connection drains the server and releases the callback.
+    socket.destroy();
+    await closed.promise;
+    expect(closeCbFired).toBe(true);
+    expect(closeEventFired).toBe(true);
+  } finally {
+    socket.destroy();
+    server.closeAllConnections();
+  }
+});
+
+// Sanity: an idle keep-alive connection at close() time is reaped by
+// closeIdleConnections(), so the callback fires promptly like before.
+test("server.close(cb) fires once an idle keep-alive connection is reaped", async () => {
+  const server = createServer((req, res) => res.end("ok"));
+  server.keepAliveTimeout = 60000;
+  server.listen(0, "127.0.0.1");
+  await once(server, "listening");
+  const { port } = server.address() as AddressInfo;
+
+  const socket = connect(port, "127.0.0.1");
+  try {
+    await once(socket, "connect");
+    let body = "";
+    socket.on("data", chunk => (body += chunk));
+    socket.on("error", () => {});
+    socket.write("GET / HTTP/1.1\r\nHost: x\r\n\r\n");
+    while (!body.includes("ok")) await once(socket, "data");
+    for (let i = 0; i < 4; i++) await new Promise<void>(r => setImmediate(r));
+
+    const closed = Promise.withResolvers<void>();
+    server.close(() => closed.resolve());
+    await closed.promise;
+  } finally {
+    socket.destroy();
+    server.closeAllConnections();
+  }
+});