From 80ce97f514baf1963f7173ab4002b7086d2309e9 Mon Sep 17 00:00:00 2001 From: Subhi Al Hasan Date: Sun, 24 Oct 2021 13:34:45 +0200 Subject: [PATCH] http: change totalSocketCount only on socket creation/close PR-URL: https://github.com/nodejs/node/pull/40572 Reviewed-By: Matteo Collina Reviewed-By: Voltrex Reviewed-By: Luigi Pinca Reviewed-By: James M Snell Reviewed-By: Minwoo Jung Reviewed-By: Ricky Zhou <0x19951125@gmail.com> --- lib/_http_agent.js | 4 ++-- test/parallel/test-http-agent-keepalive.js | 7 +++++++ test/parallel/test-http-upgrade-agent.js | 8 ++++++-- test/parallel/test-http-upgrade-client.js | 1 - 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index a42c0e83992b1f..87450993a64716 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -284,7 +284,6 @@ Agent.prototype.addRequest = function addRequest(req, options, port/* legacy */, this.reuseSocket(socket, req); setRequestSocket(this, req, socket); ArrayPrototypePush(this.sockets[name], socket); - this.totalSocketCount++; } else if (sockLen < this.maxSockets && this.totalSocketCount < this.maxTotalSockets) { debug('call onSocket', sockLen, freeLen); @@ -383,6 +382,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. + agent.totalSocketCount--; agent.removeSocket(s, options); } s.on('close', onClose); @@ -406,6 +406,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'); + agent.totalSocketCount--; agent.removeSocket(s, options); s.removeListener('close', onClose); s.removeListener('free', onFree); @@ -438,7 +439,6 @@ Agent.prototype.removeSocket = function removeSocket(s, options) { // Don't leak if (sockets[name].length === 0) delete sockets[name]; - this.totalSocketCount--; } } } diff --git a/test/parallel/test-http-agent-keepalive.js b/test/parallel/test-http-agent-keepalive.js index efa82a4d5f0be1..f7424634df1bd5 100644 --- a/test/parallel/test-http-agent-keepalive.js +++ b/test/parallel/test-http-agent-keepalive.js @@ -59,6 +59,7 @@ function checkDataAndSockets(body) { assert.strictEqual(body.toString(), 'hello world'); assert.strictEqual(agent.sockets[name].length, 1); assert.strictEqual(agent.freeSockets[name], undefined); + assert.strictEqual(agent.totalSocketCount, 1); } function second() { @@ -73,6 +74,7 @@ function second() { process.nextTick(common.mustCall(() => { assert.strictEqual(agent.sockets[name], undefined); assert.strictEqual(agent.freeSockets[name].length, 1); + assert.strictEqual(agent.totalSocketCount, 1); remoteClose(); })); })); @@ -91,10 +93,12 @@ function remoteClose() { process.nextTick(common.mustCall(() => { assert.strictEqual(agent.sockets[name], undefined); assert.strictEqual(agent.freeSockets[name].length, 1); + assert.strictEqual(agent.totalSocketCount, 1); // Waiting remote server close the socket setTimeout(common.mustCall(() => { assert.strictEqual(agent.sockets[name], undefined); assert.strictEqual(agent.freeSockets[name], undefined); + assert.strictEqual(agent.totalSocketCount, 0); remoteError(); }), common.platformTimeout(200)); })); @@ -110,10 +114,12 @@ function remoteError() { assert.strictEqual(err.message, 'socket hang up'); assert.strictEqual(agent.sockets[name].length, 1); assert.strictEqual(agent.freeSockets[name], undefined); + assert.strictEqual(agent.totalSocketCount, 1); // Wait socket 'close' event emit setTimeout(common.mustCall(() => { assert.strictEqual(agent.sockets[name], undefined); assert.strictEqual(agent.freeSockets[name], undefined); + assert.strictEqual(agent.totalSocketCount, 0); server.close(); }), common.platformTimeout(1)); })); @@ -132,6 +138,7 @@ server.listen(0, common.mustCall(() => { process.nextTick(common.mustCall(() => { assert.strictEqual(agent.sockets[name], undefined); assert.strictEqual(agent.freeSockets[name].length, 1); + assert.strictEqual(agent.totalSocketCount, 1); second(); })); })); diff --git a/test/parallel/test-http-upgrade-agent.js b/test/parallel/test-http-upgrade-agent.js index 28498819490b4f..d2969522433db1 100644 --- a/test/parallel/test-http-upgrade-agent.js +++ b/test/parallel/test-http-upgrade-agent.js @@ -61,7 +61,12 @@ server.listen(0, '127.0.0.1', common.mustCall(function() { const req = http.request(options); req.end(); + req.on('socket', common.mustCall(function() { + assert.strictEqual(req.agent.totalSocketCount, 1); + })); + req.on('upgrade', common.mustCall(function(res, socket, upgradeHead) { + assert.strictEqual(req.agent.totalSocketCount, 0); let recvData = upgradeHead; socket.on('data', function(d) { recvData += d; @@ -71,14 +76,13 @@ server.listen(0, '127.0.0.1', common.mustCall(function() { assert.strictEqual(recvData.toString(), 'nurtzo'); })); - console.log(res.headers); const expectedHeaders = { 'hello': 'world', 'connection': 'upgrade', 'upgrade': 'websocket' }; assert.deepStrictEqual(expectedHeaders, res.headers); // Make sure this request got removed from the pool. - assert(!(name in http.globalAgent.sockets)); + assert(!(name in req.agent.sockets)); req.on('close', common.mustCall(function() { socket.end(); diff --git a/test/parallel/test-http-upgrade-client.js b/test/parallel/test-http-upgrade-client.js index e86979fffd0d1c..fbeaf08c2c5582 100644 --- a/test/parallel/test-http-upgrade-client.js +++ b/test/parallel/test-http-upgrade-client.js @@ -82,7 +82,6 @@ server.listen(0, common.mustCall(function() { assert.strictEqual(recvData.toString(), expectedRecvData); })); - console.log(res.headers); const expectedHeaders = { hello: 'world', connection: 'upgrade',