Skip to content

Commit

Permalink
net: reduce likelihood of race conditions on keep-alive timeout calcu…
Browse files Browse the repository at this point in the history
…lation between http1.1 servers and clients

reduced likelihood of race conditions on keep-alive timeout calculation between http1.1 servers and clients and honor server keep-alive timeout when agentTimeout is not set

Fixes: nodejs#47130
Fixes: nodejs#52649
  • Loading branch information
zanettea authored and Zanette Arrigo committed Apr 24, 2024
1 parent 5548928 commit 2124b55
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
14 changes: 10 additions & 4 deletions lib/_http_agent.js
Expand Up @@ -488,6 +488,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'];
Expand All @@ -496,9 +497,14 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) {
const hint = RegExpPrototypeExec(/^timeout=(\d+)/, keepAliveHint)?.[1];

if (hint) {
const serverHintTimeout = NumberParseInt(hint) * 1000;

if (serverHintTimeout < agentTimeout) {
// Let the timer expires 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 (!agentTimeout || serverHintTimeout < agentTimeout) {
agentTimeout = serverHintTimeout;
}
}
Expand All @@ -509,7 +515,7 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) {
socket.setTimeout(agentTimeout);
}

return true;
return canKeepSocketAlive;
};

Agent.prototype.reuseSocket = function reuseSocket(socket, req) {
Expand Down
4 changes: 3 additions & 1 deletion lib/_http_server.js
Expand Up @@ -1010,7 +1010,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 likeliwood of ECONNRESET errors
// due to race conditions between the client and server timeout calculation
socket.setTimeout(server.keepAliveTimeout + 1000);
state.keepAliveTimeoutSet = true;
}
} else {
Expand Down

0 comments on commit 2124b55

Please sign in to comment.