node:http: make closeAllConnections()/closeIdleConnections() leave the listener alone and work after close()

src/js/node/_http_server.ts

@@ -472,15 +472,25 @@ 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: the server keeps accepting. Iterating the tracked-connection set
+  // (rather than routing through the native handle) also keeps this working
+  // once close() has dropped that handle, which is the forced half of the
+  // close() + closeAllConnections() drain used by http-terminator et al.
+  const connections = this[kTrackedConnections];
+  if (!connections) {
     return;
   }
-  this[serverSymbol] = undefined;
-  clearInterval(this[kConnectionsCheckingInterval]);
-  this.listening = false;
-
-  server.stop(true);
+  for (const socket of connections) {
+    // Node.js's ConnectionsList is parser-keyed; freeParser() removes the
+    // entry when the socket is handed to 'upgrade'/'connect', so upgraded
+    // sockets are not touched. releaseServerParserShim nulls socket.parser on
+    // that same handoff here.
+    if (socket.parser == null) {
+      continue;
+    }
+    socket.destroy();
+  }
 };
 
 Server.prototype.getConnections = function (callback) {
@@ -494,8 +504,32 @@ Server.prototype.getConnections = function (callback) {
 };
 
 Server.prototype.closeIdleConnections = function () {
-  const server = this[serverSymbol];
-  server?.closeIdleConnections();
+  // Node.js destroys each tracked connection that is between request/response
+  // cycles. Iterating the tracked-connection set keeps this working once
+  // close() has dropped the native handle, which is the graceful-drain
+  // pattern:
+  //   server.close(cb); setTimeout(() => server.closeIdleConnections(), grace)
+  const connections = this[kTrackedConnections];
+  if (!connections) {
+    return;
+  }
+  for (const socket of connections) {
+    // Node.js's ConnectionsList.idle() skips the connection when:
+    // - the parser was released for 'upgrade'/'connect' handoff
+    //   (releaseServerParserShim nulls socket.parser), or
+    // - a request message is currently being received
+    //   (parser.last_message_start_ != 0; the native handle tracks the same
+    //   timestamp as lastMessageStartMs).
+    // _httpMessage covers the "response in flight" half that Node leaves to
+    // the parser's message-complete bookkeeping.
+    if (socket.parser == null || socket._httpMessage || socket[kPipelinedResponses]?.length) {
+      continue;
+    }
+    if (socket[kHandle]?.hasIncompleteRequest) {
+      continue;
+    }
+    socket.destroy();
+  }
 };
 
 Server.prototype.close = function (optionalCallback?) {

src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp

@@ -298,6 +298,27 @@ bool JSNodeHTTPServerSocket::isRequestTimedOut(uint64_t headersTimeoutMs, uint64
     return isRequestTimedOutImpl<false>(socket, headersTimeoutMs, requestTimeoutMs);
 }
 
+template<bool SSL>
+static bool hasIncompleteRequestImpl(us_socket_t* socket)
+{
+    auto* httpResponseData = reinterpret_cast<uWS::NodeHttpResponseData<SSL>*>(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<true>(socket);
+    }
+    return hasIncompleteRequestImpl<false>(socket);
+}
+
 bool JSNodeHTTPServerSocket::isAuthorized() const
 {
     // is secure means that tls was established successfully

src/jsc/bindings/node/JSNodeHTTPServerSocket.h

@@ -85,6 +85,12 @@ 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).
+     * Used by Server.prototype.closeIdleConnections() so a connection with a
+     * partial request head in flight is not reaped as idle. */
+    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

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);
@@ -59,6 +60,7 @@ static const JSC::HashTableValue JSNodeHTTPServerSocketPrototypeTableValues[] =
     { "ondata"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterOnData, jsNodeHttpServerSocketSetterOnData } },
     { "bytesWritten"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterBytesWritten, noOpSetter } },
     { "closed"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterClosed, noOpSetter } },
+    { "hasIncompleteRequest"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterHasIncompleteRequest, noOpSetter } },
     { "response"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterResponse, noOpSetter } },
     { "duplex"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterDuplex, jsNodeHttpServerSocketSetterDuplex } },
     { "remoteAddress"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterRemoteAddress, noOpSetter } },
@@ -502,6 +504,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<JSNodeHTTPServerSocket>(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<JSNodeHTTPServerSocket>(JSC::JSValue::decode(thisValue));

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