From 170744a22c08572aaaa1311b0810714d006c47a7 Mon Sep 17 00:00:00 2001 From: Barath Date: Thu, 10 Sep 2026 11:50:00 +0530 Subject: [PATCH] http: don't destroy socket after request completes Aborting a ClientRequest after the request has finished sending and the response has fully arrived has nothing left to cancel. destroy() still called socket.destroy(err), but the resulting 'error' is emitted on a later tick. In that window, responseKeepAlive() has already removed socketErrorListener while handing the socket back to the agent's free pool, so the error lands with no listener and crashes the process. Skip the socket destroy when there is nothing left to cancel. This matches the existing behavior of keepAlive: false requests, which already drop any unread buffered response data in this situation. Fixes: https://github.com/nodejs/node/issues/65938 Signed-off-by: Barath --- lib/_http_client.js | 8 +++ ...t-http-client-abort-completed-keepalive.js | 57 +++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 test/parallel/test-http-client-abort-completed-keepalive.js diff --git a/lib/_http_client.js b/lib/_http_client.js index adcacb752e6e..4a9f3311b8ba 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -698,6 +698,14 @@ ClientRequest.prototype.destroy = function destroy(err) { this.res._dump(); } + // Nothing left to cancel: the request was fully sent and the response was + // fully received. Destroying the socket here would emit an error on a + // socket that is already being released to the agent, at which point + // socketErrorListener has been removed and nothing would handle it. + if (this.writableFinished && this.res?.complete) { + return this; + } + this[kError] = err; this.socket?.destroy(err); diff --git a/test/parallel/test-http-client-abort-completed-keepalive.js b/test/parallel/test-http-client-abort-completed-keepalive.js new file mode 100644 index 000000000000..78d2f366bd7c --- /dev/null +++ b/test/parallel/test-http-client-abort-completed-keepalive.js @@ -0,0 +1,57 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +// Aborting a request whose exchange has already completed must not destroy the +// socket that is being released to the agent. socketErrorListener has been +// removed by responseKeepAlive() at that point, so the error would be emitted +// on a socket with no 'error' listener and crash the process. +// Refs: https://github.com/nodejs/node/issues/65938 + +const agent = new http.Agent({ keepAlive: true }); + +const server = http.createServer((req, res) => { + res.end('x'); +}); + +server.listen(0, '127.0.0.1', common.mustCall(() => { + const controller = new AbortController(); + + const req = http.get({ + port: server.address().port, + host: '127.0.0.1', + agent, + signal: controller.signal, + }, common.mustCall(async (res) => { + res.on('error', common.mustNotCall()); + + for await (const chunk of res) { + assert.strictEqual(chunk.length, 1); + assert.strictEqual(res.complete, true); + assert.strictEqual(req.writableFinished, true); + controller.abort(new Error('stop reading')); + break; + } + + // The socket must survive the abort and go back to the pool, and a + // subsequent request must be able to reuse it. + const res2 = await new Promise((resolve, reject) => { + const req2 = http.get({ + port: server.address().port, + host: '127.0.0.1', + agent, + }, resolve); + req2.on('error', reject); + }); + + let body = ''; + for await (const chunk of res2) body += chunk; + assert.strictEqual(body, 'x'); + + agent.destroy(); + server.close(); + })); + + req.on('error', common.mustNotCall()); +}));