Skip to content

Commit 8a09d77

Browse files
UzlopakUzlopak
authored
fix: refactor fast timers, fix UND_ERR_CONNECT_TIMEOUT on event loop blocking (#3495)
* fix: refactor fast timers, fix UND_ERR_CONNECT_TIMEOUT on event loop blocking * also test native timers * improve the commentary of fasttimers * revert changes in client-h1.js * make fasttimers independent from performance.now * less flaky?
1 parent b725457 commit 8a09d77

10 files changed

Lines changed: 714 additions & 160 deletions

File tree

‎benchmarks/timers/compare-timer-getters.mjs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { bench, group, run } from 'mitata'
2+
3+
group('timers', () => {
4+
bench('Date.now()', () => {
5+
Date.now()
6+
})
7+
bench('performance.now()', () => {
8+
performance.now()
9+
})
10+
bench('Math.trunc(performance.now())', () => {
11+
Math.trunc(performance.now())
12+
})
13+
bench('process.uptime()', () => {
14+
process.uptime()
15+
})
16+
})
17+
18+
await run()

‎lib/core/connect.js‎

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const net = require('node:net')
44
const assert = require('node:assert')
55
const util = require('./util')
66
const { InvalidArgumentError, ConnectTimeoutError } = require('./errors')
7+
const timers = require('../util/timers')
78

89
let tls // include tls conditionally since it is not always available
910

@@ -130,12 +131,12 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess
130131
socket.setKeepAlive(true, keepAliveInitialDelay)
131132
}
132133

133-
const cancelTimeout = setupTimeout(() => onConnectTimeout(socket), timeout)
134+
const cancelConnectTimeout = setupConnectTimeout(new WeakRef(socket), timeout)
134135

135136
socket
136137
.setNoDelay(true)
137138
.once(protocol === 'https:' ? 'secureConnect' : 'connect', function () {
138-
cancelTimeout()
139+
cancelConnectTimeout()
139140

140141
if (callback) {
141142
const cb = callback
@@ -144,7 +145,7 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess
144145
}
145146
})
146147
.on('error', function (err) {
147-
cancelTimeout()
148+
cancelConnectTimeout()
148149

149150
if (callback) {
150151
const cb = callback
@@ -157,30 +158,44 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess
157158
}
158159
}
159160

160-
function setupTimeout (onConnectTimeout, timeout) {
161-
if (!timeout) {
162-
return () => {}
163-
}
161+
const setupConnectTimeout = process.platform === 'win32'
162+
? (socket, timeout) => {
163+
if (!timeout) {
164+
return () => { }
165+
}
164166

165-
let s1 = null
166-
let s2 = null
167-
const timeoutId = setTimeout(() => {
168-
// setImmediate is added to make sure that we prioritize socket error events over timeouts
169-
s1 = setImmediate(() => {
170-
if (process.platform === 'win32') {
167+
let s1 = null
168+
let s2 = null
169+
const timer = timers.setTimeout(() => {
170+
// setImmediate is added to make sure that we prioritize socket error events over timeouts
171+
s1 = setImmediate(() => {
171172
// Windows needs an extra setImmediate probably due to implementation differences in the socket logic
172-
s2 = setImmediate(() => onConnectTimeout())
173-
} else {
174-
onConnectTimeout()
173+
s2 = setImmediate(() => onConnectTimeout(socket.deref()))
174+
})
175+
}, timeout)
176+
return () => {
177+
timers.clearTimeout(timer)
178+
clearImmediate(s1)
179+
clearImmediate(s2)
175180
}
176-
})
177-
}, timeout)
178-
return () => {
179-
clearTimeout(timeoutId)
180-
clearImmediate(s1)
181-
clearImmediate(s2)
182-
}
183-
}
181+
}
182+
: (socket, timeout) => {
183+
if (!timeout) {
184+
return () => { }
185+
}
186+
187+
let s1 = null
188+
const timer = timers.setTimeout(() => {
189+
// setImmediate is added to make sure that we prioritize socket error events over timeouts
190+
s1 = setImmediate(() => {
191+
onConnectTimeout(socket.deref())
192+
})
193+
}, timeout)
194+
return () => {
195+
timers.clearTimeout(timer)
196+
clearImmediate(s1)
197+
}
198+
}
184199

185200
function onConnectTimeout (socket) {
186201
let message = 'Connect Timeout Error'

‎lib/dispatcher/client-h1.js‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -162,20 +162,20 @@ class Parser {
162162
this.maxResponseSize = client[kMaxResponseSize]
163163
}
164164

165-
setTimeout (value, type) {
165+
setTimeout (delay, type) {
166166
this.timeoutType = type
167-
if (value !== this.timeoutValue) {
168-
timers.clearTimeout(this.timeout)
169-
if (value) {
170-
this.timeout = timers.setTimeout(onParserTimeout, value, new WeakRef(this))
167+
if (delay !== this.timeoutValue) {
168+
this.timeout && timers.clearTimeout(this.timeout)
169+
if (delay) {
170+
this.timeout = timers.setTimeout(onParserTimeout, delay, new WeakRef(this))
171171
// istanbul ignore else: only for jest
172172
if (this.timeout.unref) {
173173
this.timeout.unref()
174174
}
175175
} else {
176176
this.timeout = null
177177
}
178-
this.timeoutValue = value
178+
this.timeoutValue = delay
179179
} else if (this.timeout) {
180180
// istanbul ignore else: only for jest
181181
if (this.timeout.refresh) {
@@ -286,7 +286,7 @@ class Parser {
286286
this.llhttp.llhttp_free(this.ptr)
287287
this.ptr = null
288288

289-
timers.clearTimeout(this.timeout)
289+
this.timeout && timers.clearTimeout(this.timeout)
290290
this.timeout = null
291291
this.timeoutValue = null
292292
this.timeoutType = null

0 commit comments

Comments
 (0)