Skip to content

Commit

Permalink
fix: use fasttimers for all connection timeouts
Browse files Browse the repository at this point in the history
  • Loading branch information
Uzlopak committed Sep 5, 2024
1 parent 89a46dd commit 7393ea3
Show file tree
Hide file tree
Showing 4 changed files with 736 additions and 697 deletions.
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.
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

0 comments on commit 7393ea3

Please sign in to comment.