Skip to content

Commit

Permalink
http: eliminate capture of ClientRequest in Agent
Browse files Browse the repository at this point in the history
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.

PR-URL: nodejs#10134
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
evantorrie authored and italoacasas committed Jan 24, 2017
1 parent 13b0538 commit d9c9c57
Showing 1 changed file with 30 additions and 26 deletions.
56 changes: 30 additions & 26 deletions lib/_http_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,36 +206,40 @@ 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);
installListeners(self, s, options);
cb(null, s);
}
};

function installListeners(agent, s, options) {
function onFree() {
debug('CLIENT socket onFree');
agent.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.
agent.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');
agent.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);
Expand Down

0 comments on commit d9c9c57

Please sign in to comment.