From bf0129bbab0295ffec1a0718763d5228d56a3cfc Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Mon, 20 Apr 2026 16:06:08 +0200 Subject: [PATCH 1/2] 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 --- doc/api/http.md | 3 +- lib/_http_client.js | 1 + ...test-http-client-request-listeners-leak.js | 38 +++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-http-client-request-listeners-leak.js diff --git a/doc/api/http.md b/doc/api/http.md index 8ac1ed166103b0..d96dd7de5db553 100644 --- a/doc/api/http.md +++ b/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. diff --git a/lib/_http_client.js b/lib/_http_client.js index c14e899dabbf04..62215fc4c89b70 100644 --- 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 new file mode 100644 index 00000000000000..acc6bcc17521f3 --- /dev/null +++ b/test/parallel/test-http-client-request-listeners-leak.js @@ -0,0 +1,38 @@ +'use strict'; +const common = require('../common'); +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; + + 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(); + } + server.close(); + agent.destroy(); + } + + main(); +})); From fee256b3d640f329f161e7a1bac024e2c26b7665 Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Tue, 21 Apr 2026 18:12:24 +0200 Subject: [PATCH 2/2] Tighten up test assertions after review Signed-off-by: Tim Perry --- .../test-http-client-request-listeners-leak.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-http-client-request-listeners-leak.js b/test/parallel/test-http-client-request-listeners-leak.js index acc6bcc17521f3..789ef6ca7e8011 100644 --- a/test/parallel/test-http-client-request-listeners-leak.js +++ b/test/parallel/test-http-client-request-listeners-leak.js @@ -1,5 +1,6 @@ 'use strict'; const common = require('../common'); +const assert = require('assert'); const http = require('http'); const { defaultMaxListeners } = require('events'); @@ -16,6 +17,14 @@ 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 }); @@ -30,9 +39,12 @@ server.listen(0, common.mustCall(() => { for (let i = 0; i < defaultMaxListeners + 1; i++) { await executeHttpGet(); } + + assert.strictEqual(createSocketCount, 1); + server.close(); agent.destroy(); } - main(); + main().then(common.mustCall()); }));