Skip to content

Commit

Permalink
http: emit close on socket re-use
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
ronag authored and Trott committed Oct 15, 2019
1 parent 90b5f1b commit d247a8e
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 4 deletions.
16 changes: 12 additions & 4 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ function responseKeepAlive(req) {
const asyncId = socket._handle ? socket._handle.getAsyncId() : undefined;
// Mark this socket as available, AFTER user-added end
// handlers have a chance to run.
defaultTriggerAsyncIdScope(asyncId, process.nextTick, emitFreeNT, socket);
defaultTriggerAsyncIdScope(asyncId, process.nextTick, emitFreeNT, req);
}

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

function emitFreeNT(socket) {
socket.emit('free');
function emitFreeNT(req) {
req.emit('close');
if (req.res) {
req.res.emit('close');
}

if (req.socket) {
req.socket.emit('free');
}
}

function tickOnSocket(req, socket) {
Expand Down Expand Up @@ -718,6 +725,7 @@ function onSocketNT(req, socket) {
if (!req.agent) {
socket.destroy();
} else {
req.emit('close');
socket.emit('free');
}
} else {
Expand Down
23 changes: 23 additions & 0 deletions test/parallel/test-http-client-agent-abort-close-event.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';
const common = require('../common');
const http = require('http');

const server = http.createServer(common.mustNotCall());

const keepAliveAgent = new http.Agent({ keepAlive: true });

server.listen(0, common.mustCall(() => {
const req = http.get({
port: server.address().port,
agent: keepAliveAgent
});

req
.on('socket', common.mustNotCall())
.on('response', common.mustNotCall())
.on('close', common.mustCall(() => {
server.close();
keepAliveAgent.destroy();
}))
.abort();
}));
27 changes: 27 additions & 0 deletions test/parallel/test-http-client-agent-end-close-event.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';
const common = require('../common');
const http = require('http');

const server = http.createServer(common.mustCall((req, res) => {
res.end('hello');
}));

const keepAliveAgent = new http.Agent({ keepAlive: true });

server.listen(0, common.mustCall(() => {
const req = http.get({
port: server.address().port,
agent: keepAliveAgent
});

req
.on('response', common.mustCall((res) => {
res
.on('close', common.mustCall(() => {
server.close();
keepAliveAgent.destroy();
}))
.on('data', common.mustCall());
}))
.end();
}));

0 comments on commit d247a8e

Please sign in to comment.