From 3967101032afeccdcc8e3c3e2cd6830977a179b9 Mon Sep 17 00:00:00 2001 From: Evan Torrie Date: Sun, 4 Dec 2016 17:51:51 -0800 Subject: [PATCH 1/4] Eliminate capture of "cb" in createSocket context Move the onFree/onClose/onRemote listeners to a separate function --- lib/_http_agent.js | 58 +++++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 2a07cd25e26dd9..43fc37cafd9d45 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -206,36 +206,42 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) { } self.sockets[name].push(s); debug('sockets', name, self.sockets[name].length); - - function onFree() { - self.emit('free', s, options); - } - s.on('free', onFree); - - function onClose(err) { - debug('CLIENT socket onClose'); - // This is the only place where sockets get removed from the Agent. - // If you want to remove a socket from the pool, just close it. - // All socket errors end in a close event anyway. - self.removeSocket(s, options); - } - s.on('close', onClose); - - function onRemove() { - // We need this function for cases like HTTP 'upgrade' - // (defined by WebSockets) where we need to remove a socket from the - // pool because it'll be locked up indefinitely - debug('CLIENT socket onRemove'); - self.removeSocket(s, options); - s.removeListener('close', onClose); - s.removeListener('free', onFree); - s.removeListener('agentRemove', onRemove); - } - s.on('agentRemove', onRemove); + self._installListeners(s,options); cb(null, s); } }; +Agent.prototype._installListeners = function installListeners(s, options) { + var self = this; + + function onFree() { + debug('CLIENT socket onFree'); + self.emit('free', s, options); + } + s.on('free', onFree); + + function onClose(err) { + debug('CLIENT socket onClose'); + // This is the only place where sockets get removed from the Agent. + // If you want to remove a socket from the pool, just close it. + // All socket errors end in a close event anyway. + self.removeSocket(s, options); + } + s.on('close', onClose); + + function onRemove() { + // We need this function for cases like HTTP 'upgrade' + // (defined by WebSockets) where we need to remove a socket from the + // pool because it'll be locked up indefinitely + debug('CLIENT socket onRemove'); + self.removeSocket(s, options); + s.removeListener('close', onClose); + s.removeListener('free', onFree); + s.removeListener('agentRemove', onRemove); + } + s.on('agentRemove', onRemove); +}; + Agent.prototype.removeSocket = function removeSocket(s, options) { var name = this.getName(options); debug('removeSocket', name, 'writable:', s.writable); From f774e72333275e78e786a2e2dd38bcfd3db11913 Mon Sep 17 00:00:00 2001 From: Evan Torrie Date: Tue, 6 Dec 2016 09:45:32 -0800 Subject: [PATCH 2/4] change to local function instead of prototype --- lib/_http_agent.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 43fc37cafd9d45..a5ed96b8e52924 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -206,13 +206,13 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) { } self.sockets[name].push(s); debug('sockets', name, self.sockets[name].length); - self._installListeners(s,options); + installListeners(self, s, options); cb(null, s); } }; -Agent.prototype._installListeners = function installListeners(s, options) { - var self = this; +function installListeners(agent, s, options) { + var self = agent; function onFree() { debug('CLIENT socket onFree'); From eb5b0d35e20dcb903c4ac59285a2f235bc44918c Mon Sep 17 00:00:00 2001 From: Evan Torrie Date: Tue, 6 Dec 2016 17:01:12 -0800 Subject: [PATCH 3/4] change self to agent --- lib/_http_agent.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index a5ed96b8e52924..214c830ae8db50 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -212,11 +212,10 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) { }; function installListeners(agent, s, options) { - var self = agent; function onFree() { debug('CLIENT socket onFree'); - self.emit('free', s, options); + agent.emit('free', s, options); } s.on('free', onFree); @@ -225,7 +224,7 @@ function installListeners(agent, s, options) { // This is the only place where sockets get removed from the Agent. // If you want to remove a socket from the pool, just close it. // All socket errors end in a close event anyway. - self.removeSocket(s, options); + agent.removeSocket(s, options); } s.on('close', onClose); @@ -234,7 +233,7 @@ function installListeners(agent, s, options) { // (defined by WebSockets) where we need to remove a socket from the // pool because it'll be locked up indefinitely debug('CLIENT socket onRemove'); - self.removeSocket(s, options); + agent.removeSocket(s, options); s.removeListener('close', onClose); s.removeListener('free', onFree); s.removeListener('agentRemove', onRemove); From 518d33d61eb4b71dfcc74ff9785b192b91479e83 Mon Sep 17 00:00:00 2001 From: Evan Torrie Date: Thu, 8 Dec 2016 10:44:52 -0800 Subject: [PATCH 4/4] http: eliminate capture of ClientRequest in Agent Keepalive sockets that are returned to the agent's freesocket pool were previously capturing a reference to the ClientRequest that initiated the request. This commit eliminates that by moving the installation of the socket listeners to a different function. --- lib/_http_agent.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 214c830ae8db50..eebdb242463b5d 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -212,7 +212,6 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) { }; function installListeners(agent, s, options) { - function onFree() { debug('CLIENT socket onFree'); agent.emit('free', s, options); @@ -239,7 +238,7 @@ function installListeners(agent, s, options) { s.removeListener('agentRemove', onRemove); } s.on('agentRemove', onRemove); -}; +} Agent.prototype.removeSocket = function removeSocket(s, options) { var name = this.getName(options);