Skip to content

Commit d247a8e

Browse files
ronagTrott
authored andcommitted
http: emit close on socket re-use
PR-URL: #28685 Refs: #28684 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent 90b5f1b commit d247a8e

File tree

3 files changed

+62
-4
lines changed

3 files changed

+62
-4
lines changed

lib/_http_client.js

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,7 @@ function responseKeepAlive(req) {
608608
const asyncId = socket._handle ? socket._handle.getAsyncId() : undefined;
609609
// Mark this socket as available, AFTER user-added end
610610
// handlers have a chance to run.
611-
defaultTriggerAsyncIdScope(asyncId, process.nextTick, emitFreeNT, socket);
611+
defaultTriggerAsyncIdScope(asyncId, process.nextTick, emitFreeNT, req);
612612
}
613613

614614
function responseOnEnd() {
@@ -630,7 +630,7 @@ function responseOnEnd() {
630630
socket.end();
631631
}
632632
assert(!socket.writable);
633-
} else if (req.finished) {
633+
} else if (req.finished && !this.aborted) {
634634
// We can assume `req.finished` means all data has been written since:
635635
// - `'responseOnEnd'` means we have been assigned a socket.
636636
// - when we have a socket we write directly to it without buffering.
@@ -647,8 +647,15 @@ function requestOnPrefinish() {
647647
responseKeepAlive(req);
648648
}
649649

650-
function emitFreeNT(socket) {
651-
socket.emit('free');
650+
function emitFreeNT(req) {
651+
req.emit('close');
652+
if (req.res) {
653+
req.res.emit('close');
654+
}
655+
656+
if (req.socket) {
657+
req.socket.emit('free');
658+
}
652659
}
653660

654661
function tickOnSocket(req, socket) {
@@ -718,6 +725,7 @@ function onSocketNT(req, socket) {
718725
if (!req.agent) {
719726
socket.destroy();
720727
} else {
728+
req.emit('close');
721729
socket.emit('free');
722730
}
723731
} else {
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict';
2+
const common = require('../common');
3+
const http = require('http');
4+
5+
const server = http.createServer(common.mustNotCall());
6+
7+
const keepAliveAgent = new http.Agent({ keepAlive: true });
8+
9+
server.listen(0, common.mustCall(() => {
10+
const req = http.get({
11+
port: server.address().port,
12+
agent: keepAliveAgent
13+
});
14+
15+
req
16+
.on('socket', common.mustNotCall())
17+
.on('response', common.mustNotCall())
18+
.on('close', common.mustCall(() => {
19+
server.close();
20+
keepAliveAgent.destroy();
21+
}))
22+
.abort();
23+
}));
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
const common = require('../common');
3+
const http = require('http');
4+
5+
const server = http.createServer(common.mustCall((req, res) => {
6+
res.end('hello');
7+
}));
8+
9+
const keepAliveAgent = new http.Agent({ keepAlive: true });
10+
11+
server.listen(0, common.mustCall(() => {
12+
const req = http.get({
13+
port: server.address().port,
14+
agent: keepAliveAgent
15+
});
16+
17+
req
18+
.on('response', common.mustCall((res) => {
19+
res
20+
.on('close', common.mustCall(() => {
21+
server.close();
22+
keepAliveAgent.destroy();
23+
}))
24+
.on('data', common.mustCall());
25+
}))
26+
.end();
27+
}));

0 commit comments

Comments
 (0)