From 8bfe887486bae4bab2dffefe204a0bd65170273a Mon Sep 17 00:00:00 2001 From: Dave Date: Sun, 6 Dec 2015 04:55:02 -0800 Subject: [PATCH] HTTP: remove excess calls to removeSocket 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 https://github.com/nodejs/node/issues/4050. Let's get rid of the extra calls to removeSocket so we only call it once per completed request. --- lib/_http_agent.js | 2 - ...test-http-agent-maxsockets-regress-4050.js | 43 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-http-agent-maxsockets-regress-4050.js diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 305baa2cbd2f03..c6e3ef63bd72af 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -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 || []; @@ -78,7 +77,6 @@ function Agent(options) { freeSockets.push(socket); } } else { - self.removeSocket(socket, options); socket.destroy(); } } diff --git a/test/parallel/test-http-agent-maxsockets-regress-4050.js b/test/parallel/test-http-agent-maxsockets-regress-4050.js new file mode 100644 index 00000000000000..1cbe37cf0cc443 --- /dev/null +++ b/test/parallel/test-http-agent-maxsockets-regress-4050.js @@ -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(); + } + }); + } +});