Skip to content

Commit c1b2f6a

Browse files
committed
http: detach socket from IncomingMessage on keep-alive
If the socket is not detached then a future call to res.destroy (through e.g. pipeline) would unecessarily kill the socket while its in the agent free list. PR-URL: #32153 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 30bbeb7 commit c1b2f6a

File tree

3 files changed

+46
-1
lines changed

3 files changed

+46
-1
lines changed

lib/_http_client.js

+6
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,12 @@ function responseKeepAlive(req) {
665665
// Mark this socket as available, AFTER user-added end
666666
// handlers have a chance to run.
667667
defaultTriggerAsyncIdScope(asyncId, process.nextTick, emitFreeNT, req);
668+
669+
if (req.res) {
670+
// Detach socket from IncomingMessage to avoid destroying the freed
671+
// socket in IncomingMessage.destroy().
672+
req.res.socket = null;
673+
}
668674
}
669675

670676
function responseOnEnd() {

test/parallel/test-http-agent-destroyed-socket.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ const server = http.createServer(common.mustCall((req, res) => {
4848
response.on('end', common.mustCall(() => {
4949
request1.socket.destroy();
5050

51-
response.socket.once('close', common.mustCall(() => {
51+
request1.socket.once('close', common.mustCall(() => {
5252
// Assert request2 was removed from the queue
5353
assert(!agent.requests[key]);
5454
process.nextTick(() => {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const http = require('http');
5+
6+
let socketsCreated = 0;
7+
8+
class Agent extends http.Agent {
9+
createConnection(options, oncreate) {
10+
const socket = super.createConnection(options, oncreate);
11+
socketsCreated++;
12+
return socket;
13+
}
14+
}
15+
16+
const server = http.createServer((req, res) => res.end());
17+
18+
server.listen(0, common.mustCall(() => {
19+
const port = server.address().port;
20+
const agent = new Agent({
21+
keepAlive: true,
22+
maxSockets: 1
23+
});
24+
25+
const req = http.get({ agent, port }, common.mustCall((res) => {
26+
res.resume();
27+
res.on('end', () => {
28+
res.destroy();
29+
30+
http.get({ agent, port }, common.mustCall((res) => {
31+
res.resume();
32+
assert.strictEqual(socketsCreated, 1);
33+
agent.destroy();
34+
server.close();
35+
}));
36+
});
37+
}));
38+
req.end();
39+
}));

0 commit comments

Comments
 (0)