From cccd36324460b432451d2fd2bb22dbf54351fae7 Mon Sep 17 00:00:00 2001 From: uzlopak Date: Thu, 5 Sep 2024 19:02:12 +0200 Subject: [PATCH 1/4] fix: use native timeouts for TIMEOUT_IDLE, rename TIMEOUT_IDLE to TIMEOUT_KEEP_ALIVE --- lib/dispatcher/client-h1.js | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js index 3895fc5874f..b5a1ea8bbf7 100644 --- a/lib/dispatcher/client-h1.js +++ b/lib/dispatcher/client-h1.js @@ -131,7 +131,7 @@ let currentBufferPtr = null const TIMEOUT_HEADERS = 1 const TIMEOUT_BODY = 2 -const TIMEOUT_IDLE = 3 +const TIMEOUT_KEEP_ALIVE = 3 class Parser { constructor (client, socket, { exports }) { @@ -165,12 +165,27 @@ class Parser { setTimeout (delay, type) { this.timeoutType = type if (delay !== this.timeoutValue) { - this.timeout && timers.clearFastTimeout(this.timeout) - if (delay) { - this.timeout = timers.setFastTimeout(onParserTimeout, delay, new WeakRef(this)) - } else { + if (this.timeout) { + timers.clearTimeout(this.timeout) this.timeout = null } + + if (delay) { + switch (type) { + case TIMEOUT_HEADERS: + case TIMEOUT_BODY: + // Use fast timers for headers and body to take eventual event loop + // latency into account. + this.timeout = timers.setFastTimeout(onParserTimeout, delay, new WeakRef(this)) + break + case TIMEOUT_KEEP_ALIVE: + // Use native timers to ignore event loop latency for keep-alive + // handling. + this.timeout = setTimeout(onParserTimeout, delay, new WeakRef(this)) + this.timeout.unref() + break + } + } this.timeoutValue = delay } else if (this.timeout) { // istanbul ignore else: only for jest @@ -625,7 +640,7 @@ function onParserTimeout (parser) { if (!paused) { util.destroy(socket, new BodyTimeoutError()) } - } else if (timeoutType === TIMEOUT_IDLE) { + } else if (timeoutType === TIMEOUT_KEEP_ALIVE) { assert(client[kRunning] === 0 && client[kKeepAliveTimeoutValue]) util.destroy(socket, new InformationalError('socket idle timeout')) } @@ -803,8 +818,8 @@ function resumeH1 (client) { } if (client[kSize] === 0) { - if (socket[kParser].timeoutType !== TIMEOUT_IDLE) { - socket[kParser].setTimeout(client[kKeepAliveTimeoutValue], TIMEOUT_IDLE) + if (socket[kParser].timeoutType !== TIMEOUT_KEEP_ALIVE) { + socket[kParser].setTimeout(client[kKeepAliveTimeoutValue], TIMEOUT_KEEP_ALIVE) } } else if (client[kRunning] > 0 && socket[kParser].statusCode < 200) { if (socket[kParser].timeoutType !== TIMEOUT_HEADERS) { From d657050798bf78349a522e44da232ef732cad81f Mon Sep 17 00:00:00 2001 From: uzlopak Date: Thu, 5 Sep 2024 22:09:10 +0200 Subject: [PATCH 2/4] ensure that fast timers and native timers are set properly --- lib/dispatcher/client-h1.js | 46 ++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js index b5a1ea8bbf7..9df266d945c 100644 --- a/lib/dispatcher/client-h1.js +++ b/lib/dispatcher/client-h1.js @@ -129,9 +129,17 @@ let currentBufferRef = null let currentBufferSize = 0 let currentBufferPtr = null -const TIMEOUT_HEADERS = 1 -const TIMEOUT_BODY = 2 -const TIMEOUT_KEEP_ALIVE = 3 +const USE_NATIVE_TIMER = 0 +const USE_FAST_TIMER = 1 + +// Use fast timers for headers and body to take eventual event loop +// latency into account. +const TIMEOUT_HEADERS = 2 | USE_FAST_TIMER +const TIMEOUT_BODY = 4 | USE_FAST_TIMER + +// Use native timers to ignore event loop latency for keep-alive +// handling. +const TIMEOUT_KEEP_ALIVE = 8 | USE_NATIVE_TIMER class Parser { constructor (client, socket, { exports }) { @@ -163,29 +171,29 @@ class Parser { } setTimeout (delay, type) { - this.timeoutType = type - if (delay !== this.timeoutValue) { + // If the existing timer and the new timer are of different timer type + // (fast or native) or have different delay, we need to clear the existing + // timer and set a new one. + if ( + (type & USE_FAST_TIMER) !== (this.timeoutType & USE_FAST_TIMER) || + delay !== this.timeoutValue + ) { + // If a timeout is already set, clear it with clearTimeout of the fast + // timer implementation, as it can clear fast and native timers. if (this.timeout) { timers.clearTimeout(this.timeout) this.timeout = null } if (delay) { - switch (type) { - case TIMEOUT_HEADERS: - case TIMEOUT_BODY: - // Use fast timers for headers and body to take eventual event loop - // latency into account. - this.timeout = timers.setFastTimeout(onParserTimeout, delay, new WeakRef(this)) - break - case TIMEOUT_KEEP_ALIVE: - // Use native timers to ignore event loop latency for keep-alive - // handling. - this.timeout = setTimeout(onParserTimeout, delay, new WeakRef(this)) - this.timeout.unref() - break + if (type & USE_FAST_TIMER) { + this.timeout = timers.setFastTimeout(onParserTimeout, delay, new WeakRef(this)) + } else { + this.timeout = setTimeout(onParserTimeout, delay, new WeakRef(this)) + this.timeout.unref() } } + this.timeoutValue = delay } else if (this.timeout) { // istanbul ignore else: only for jest @@ -193,6 +201,8 @@ class Parser { this.timeout.refresh() } } + + this.timeoutType = type } resume () { From 8bde08ade8eb65ffb8056d7641cd936ee63ea04b Mon Sep 17 00:00:00 2001 From: uzlopak Date: Thu, 5 Sep 2024 22:51:50 +0200 Subject: [PATCH 3/4] . --- lib/dispatcher/client-h1.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js index 9df266d945c..bf8ecfb55fe 100644 --- a/lib/dispatcher/client-h1.js +++ b/lib/dispatcher/client-h1.js @@ -175,8 +175,8 @@ class Parser { // (fast or native) or have different delay, we need to clear the existing // timer and set a new one. if ( - (type & USE_FAST_TIMER) !== (this.timeoutType & USE_FAST_TIMER) || - delay !== this.timeoutValue + delay !== this.timeoutValue || + (type & USE_FAST_TIMER) !== (this.timeoutType & USE_FAST_TIMER) ) { // If a timeout is already set, clear it with clearTimeout of the fast // timer implementation, as it can clear fast and native timers. From 288c7025588784553f1d53fc6f42e35020cddfc4 Mon Sep 17 00:00:00 2001 From: uzlopak Date: Thu, 5 Sep 2024 22:54:24 +0200 Subject: [PATCH 4/4] . --- lib/dispatcher/client-h1.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js index bf8ecfb55fe..cb51ce2fb27 100644 --- a/lib/dispatcher/client-h1.js +++ b/lib/dispatcher/client-h1.js @@ -176,7 +176,7 @@ class Parser { // timer and set a new one. if ( delay !== this.timeoutValue || - (type & USE_FAST_TIMER) !== (this.timeoutType & USE_FAST_TIMER) + (type & USE_FAST_TIMER) ^ (this.timeoutType & USE_FAST_TIMER) ) { // If a timeout is already set, clear it with clearTimeout of the fast // timer implementation, as it can clear fast and native timers.