http: reduce likelihood of race conditions on keep-alive timeout

Fixes: #52649
Refs: #54293

Co-authored-by: Zanette Arrigo <[email protected]>

Author: jazelly

lib/_http_agent.js

@@ -473,6 +473,7 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) {
   socket.unref();
 
   let agentTimeout = this.options.timeout || 0;
+  let canKeepSocketAlive = true;
 
   if (socket._httpMessage?.res) {
     const keepAliveHint = socket._httpMessage.res.headers['keep-alive'];
@@ -481,9 +482,15 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) {
       const hint = /^timeout=(\d+)/.exec(keepAliveHint)?.[1];
 
       if (hint) {
-        const serverHintTimeout = NumberParseInt(hint) * 1000;
-
-        if (serverHintTimeout < agentTimeout) {
+        // Let the timer expire before the announced timeout to reduce
+        // the likelihood of ECONNRESET errors
+        let serverHintTimeout = (NumberParseInt(hint) * 1000) - 1000;
+        serverHintTimeout = serverHintTimeout > 0 ? serverHintTimeout : 0;
+        if (serverHintTimeout === 0) {
+          // Cannot safely reuse the socket because the server timeout is
+          // too short
+          canKeepSocketAlive = false;
+        } else if (serverHintTimeout < agentTimeout) {
           agentTimeout = serverHintTimeout;
         }
       }
@@ -494,7 +501,7 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) {
     socket.setTimeout(agentTimeout);
   }
 
-  return true;
+  return canKeepSocketAlive;
 };
 
 Agent.prototype.reuseSocket = function reuseSocket(socket, req) {

lib/_http_server.js

@@ -1007,7 +1007,9 @@ function resOnFinish(req, res, socket, state, server) {
     }
   } else if (state.outgoing.length === 0) {
     if (server.keepAliveTimeout && typeof socket.setTimeout === 'function') {
-      socket.setTimeout(server.keepAliveTimeout);
+      // Increase the internal timeout wrt the advertised value to reduce
+      // the likelihood of ECONNRESET errors.
+      socket.setTimeout(server.keepAliveTimeout + 1000);
       state.keepAliveTimeoutSet = true;
     }
   } else {

test/parallel/test-http-keep-alive-timeout-race-condition.js

@@ -0,0 +1,39 @@
+'use strict';
+
+const common = require('../common');
+const http = require('http');
+
+const makeRequest = (port, agent) =>
+  new Promise((resolve, reject) => {
+    const req = http.get(
+      { path: '/', port, agent },
+      (res) => {
+        res.resume();
+        res.on('end', () => resolve());
+      },
+    );
+    req.on('error', (e) => reject(e));
+    req.end();
+  });
+
+const server = http.createServer(
+  { keepAliveTimeout: common.platformTimeout(2000), keepAlive: true },
+  common.mustCall((req, res) => {
+    const body = 'hello world\n';
+    res.writeHead(200, { 'Content-Length': body.length });
+    res.write(body);
+    res.end();
+  }, 2)
+);
+
+const agent = new http.Agent({ maxSockets: 5, keepAlive: true });
+
+server.listen(0, common.mustCall(async function() {
+  await makeRequest(this.address().port, agent);
+  const timestamp = Date.now();
+  // Block the event loop for 2 seconds
+  while (Date.now() < timestamp + 2000);
+  await makeRequest(this.address().port, agent);
+  server.close();
+  agent.destroy();
+}));