Skip to content

Commit

Permalink
http: change totalSocketCount only on socket creation/close
Browse files Browse the repository at this point in the history
PR-URL: #40572
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
  • Loading branch information
jodevsa authored and danielleadams committed Feb 1, 2022
1 parent d9ae43e commit be53296
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 5 deletions.
4 changes: 2 additions & 2 deletions lib/_http_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -438,7 +439,6 @@ Agent.prototype.removeSocket = function removeSocket(s, options) {
// Don't leak
if (sockets[name].length === 0)
delete sockets[name];
this.totalSocketCount--;
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions test/parallel/test-http-agent-keepalive.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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();
}));
}));
Expand All @@ -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));
}));
Expand All @@ -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));
}));
Expand All @@ -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();
}));
}));
Expand Down
8 changes: 6 additions & 2 deletions test/parallel/test-http-upgrade-agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-http-upgrade-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ server.listen(0, '127.0.0.1', common.mustCall(function() {
assert.strictEqual(recvData.toString(), expectedRecvData);
}));

console.log(res.headers);
const expectedHeaders = {
hello: 'world',
connection: 'upgrade',
Expand Down

0 comments on commit be53296

Please sign in to comment.