Skip to content

Commit 3708eac

Browse files
timers: prevent setTimeout from firing callback early
libuv's uv_now() truncates sub-millisecond time to integer milliseconds. When a timer is scheduled at a fractional millisecond boundary (e.g., real time 100.7ms, uv_now() returns 100), the timer can fire up to 1ms before the requested delay has actually elapsed when measured with high-resolution clocks like process.hrtime() or Date.now(). Add 1ms to the duration passed to uv_timer_start() in Environment::ScheduleTimer to compensate for this truncation. This ensures that setTimeout/setInterval callbacks never fire before the requested delay, at the cost of up to 1ms additional latency. The fix is applied at the C++ boundary (ScheduleTimer) rather than in JavaScript because it covers both scheduling paths: the initial schedule from JS (binding.scheduleTimer) and the rescheduling after timer processing in RunTimers. Fixes: #26578 Signed-off-by: V Govindarajan <vijay.govindarajan91@gmail.com>
1 parent 7d1f1b4 commit 3708eac

2 files changed

Lines changed: 50 additions & 1 deletion

File tree

src/env.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1505,7 +1505,11 @@ void Environment::RequestInterruptFromV8() {
15051505

15061506
void Environment::ScheduleTimer(int64_t duration_ms) {
15071507
if (started_cleanup_) return;
1508-
uv_timer_start(timer_handle(), RunTimers, duration_ms, 0);
1508+
// Add 1ms to compensate for libuv's uv_now() truncating sub-millisecond
1509+
// time. Without this, timers can fire up to 1ms before the requested
1510+
// delay when measured with high-resolution clocks (process.hrtime(),
1511+
// Date.now()). See: https://github.com/nodejs/node/issues/26578
1512+
uv_timer_start(timer_handle(), RunTimers, duration_ms + 1, 0);
15091513
}
15101514

15111515
void Environment::ToggleTimerRef(bool ref) {
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
'use strict';
2+
3+
// This test verifies that setTimeout never fires its callback before the
4+
// requested delay has elapsed, as measured by process.hrtime.bigint().
5+
//
6+
// The bug: libuv's uv_now() truncates sub-millisecond time to integer
7+
// milliseconds. When a timer is scheduled at real time 100.7ms, uv_now()
8+
// returns 100. The timer fires when uv_now() >= 100 + delay, but real
9+
// elapsed time is only delay - 0.7ms. The interaction with setImmediate()
10+
// makes this more likely to occur because it affects when uv_update_time()
11+
// is called.
12+
//
13+
// See: https://github.com/nodejs/node/issues/26578
14+
15+
const common = require('../common');
16+
const assert = require('assert');
17+
18+
const DELAY_MS = 100;
19+
const ITERATIONS = 50;
20+
21+
let completed = 0;
22+
23+
function test() {
24+
const start = process.hrtime.bigint();
25+
26+
setTimeout(common.mustCall(() => {
27+
const elapsed = process.hrtime.bigint() - start;
28+
const elapsedMs = Number(elapsed) / 1e6;
29+
30+
assert(
31+
elapsedMs >= DELAY_MS,
32+
`setTimeout(${DELAY_MS}) fired after only ${elapsedMs.toFixed(3)}ms`
33+
);
34+
35+
completed++;
36+
if (completed < ITERATIONS) {
37+
// Use setImmediate to schedule the next iteration, which is critical
38+
// to reproducing the original bug (the interaction between
39+
// setImmediate and setTimeout affects uv_update_time() timing).
40+
setImmediate(test);
41+
}
42+
}), DELAY_MS);
43+
}
44+
45+
test();

0 commit comments

Comments
 (0)