Repository: nodejs/node
Pull request: http: change totalSocketCount only on socket creation or when the socket is closed

 #40452
<!--
Before submitting a pull request, please read
https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md.

Commit message formatting guidelines:
https://github.com/nodejs/node/blob/HEAD/doc/guides/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: lib/_http_agent.js
patch: @@ -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--;
       }
     }
   }

filename: test/parallel/test-http-agent-keepalive.js
patch: @@ -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();
       }));
     }));

filename: test/parallel/test-http-upgrade-agent.js
patch: @@ -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();

filename: test/parallel/test-http-upgrade-client.js
patch: @@ -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',