http: option to mark active connections to close when idle

doc/api/http.md

@@ -1829,23 +1829,38 @@ setTimeout(() => {
 }, 10000);
 ```
 
-### `server.closeIdleConnections()`
+### `server.closeIdleConnections([closeWhenIdle])`
 
 <!-- YAML
-added: v18.2.0
+added:
+  - v18.2.0
+  - REPLACEME
+changes:
+  - version: REPLACEME
+    pr-url: REPLACEME
+    description: Added the `closeWhenIdle` parameter.
 -->
 
+* `closeWhenIdle` {boolean} If `true`, connections that are currently sending a
+  request or waiting for a response are marked to close as soon as that
+  request/response finishes, instead of being reused for a subsequent
+  keep-alive request. Without this, such a connection stays open until
+  `server.keepAliveTimeout` elapses. **Default:** `false`.
+
 Closes all connections connected to this server which are not sending a request
 or waiting for a response.
 
 > Starting with Node.js 19.0.0, there's no need for calling this method in
-> conjunction with `server.close` to reap `keep-alive` connections. Using it
-> won't cause any harm though, and it can be useful to ensure backwards
-> compatibility for libraries and applications that need to support versions
-> older than 19.0.0. Whenever using this in conjunction with `server.close`,
-> calling this _after_ `server.close` is recommended as to avoid race
-> conditions where new connections are created between a call to this and a
-> call to `server.close`.
+> conjunction with `server.close` to reap already-idle `keep-alive`
+> connections — `server.close()` does so automatically. It does not, however,
+> cover a connection that is still active when `server.close()` is called and
+> only becomes idle afterward; pass `closeWhenIdle: true` for that. Using
+> `closeIdleConnections()` explicitly won't cause any harm though, and it can
+> be useful to ensure backwards compatibility for libraries and applications
+> that need to support versions older than 19.0.0. Whenever using this in
+> conjunction with `server.close`, calling this _after_ `server.close` is
+> recommended as to avoid race conditions where new connections are created
+> between a call to this and a call to `server.close`.
 
 ```js
 const http = require('node:http');

lib/_http_outgoing.js

@@ -38,7 +38,7 @@ const { getDefaultHighWaterMark } = require('internal/streams/state');
 const assert = require('internal/assert');
 const EE = require('events');
 const Stream = require('stream');
