From 85b333b8f86d41a3ed4c79f2934e8298e92d5ec6 Mon Sep 17 00:00:00 2001 From: Robert Nagy <ronagy@icloud.com> Date: Sun, 12 Apr 2020 22:40:58 +0200 Subject: [PATCH] http: refactor agent 'free' handler Remove nesting in favor of early returns. PR-URL: https://github.com/nodejs/node/pull/32801 Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> --- lib/_http_agent.js | 97 +++++++++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 48 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 66e8632c9d1285..2618c6c3cb8223 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. @@ -94,52 +96,55 @@ 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) { - const req = this.requests[name].shift(); + if (!socket.writable) { + socket.destroy(); + return; + } + + const requests = this.requests[name]; + if (requests && requests.length) { + const req = requests.shift(); setRequestSocket(this, req, socket); - if (this.requests[name].length === 0) { - // don't leak + if (requests.length === 0) { 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. @@ -266,12 +271,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]) { @@ -281,7 +282,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)