From d247a8e1dc398b76558b7eb8cceace911047e912 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 14 Jul 2019 21:45:04 +0200 Subject: [PATCH] http: emit close on socket re-use PR-URL: https://github.com/nodejs/node/pull/28685 Refs: https://github.com/nodejs/node/issues/28684 Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell Reviewed-By: Trivikram Kamat Reviewed-By: Rich Trott --- lib/_http_client.js | 16 ++++++++--- ...est-http-client-agent-abort-close-event.js | 23 ++++++++++++++++ .../test-http-client-agent-end-close-event.js | 27 +++++++++++++++++++ 3 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-http-client-agent-abort-close-event.js create mode 100644 test/parallel/test-http-client-agent-end-close-event.js diff --git a/lib/_http_client.js b/lib/_http_client.js index d55bc850bcb9e5..fcf1cf90fda1c0 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -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() { @@ -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. @@ -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) { @@ -718,6 +725,7 @@ function onSocketNT(req, socket) { if (!req.agent) { socket.destroy(); } else { + req.emit('close'); socket.emit('free'); } } else { diff --git a/test/parallel/test-http-client-agent-abort-close-event.js b/test/parallel/test-http-client-agent-abort-close-event.js new file mode 100644 index 00000000000000..437d2aad222780 --- /dev/null +++ b/test/parallel/test-http-client-agent-abort-close-event.js @@ -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(); +})); diff --git a/test/parallel/test-http-client-agent-end-close-event.js b/test/parallel/test-http-client-agent-end-close-event.js new file mode 100644 index 00000000000000..de3ce193fbba9d --- /dev/null +++ b/test/parallel/test-http-client-agent-end-close-event.js @@ -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(); +}));