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

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: nodejs#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>

Author: pimterry

doc/api/http.md

@@ -957,7 +957,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.
 

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') {

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