From bcbf1d7cbd0f2ccc5ed152d04be7e47ac8d6e370 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 12 Apr 2020 22:40:58 +0200 Subject: [PATCH 1/4] http: refactor agent 'free' handler Remove nesting in favor of early returns. --- lib/_http_agent.js | 82 ++++++++++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 66e8632c9d1285..41391469030156 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -94,52 +94,54 @@ function Agent(options) { // case of socket.destroy() below this 'error' has no handler // and could cause unhandled exception. - if (socket.writable && - this.requests[name] && this.requests[name].length) { + if (!socket.writable) { + socket.destroy(); + return; + } + + if (this.requests[name] && this.requests[name].length) { const req = this.requests[name].shift(); setRequestSocket(this, req, socket); if (this.requests[name].length === 0) { - // don't leak delete this.requests[name]; } - } else { - // If there are no pending requests, then put it in - // the freeSockets pool, but only if we're allowed to do so. - const req = socket._httpMessage; - if (req && - req.shouldKeepAlive && - socket.writable && - this.keepAlive) { - let freeSockets = this.freeSockets[name]; - const freeLen = freeSockets ? freeSockets.length : 0; - let count = freeLen; - if (this.sockets[name]) - count += this.sockets[name].length; - - if (count > this.maxSockets || freeLen >= this.maxFreeSockets) { - socket.destroy(); - } else if (this.keepSocketAlive(socket)) { - freeSockets = freeSockets || []; - this.freeSockets[name] = freeSockets; - socket[async_id_symbol] = -1; - socket._httpMessage = null; - this.removeSocket(socket, options); - - const agentTimeout = this.options.timeout || 0; - if (socket.timeout !== agentTimeout) { - socket.setTimeout(agentTimeout); - } - - socket.once('error', freeSocketErrorListener); - freeSockets.push(socket); - } else { - // Implementation doesn't want to keep socket alive - socket.destroy(); - } - } else { - socket.destroy(); - } + return; + } + + // If there are no pending requests, then put it in + // the freeSockets pool, but only if we're allowed to do so. + const req = socket._httpMessage; + if (!req || !req.shouldKeepAlive || !this.keepAlive) { + socket.destroy(); + return; + } + + let freeSockets = this.freeSockets[name]; + const freeLen = freeSockets ? freeSockets.length : 0; + let count = freeLen; + if (this.sockets[name]) + count += this.sockets[name].length; + + if (count > this.maxSockets || + freeLen >= this.maxFreeSockets || + !this.keepSocketAlive(socket)) { + socket.destroy(); + return; } + + freeSockets = freeSockets || []; + this.freeSockets[name] = freeSockets; + socket[async_id_symbol] = -1; + socket._httpMessage = null; + this.removeSocket(socket, options); + + const agentTimeout = this.options.timeout || 0; + if (socket.timeout !== agentTimeout) { + socket.setTimeout(agentTimeout); + } + + socket.once('error', freeSocketErrorListener); + freeSockets.push(socket); }); // Don't emit keylog events unless there is a listener for them. From b0c8a22f88cde6d8fe5a12711c0e7c3f09df3e8f Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 12 Apr 2020 22:45:48 +0200 Subject: [PATCH 2/4] http: agent use once helper --- lib/_http_agent.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 41391469030156..ff27cba0087d3b 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -37,6 +37,8 @@ const { ERR_INVALID_ARG_TYPE, }, } = require('internal/errors'); +const { once } = require('internal/util'); + const kOnKeylog = Symbol('onkeylog'); // New Agent code. @@ -268,12 +270,8 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) { debug('createConnection', name, options); options.encoding = null; - let called = false; - const oncreate = (err, s) => { - if (called) - return; - called = true; + const oncreate = once((err, s) => { if (err) return cb(err); if (!this.sockets[name]) { @@ -283,7 +281,7 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) { debug('sockets', name, this.sockets[name].length); installListeners(this, s, options); cb(null, s); - }; + }); const newSocket = this.createConnection(options, oncreate); if (newSocket) From f88a1850cd043933c5df143f56434316bcfaba4f Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 14 Apr 2020 13:00:52 +0200 Subject: [PATCH 3/4] Update lib/_http_agent.js Co-Authored-By: Denys Otrishko --- lib/_http_agent.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index ff27cba0087d3b..08170ed03af722 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -101,10 +101,13 @@ function Agent(options) { return; } - if (this.requests[name] && this.requests[name].length) { - const req = this.requests[name].shift(); + const requests = this.requests[name]; + if (requests && requests.length) { + const req = requests.shift(); setRequestSocket(this, req, socket); - if (this.requests[name].length === 0) { + if (requests.length === 0) { + delete this.requests[name]; + } delete this.requests[name]; } return; From 41af730231a0c7641d930cb29664c1acf9c31745 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 14 Apr 2020 15:53:21 +0200 Subject: [PATCH 4/4] fixup --- lib/_http_agent.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 08170ed03af722..2618c6c3cb8223 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -107,8 +107,6 @@ function Agent(options) { setRequestSocket(this, req, socket); if (requests.length === 0) { delete this.requests[name]; - } - delete this.requests[name]; } return; }