http: fix leaked error listener on sync HTTP req create + destroy

pimterry · aduh95 · commit 64f15c274ac8 · 2026-05-07T15:41:46.000+02:00
Previously, this resulted in an error listener being added but not cleaned up when the socket was freed back to the agent pool. This also updates the docs to clarify when req.destroy() kills the underlying socket - only if it's called during request processing (more specifically: after onSocketNT, at which point we may have started reading & writing). Signed-off-by: Tim Perry <pimterry@gmail.com> PR-URL: #62872 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
diff --git a/doc/api/http.md b/doc/api/http.md
@@ -954,7 +954,8 @@ changes:
 
 Destroy the request. Optionally emit an `'error'` event,
 and emit a `'close'` event. Calling this will cause remaining data
-in the response to be dropped and the socket to be destroyed.
+in the response to be dropped, and the socket to be destroyed if used,
+or returned to the corresponding Agent pool otherwise if possible.
 
 See [`writable.destroy()`][] for further details.
 
diff --git a/lib/_http_client.js b/lib/_http_client.js
@@ -999,6 +999,7 @@ function onSocketNT(req, socket, err) {
     if (socket) {
       if (!err && req.agent && !socket.destroyed) {
         socket.emit('free');
+        socket.removeListener('error', socketErrorListener);
       } else {
         finished(socket.destroy(err || req[kError]), (er) => {
           if (er?.code === 'ERR_STREAM_PREMATURE_CLOSE') {
diff --git a/test/parallel/test-http-client-request-listeners-leak.js b/test/parallel/test-http-client-request-listeners-leak.js
@@ -0,0 +1,50 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+const { defaultMaxListeners } = require('events');
+
+// Immediately destroying an HTTP request must not leak listeners on the
+// freed socket. When sockets are reused via a keep-alive agent leaked
+// listeners would accumulate to trigger a MaxListenersExceededWarning.
+
+// Check we don't get a MaxListenersExceededWarning:
+process.on('warning', common.mustNotCall('Unexpected warning emitted'));
+
+const server = http.createServer(common.mustNotCall());
+
+server.listen(0, common.mustCall(() => {
+  const agent = new http.Agent({ keepAlive: true });
+  const port = server.address().port;
+
+  // Count actual socket creations to confirm reuse:
+  let createSocketCount = 0;
+  const origCreateSocket = agent.createSocket.bind(agent);
+  agent.createSocket = function(...args) {
+    createSocketCount++;
+    return origCreateSocket(...args);
+  };
+
+  function executeHttpGet() {
+    return new Promise((resolve) => {
+      const req = http.get({ host: '127.0.0.1', port, agent });
+      req.on('error', resolve);
+      req.on('close', resolve);
+      req.on('response', common.mustNotCall());
+      req.destroy();
+    });
+  }
+
+  async function main() {
+    for (let i = 0; i < defaultMaxListeners + 1; i++) {
+      await executeHttpGet();
+    }
+
+    assert.strictEqual(createSocketCount, 1);
+
+    server.close();
+    agent.destroy();
+  }
+
+  main().then(common.mustCall());
+}));
