diff --git a/lib/_http_agent.js b/lib/_http_agent.js index ad8eb227f6b513..1f7989b843c073 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -49,6 +49,9 @@ const { const kOnKeylog = Symbol('onkeylog'); const kRequestOptions = Symbol('requestOptions'); const kRequestAsyncResource = Symbol('requestAsyncResource'); + +// TODO(jazelly): make this configurable +const HTTP_AGENT_KEEP_ALIVE_TIMEOUT_BUFFER = 1000; // New Agent code. // The largest departure from the previous implementation is that @@ -473,6 +476,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 +485,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) - HTTP_AGENT_KEEP_ALIVE_TIMEOUT_BUFFER; + 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 +504,7 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) { socket.setTimeout(agentTimeout); } - return true; + return canKeepSocketAlive; }; Agent.prototype.reuseSocket = function reuseSocket(socket, req) { diff --git a/lib/_http_server.js b/lib/_http_server.js index b6e2ba69bc0648..ef29f4f51410cf 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -182,6 +182,8 @@ const kConnections = Symbol('http.server.connections'); const kConnectionsCheckingInterval = Symbol('http.server.connectionsCheckingInterval'); const HTTP_SERVER_TRACE_EVENT_NAME = 'http.server.request'; +// TODO(jazelly): make this configurable +const HTTP_SERVER_KEEP_ALIVE_TIMEOUT_BUFFER = 1000; class HTTPServerAsyncResource { constructor(type, socket) { @@ -1007,7 +1009,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 + HTTP_SERVER_KEEP_ALIVE_TIMEOUT_BUFFER); state.keepAliveTimeoutSet = true; } } else { diff --git a/test/parallel/test-http-keep-alive-timeout-race-condition.js b/test/parallel/test-http-keep-alive-timeout-race-condition.js new file mode 100644 index 00000000000000..08fa84265baa49 --- /dev/null +++ b/test/parallel/test-http-keep-alive-timeout-race-condition.js @@ -0,0 +1,38 @@ +'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); + // Block the event loop for 2 seconds + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 2000); + await makeRequest(this.address().port, agent); + server.close(); + agent.destroy(); +}));