Skip to content

Commit

Permalink
HTTP: remove excess calls to removeSocket
Browse files Browse the repository at this point in the history
socket.destroy() triggers a 'close' event from the socket which triggers
the onClose handler of HTTPAgent which calls self.removeSocket(). So by
calling self.removeSocket() prior to socket.destroy() we end up with two
calls to self.removeSocket().

If there are pending requests, removeSocket ends up creating a new socket.
So if there are pending requests, each time a request completes, we tear
down one socket and create two more. So the total number of sockets grows
exponentially and without regard for any maxSockets settings. This was
noticed in #4050. Let's get rid of
the extra calls to removeSocket so we only call it once per completed
request.
  • Loading branch information
Dave committed Dec 21, 2015
1 parent 8c35903 commit 8bfe887
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 2 deletions.
2 changes: 0 additions & 2 deletions lib/_http_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ function Agent(options) {
count += self.sockets[name].length;

if (count > self.maxSockets || freeLen >= self.maxFreeSockets) {
self.removeSocket(socket, options);
socket.destroy();
} else {
freeSockets = freeSockets || [];
Expand All @@ -78,7 +77,6 @@ function Agent(options) {
freeSockets.push(socket);
}
} else {
self.removeSocket(socket, options);
socket.destroy();
}
}
Expand Down
43 changes: 43 additions & 0 deletions test/parallel/test-http-agent-maxsockets-regress-4050.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const http = require('http');

const MAX_SOCKETS = 2;

const agent = new http.Agent({
keepAlive: true,
keepAliveMsecs: 1000,
maxSockets: MAX_SOCKETS,
maxFreeSockets: 2
});

const server = http.createServer(function(req, res) {
res.end('hello world');
});

function get(path, callback) {
return http.get({
host: 'localhost',
port: common.PORT,
agent: agent,
path: path
}, callback);
}

server.listen(common.PORT, function() {
var finished = 0;
const num_requests = 6;
for (var i = 0; i < num_requests; i++) {
const request = get('/1', function() {
});
request.on('response', function() {
request.abort();
const sockets = agent.sockets[Object.keys(agent.sockets)[0]];
assert(sockets.length <= MAX_SOCKETS);
if (++finished === num_requests) {
server.close();
}
});
}
});

0 comments on commit 8bfe887

Please sign in to comment.