mirror of
https://github.com/oven-sh/bun
synced 2026-02-02 15:08:46 +00:00
fix(timers): ensure all ready timers fire before setImmediate callbacks
When multiple setTimeout timers are scheduled to fire at the same time (same millisecond), they should all execute before any setImmediate callbacks that were scheduled by those timer callbacks. This matches Node.js behavior. The bug occurred because the timer readiness check in `drainTimers()` used full nanosecond precision, while the timer heap ordering uses millisecond-truncated precision for JavaScript timers. This meant two timers scheduled within the same millisecond could have different nanosecond values, causing one to be considered "in the future" even though both should fire together. The fix aligns the timer readiness check with the heap ordering by truncating to millisecond precision for JavaScript timers. Fixes #26508 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -297,7 +297,7 @@ pub const All = struct {
|
||||
now.* = timespec.now(.allow_mocked_time);
|
||||
has_set_now.* = true;
|
||||
}
|
||||
if (timer.next.greater(now)) {
|
||||
if (timerIsInFuture(timer, now)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -308,6 +308,28 @@ pub const All = struct {
|
||||
return null;
|
||||
}
|
||||
|
||||
/// Returns true if the timer is scheduled for a time in the future.
|
||||
/// For JavaScript timers (setTimeout, setInterval, setImmediate), we collapse
|
||||
/// sub-millisecond precision to match the heap ordering behavior in EventLoopTimer.less().
|
||||
/// This ensures that all timers scheduled for the same millisecond fire together,
|
||||
/// preventing setImmediate callbacks from running between timer callbacks that
|
||||
/// should fire at the same time.
|
||||
fn timerIsInFuture(timer: *const EventLoopTimer, now: *const timespec) bool {
|
||||
const sec_order = std.math.order(timer.next.sec, now.sec);
|
||||
if (sec_order != .eq) return sec_order == .gt;
|
||||
|
||||
// For JavaScript timers, collapse sub-millisecond precision
|
||||
// This matches the behavior in EventLoopTimer.less()
|
||||
var timer_ns = timer.next.nsec;
|
||||
var now_ns = now.nsec;
|
||||
if (timer.jsTimerInternalsFlags() != null) {
|
||||
timer_ns = std.time.ns_per_ms * @divTrunc(timer_ns, std.time.ns_per_ms);
|
||||
now_ns = std.time.ns_per_ms * @divTrunc(now_ns, std.time.ns_per_ms);
|
||||
}
|
||||
|
||||
return timer_ns > now_ns;
|
||||
}
|
||||
|
||||
pub fn drainTimers(this: *All, vm: *VirtualMachine) void {
|
||||
// Set in next().
|
||||
var now: timespec = undefined;
|
||||
|
||||
61
test/regression/issue/26508.test.ts
Normal file
61
test/regression/issue/26508.test.ts
Normal file
@@ -0,0 +1,61 @@
|
||||
import { expect, test } from "bun:test";
|
||||
import { bunEnv, bunExe } from "harness";
|
||||
|
||||
// https://github.com/oven-sh/bun/issues/26508
|
||||
// When multiple setTimeout timers are ready to fire at the same time, Bun should
|
||||
// execute all ready timer callbacks before any setImmediate callbacks that were
|
||||
// scheduled by those timer callbacks. This matches Node.js behavior.
|
||||
test("setImmediate scheduled by timer should run after all ready timers fire", async () => {
|
||||
// Run multiple times to catch the intermittent nature of the bug
|
||||
for (let i = 0; i < 20; i++) {
|
||||
await using proc = Bun.spawn({
|
||||
cmd: [
|
||||
bunExe(),
|
||||
"-e",
|
||||
`
|
||||
let immediateRan = false;
|
||||
let t2Ran = false;
|
||||
|
||||
const t1 = setTimeout(() => {
|
||||
setImmediate(() => {
|
||||
immediateRan = true;
|
||||
// Check after both timer and immediate have run
|
||||
if (!t2Ran) {
|
||||
console.log("FAIL: immediate ran before t2");
|
||||
process.exit(1);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
const t2 = setTimeout(() => {
|
||||
t2Ran = true;
|
||||
if (immediateRan) {
|
||||
console.log("FAIL: immediate ran before second timeout");
|
||||
process.exit(1);
|
||||
}
|
||||
});
|
||||
|
||||
// Force both timers to be scheduled at the same millisecond
|
||||
// by setting them to have the same _idleStart value
|
||||
t2._idleStart = t1._idleStart;
|
||||
`,
|
||||
],
|
||||
env: bunEnv,
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
});
|
||||
|
||||
const [exitCode, stdout, stderr] = await Promise.all([
|
||||
proc.exited,
|
||||
new Response(proc.stdout).text(),
|
||||
new Response(proc.stderr).text(),
|
||||
]);
|
||||
|
||||
if (exitCode !== 0) {
|
||||
console.error(`Iteration ${i} failed:`);
|
||||
console.error("stdout:", stdout);
|
||||
console.error("stderr:", stderr);
|
||||
}
|
||||
expect(exitCode).toBe(0);
|
||||
}
|
||||
}, 60000); // 60 second timeout for spawning 20 subprocesses
|
||||
Reference in New Issue
Block a user