-const { kOutHeaders, utcDate, kNeedDrain } = require('internal/http');
+const { kOutHeaders, utcDate, kNeedDrain, kCloseWhenIdle } = require('internal/http');
 const { Buffer } = require('buffer');
 const {
   _checkIsHttpToken: checkIsHttpToken,
@@ -536,7 +536,11 @@ function _storeHeader(firstLine, headers) {
     // even if the connection header isn't sent, we still persist by default.
     this._last = !this.shouldKeepAlive;
   } else if (!state.connection) {
+    // A socket that `server.closeIdleConnections(true)` has marked to close
+    // once it goes idle must not advertise keep-alive since the connection will
+    // not be kept around for reuse.
     const shouldSendKeepAlive = this.shouldKeepAlive &&
+        !this[kSocket]?.[kCloseWhenIdle] &&
         (state.contLen || this.useChunkedEncodingByDefault || this.agent);
     if (shouldSendKeepAlive && this.maxRequestsOnConnectionReached) {
       header += 'Connection: close\r\n';

lib/_http_server.js

@@ -61,6 +61,7 @@ const {
 const {
   kOutHeaders,
   kNeedDrain,
+  kCloseWhenIdle,
   isTraceHTTPEnabled,
   traceBegin,
   traceEnd,
@@ -630,7 +631,7 @@ function setupConnectionsTracking() {
 }
 
 function httpServerPreClose(server) {
-  server.closeIdleConnections();
+  server.closeIdleConnections(true);
   clearInterval(server[kConnectionsCheckingInterval]);
 }
 
@@ -700,7 +701,20 @@ Server.prototype.closeAllConnections = function closeAllConnections() {
   }
 };
 
-Server.prototype.closeIdleConnections = function closeIdleConnections() {
+/**
+ * Closes every connection that is currently idle (no request being received,
+ * no response in flight or queued). With `closeWhenIdle` set to `true`,
+ * connections that are busy right now are additionally marked to close as
+ * soon as their in-flight work completes, instead of being eligible for
+ * keep-alive reuse - see the `kCloseWhenIdle` check in `resOnFinish()`.
+ * Without this mark, such a connection would sit open until
+ * `keepAliveTimeout` reaps it, well after this sweep ran. Defaults to
+ * `false` when omitted.
+ * @param {boolean} [closeWhenIdle]
+ */
+Server.prototype.closeIdleConnections = function closeIdleConnections(closeWhenIdle = false) {
+  validateBoolean(closeWhenIdle, 'closeWhenIdle');
+
   if (!this[kConnections]) {
     return;
   }
@@ -714,6 +728,14 @@ Server.prototype.closeIdleConnections = function closeIdleConnections() {
 
     connections[i].socket.destroy();
   }
+
+  if (closeWhenIdle) {
+    const active = this[kConnections].active();
+
+    for (let i = 0, l = active.length; i < l; i++) {
+      active[i].socket[kCloseWhenIdle] = true;
+    }
+  }
 };
 
 Server.prototype.setTimeout = function setTimeout(msecs, callback) {
@@ -1228,7 +1250,7 @@ function resOnFinish(req, res, socket, state, server) {
   clearIncoming(req);
   process.nextTick(emitCloseNT, res);
 
-  if (res._last) {
+  if (res._last || (socket[kCloseWhenIdle] && state.outgoing.length === 0)) {
     if (typeof socket.destroySoon === 'function') {
       socket.destroySoon();
     } else {

lib/internal/http.js

@@ -268,6 +268,7 @@ function getGlobalAgent(proxyEnv, Agent) {
 module.exports = {
   kOutHeaders: Symbol('kOutHeaders'),
   kNeedDrain: Symbol('kNeedDrain'),
+  kCloseWhenIdle: Symbol('http.server.closeWhenIdle'),
   kProxyConfig: Symbol('kProxyConfig'),
   kWaitForProxyTunnel: Symbol('kWaitForProxyTunnel'),
   checkShouldUseProxy,

test/parallel/test-http-server-close-when-idle.js

@@ -0,0 +1,62 @@
+'use strict';
+
+// This tests that `server.close()` marks an active connection (mid-request
+// or awaiting a response) to close once its in-flight work completes,
+// instead of leaving it open for keep-alive reuse until `keepAliveTimeout`.
+// It also checks the response then advertises `Connection: close` instead
+// of `keep-alive`.
+
+const common = require('../common');
+const assert = require('assert');
+
+const { createServer, get, Agent } = require('http');
+
+const agent = new Agent({ keepAlive: true });
+
+let failTimer;
+
+const server = createServer(
+  { keepAliveTimeout: 2000 },
+  common.mustCall((req, res) => {
+    req.resume();
+
+    failTimer = setTimeout(() => {
+      assert.fail(
+        `expected server.close() to finish within 1000ms of the response completing,` +
+          ` but it seems to be waiting for the keepAliveTimeout to elapse`,
+      );
+    }, common.platformTimeout(1000));
+
+    server.close(
+      common.mustCall(() => {
+        clearTimeout(failTimer);
+      }),
+    );
+
+    setTimeout(
+      common.mustCall(() => res.end('ok')),
+      common.platformTimeout(500),
+    );
+  }),
+);
+
+server.listen(
+  0,
+  common.mustCall(() => {
+    const port = server.address().port;
+
+    get(
+      { port, agent },
+      common.mustCall((res) => {
+        assert.strictEqual(res.headers.connection, 'close');
+
+        let body = '';
+        res.on('data', (chunk) => (body += chunk));
+        res.on(
+          'end',
+          common.mustCall(() => assert.strictEqual(body, 'ok')),
+        );
+      }),
+    );
+  }),
+);