node:http: gate server.close() on connection drain, not request count

src/js/node/_http_server.ts

@@ -86,6 +86,8 @@ const OutgoingMessagePrototype = OutgoingMessage.prototype;
 const { kIncomingMessage } = require("node:_http_common");
 const kConnectionsCheckingInterval = Symbol("http.server.connectionsCheckingInterval");
 const kTrackedConnections = Symbol("http.server.trackedConnections");
+const kClosing = Symbol("http.server.closing");
+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 +122,23 @@ const DateNow = Date.now;
 let cluster;
 
 function emitCloseServer(self: Server) {
+  // Like Node.js's net.Server#_emitCloseIfDrained: 'close' (and the close
+  // callback) only fire once every accepted connection has ended, and a
+  // re-listen before the drain completes cancels it (Node bails when
+  // `_handle` is set again). The native all-closed promise that schedules
+  // this 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; the last connection's #onClose re-checks and reschedules once
+  // kTrackedConnections empties.
+  if (!self[kClosing]) return;
+  const connections = self[kTrackedConnections];
+  if (connections && connections.size > 0) {
+    self[kPendingDrainClose] = true;
+    return;
+  }
+  self[kPendingDrainClose] = false;
+  self[kClosing] = false;
+  self[serverSymbol] = undefined;
   callCloseCallback(self);
   self.emit("close");
 }
@@ -276,7 +295,7 @@ function emitRequestCloseNT(self) {
 }
 
 function emitListeningNextTick(self, hostname, port) {
-  if ((self.listening = !!self[serverSymbol])) {
+  if ((self.listening = !!self[serverSymbol] && !self[kClosing])) {
     // TODO: remove the arguments
     // Note does not pass any arguments.
     self.emit("listening", null, hostname, port);
@@ -309,6 +328,8 @@ function Server(options, callback): void {
   defineHttpAllowHalfOpen(this);
   this[kInternalSocketData] = undefined;
   this[kTrackedConnections] = new Set();
+  this[kClosing] = false;
+  this[kPendingDrainClose] = false;
   this[tlsSymbol] = null;
   this.noDelay = true;
   if (typeof options === "function") {
@@ -472,15 +493,14 @@ Server.prototype.unref = function () {
 };
 
 Server.prototype.closeAllConnections = function () {
-  const server = this[serverSymbol];
-  if (!server) {
-    return;
+  // Node destroys every tracked connection and leaves the listen socket alone.
+  // Driven off kTrackedConnections so it also works as the forced half of a
+  // close() + closeAllConnections() drain, once the native handle is gone.
+  const connections = this[kTrackedConnections];
+  if (!connections) return;
+  for (const socket of connections) {
+    socket.destroy();
   }
-  this[serverSymbol] = undefined;
-  clearInterval(this[kConnectionsCheckingInterval]);
-  this.listening = false;
-
-  server.stop(true);
 };
 
 Server.prototype.getConnections = function (callback) {
@@ -494,21 +514,40 @@ Server.prototype.getConnections = function (callback) {
 };
 
 Server.prototype.closeIdleConnections = function () {
-  const server = this[serverSymbol];
-  server?.closeIdleConnections();
+  // Like Node.js: destroy connections that are neither writing a response nor
+  // receiving a request, and leave the listen socket alone. On a live server
+  // the native sweep is authoritative (its isIdle flag spares connections
+  // whose request head is still arriving); the kTrackedConnections pass runs
+  // only during a close() drain, so it keeps working once the native app has
+  // deinit'd without undoing that decision on a live server.
+  this[serverSymbol]?.closeIdleConnections();
+  if (!this[kClosing]) return;
+  const connections = this[kTrackedConnections];
+  if (!connections) return;
+  for (const socket of connections) {
+    if (socket.destroyed) continue;
+    const message = socket._httpMessage;
+    if (message && !message.finished) continue;
+    if (socket[kPipelinedResponses]?.length) continue;
+    if (socket[kHandle]?.response) continue;
+    socket.destroy();
+  }
 };
 
 Server.prototype.close = function (optionalCallback?) {
   const server = this[serverSymbol];
   // Node.js's httpServerPreClose clears the connections-checking interval
   // even when the server was never listening.
   clearInterval(this[kConnectionsCheckingInterval]);
-  if (!server) {
+  if (!server || this[kClosing]) {
     if (typeof optionalCallback === "function") process.nextTick(optionalCallback, $ERR_SERVER_NOT_RUNNING());
     // Like Node.js's net.Server#close, close() returns the server.
     return this;
   }
-  this[serverSymbol] = undefined;
+  // The native handle stays reachable until every connection has drained
+  // (emitCloseServer clears it) so closeIdleConnections/closeAllConnections
+  // keep working after close(); kClosing is Node's "_handle == null" stand-in.
+  this[kClosing] = true;
   if (typeof optionalCallback === "function") setCloseCallback(this, optionalCallback);
   this.listening = false;
   server.closeIdleConnections();
@@ -553,7 +592,7 @@ Server.prototype[Symbol.asyncDispose] = function () {
 };
 
 Server.prototype.address = function () {
-  if (!this[serverSymbol]) return null;
+  if (!this[serverSymbol] || this[kClosing]) return null;
   return this[serverSymbol].address;
 };
 
@@ -671,6 +710,10 @@ Server.prototype[kRealListen] = function (tls, port, host, socketPath, reusePort
     if (tls) {
       this.serverName = tls.serverName || host || "localhost";
     }
+    // A listen() during a prior close()'s drain installs a fresh handle; the
+    // pending drain must not clear it (emitCloseServer bails on !kClosing).
+    this[kClosing] = false;
+    this[kPendingDrainClose] = false;
     this[serverSymbol] = Bun.serve<any>({
       idleTimeout: 0, // nodejs dont have a idleTimeout by default
       tls,
@@ -1667,7 +1710,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/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");

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");

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();
     }
   }
   {

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);
       }