Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use fasttimers for all connection timeouts #3552

Merged
merged 16 commits into from
Sep 8, 2024
Merged
8 changes: 2 additions & 6 deletions lib/dispatcher/client-h1.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,9 @@ class Parser {
setTimeout (delay, type) {
this.timeoutType = type
if (delay !== this.timeoutValue) {
this.timeout && timers.clearTimeout(this.timeout)
this.timeout && timers.clearFastTimeout(this.timeout)
if (delay) {
this.timeout = timers.setTimeout(onParserTimeout, delay, new WeakRef(this))
// istanbul ignore else: only for jest
if (this.timeout.unref) {
this.timeout.unref()
}
this.timeout = timers.setFastTimeout(onParserTimeout, delay, new WeakRef(this))
} else {
this.timeout = null
}
Expand Down
34 changes: 33 additions & 1 deletion lib/util/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ module.exports = {
* The clearTimeout method cancels an instantiated Timer previously created
* by calling setTimeout.
*
* @param {FastTimer} timeout
* @param {NodeJS.Timeout|FastTimer} timeout
*/
clearTimeout (timeout) {
// If the timeout is a FastTimer, call its own clear method.
Expand All @@ -362,6 +362,31 @@ module.exports = {
nativeClearTimeout(timeout)
}
},
/**
* The setFastTimeout() method sets a fastTimer which executes a function once
* the timer expires.
* @param {Function} callback A function to be executed after the timer
* expires.
* @param {number} delay The time, in milliseconds that the timer should
* wait before the specified function or code is executed.
* @param {*} [arg] An optional argument to be passed to the callback function
* when the timer expires.
* @returns {FastTimer}
*/
setFastTimeout (callback, delay, arg) {
// If the delay is less than or equal to the RESOLUTION_MS value return a
// native Node.js Timer instance.
Uzlopak marked this conversation as resolved.
Show resolved Hide resolved
return new FastTimer(callback, delay, arg)
},
/**
* The clearTimeout method cancels an instantiated FastTimer previously
* created by calling setFastTimeout.
*
* @param {FastTimer} timeout
*/
clearFastTimeout (timeout) {
timeout.clear()
},
/**
* The now method returns the value of the internal fast timer clock.
*
Expand All @@ -370,6 +395,13 @@ module.exports = {
now () {
return fastNow
},
/**
* Trigger the onTick function to process the fastTimers array.
* Exported for testing purposes only.
*/
tick () {
onTick()
},
/**
* Exporting for testing purposes only.
* Marking as deprecated to discourage any use outside of testing.
Expand Down
6 changes: 5 additions & 1 deletion test/issue-3356.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { tspl } = require('@matteo.collina/tspl')
const { test, after } = require('node:test')
const { createServer } = require('node:http')
const { once } = require('node:events')

const { tick: fastTimerTick } = require('../lib/util/timers')
const { fetch, Agent, RetryAgent } = require('..')

test('https://github.com/nodejs/undici/issues/3356', async (t) => {
Expand Down Expand Up @@ -42,6 +42,10 @@ test('https://github.com/nodejs/undici/issues/3356', async (t) => {
const response = await fetch(`http://localhost:${server.address().port}`, {
dispatcher: agent
})

fastTimerTick()
fastTimerTick()

setTimeout(async () => {
try {
t.equal(response.status, 200)
Expand Down
Loading
Loading