From db66601dbb01231651706cd564a994ddb9b537f0 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Wed, 24 Jul 2024 03:50:40 +0200 Subject: [PATCH 1/2] fix: Improve timeout handling during event loop lag Make timeouts behave more closer that what is expected during event loop lag. --- lib/dispatcher/client-h1.js | 14 ++++++++++++++ lib/util/timers.js | 4 ++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js index 2f1c96724d3..e0883663685 100644 --- a/lib/dispatcher/client-h1.js +++ b/lib/dispatcher/client-h1.js @@ -158,6 +158,7 @@ class Parser { this.bytesRead = 0 + this.timeoutDeadline = 0 this.keepAlive = '' this.contentLength = '' this.connection = '' @@ -170,6 +171,8 @@ class Parser { timers.clearTimeout(this.timeout) if (value) { this.timeout = timers.setTimeout(onParserTimeout, value, this) + this.timeoutDeadline = this.timeoutValue ? Date.now() + this.timeoutValue : 0 + // istanbul ignore else: only for jest if (this.timeout.unref) { this.timeout.unref() @@ -182,10 +185,17 @@ class Parser { // istanbul ignore else: only for jest if (this.timeout.refresh) { this.timeout.refresh() + this.timeoutDeadline = this.timeoutValue ? Date.now() + this.timeoutValue : 0 } } } + updateTimeout () { + if (this.timeoutDeadline && this.timeoutDeadline < Date.now()) { + onParserTimeout(this) + } + } + resume () { if (this.socket.destroyed || !this.paused) { return @@ -615,6 +625,8 @@ class Parser { function onParserTimeout (parser) { const { socket, timeoutType, client } = parser + parser.timeoutDeadline = 0 + /* istanbul ignore else */ if (timeoutType === TIMEOUT_HEADERS) { if (!socket[kWriting] || socket.writableNeedDrain || client[kRunning] > 1) { @@ -815,6 +827,8 @@ function resumeH1 (client) { socket[kParser].setTimeout(headersTimeout, TIMEOUT_HEADERS) } } + + socket[kParser].updateTimeout() } } diff --git a/lib/util/timers.js b/lib/util/timers.js index d0091cc15f7..fa56be8b10e 100644 --- a/lib/util/timers.js +++ b/lib/util/timers.js @@ -2,13 +2,13 @@ const TICK_MS = 499 -let fastNow = Date.now() +let fastNow = 0 let fastNowTimeout const fastTimers = [] function onTimeout () { - fastNow = Date.now() + fastNow += TICK_MS let len = fastTimers.length let idx = 0 From afe3d6edfca018e450d39fc960ceb782e5bcd09a Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Wed, 24 Jul 2024 13:48:04 +0200 Subject: [PATCH 2/2] fixup --- lib/core/connect.js | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/lib/core/connect.js b/lib/core/connect.js index b388f022298..65fb4d37d1e 100644 --- a/lib/core/connect.js +++ b/lib/core/connect.js @@ -4,6 +4,7 @@ const net = require('node:net') const assert = require('node:assert') const util = require('./util') const { InvalidArgumentError, ConnectTimeoutError } = require('./errors') +const timers = require('../util/timers') let tls // include tls conditionally since it is not always available @@ -130,12 +131,12 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess socket.setKeepAlive(true, keepAliveInitialDelay) } - const cancelTimeout = setupTimeout(() => onConnectTimeout(socket), timeout) + const connectTimeout = timers.setTimeout(onConnectTimeout, timeout, socket) socket .setNoDelay(true) .once(protocol === 'https:' ? 'secureConnect' : 'connect', function () { - cancelTimeout() + timers.clearTimeout(connectTimeout) if (callback) { const cb = callback @@ -144,7 +145,7 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess } }) .on('error', function (err) { - cancelTimeout() + timers.clearTimeout(connectTimeout) if (callback) { const cb = callback @@ -157,31 +158,6 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess } } -function setupTimeout (onConnectTimeout, timeout) { - if (!timeout) { - return () => {} - } - - let s1 = null - let s2 = null - const timeoutId = setTimeout(() => { - // setImmediate is added to make sure that we prioritize socket error events over timeouts - s1 = setImmediate(() => { - if (process.platform === 'win32') { - // Windows needs an extra setImmediate probably due to implementation differences in the socket logic - s2 = setImmediate(() => onConnectTimeout()) - } else { - onConnectTimeout() - } - }) - }, timeout) - return () => { - clearTimeout(timeoutId) - clearImmediate(s1) - clearImmediate(s2) - } -} - function onConnectTimeout (socket) { let message = 'Connect Timeout Error' if (Array.isArray(socket.autoSelectFamilyAttemptedAddresses)) {