From 95c00345b417e12cf00999966d27be9884e160e7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 30 Jul 2026 18:05:38 +0000 Subject: [PATCH 1/6] event_loop: yield mid-tick concurrent re-drain to due timers and immediates EventLoop::tick() re-drained concurrent_tasks (thread-pool completions) inside its inner drain loop, so a chain of fs.read callbacks that each submit the next read ran to completion inside one tick() and due setInterval/setTimeout/setImmediate never fired until the chain stopped. Node/libuv delivers thread-pool completions once per poll and runs the timers/check phases between polls, so timers interleave. Gate the three mid-tick tick_concurrent() calls on 'no setImmediate pending and no JS timer due'. The initial tick_concurrent() at the start of tick() stays as the once-per-iteration batch boundary. The due-timer probe peeks the regular heap and reads the clock only when it is non-empty, reached via a link-time extern into bun_runtime (the timer heap lives there). --- src/jsc/event_loop.rs | 24 +++++++- src/runtime/dispatch.rs | 17 ++++++ src/runtime/timer/mod.rs | 20 +++++++ test/js/node/timers/node-timers.test.ts | 80 +++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 3 deletions(-) diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index be5c9fe4a5a3..822bfad2a607 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -191,6 +191,10 @@ unsafe extern "Rust" { /// `WTFTimer::run` — `timer` is an erased `*mut bun_runtime::timer::WTFTimer`. /// Defined in `bun_runtime::dispatch`. Link-time resolved. fn __bun_run_wtf_timer(timer: *mut (), vm: *mut VirtualMachine); + /// `timer::All::has_due_regular_timer` — peek the JS timer heap and + /// report whether its soonest entry is already due. Defined in + /// `bun_runtime::dispatch`. Link-time resolved. + safe fn __bun_has_due_timer() -> bool; /// Tag-specific shutdown release for a queued-but-never-run task. Called /// from `release_queued_tasks_for_shutdown` (after `shutdown_for_exit`, /// before `destructOnExit`) for every entry left in `self.tasks`. @@ -455,6 +459,20 @@ impl EventLoop { let _ = self.tick_concurrent_with_count(); } + /// Re-drain the concurrent queue mid-tick only when nothing is waiting on + /// `tick()` to return. libuv delivers thread-pool completions once per + /// poll and runs the timers/check phases between polls, so Node interleaves + /// due `setInterval`/`setTimeout` (and `setImmediate`) with a chain of + /// `fs.read` callbacks that each submit the next read. The unconditional + /// `tick_concurrent()` at the start of `tick()` is that once-per-iteration + /// batch boundary; re-draining here is a throughput nicety that must yield + /// when a timer is due or an immediate is pending. + fn tick_concurrent_unless_due(&mut self) { + if self.immediate_tasks.is_empty() && !__bun_has_due_timer() { + self.tick_concurrent(); + } + } + /// Check whether refConcurrently has been called but the change has not yet been applied to the /// underlying event loop's `active` counter pub fn has_pending_refs(&self) -> bool { @@ -620,7 +638,7 @@ impl EventLoop { loop { while self.tick_with_count(ctx) > 0 { - self.tick_concurrent(); + self.tick_concurrent_unless_due(); self.global_ref().handle_rejected_promises(); } if self @@ -631,7 +649,7 @@ impl EventLoop { self.entered_event_loop_count -= 1; return; } - self.tick_concurrent(); + self.tick_concurrent_unless_due(); if self.tasks.readable_length() > 0 { continue; } @@ -639,7 +657,7 @@ impl EventLoop { } while self.tick_with_count(ctx) > 0 { - self.tick_concurrent(); + self.tick_concurrent_unless_due(); } self.global_ref().handle_rejected_promises(); diff --git a/src/runtime/dispatch.rs b/src/runtime/dispatch.rs index 3dbffa4cb078..13d7bad019d6 100644 --- a/src/runtime/dispatch.rs +++ b/src/runtime/dispatch.rs @@ -878,6 +878,23 @@ unsafe fn __bun_cancel_pending_immediate( } } +/// `__bun_has_due_timer` body — declared `extern "Rust"` in +/// `bun_jsc::event_loop`. `EventLoop::tick()` polls this to decide whether to +/// keep re-draining `concurrent_tasks` mid-tick or yield so `auto_tick*` can +/// run the timers phase (Node interleaves due timers between thread-pool +/// completion batches; without this a self-feeding `fs.read` chain starves +/// `setInterval`). +#[unsafe(no_mangle)] +fn __bun_has_due_timer() -> bool { + let all = crate::jsc_hooks::timer_all(); + if all.is_null() { + return false; + } + // SAFETY: `all` is the live per-thread `All`; single JS thread, and + // `has_due_regular_timer` only peeks (no heap mutation, no JS re-entry). + unsafe { (*all).has_due_regular_timer() } +} + /// `__bun_run_wtf_timer` body — cast the low-tier erased `*mut ()` to the real /// `crate::timer::WTFTimer` and fire it. /// diff --git a/src/runtime/timer/mod.rs b/src/runtime/timer/mod.rs index fc4cb96665e4..3af7f036602d 100644 --- a/src/runtime/timer/mod.rs +++ b/src/runtime/timer/mod.rs @@ -1013,6 +1013,26 @@ impl All { } } + /// Cheap "is the soonest JS timer already due" probe for + /// `EventLoop::tick()`'s mid-tick yield check. Reads the clock only when + /// the regular heap is non-empty. WTF timers are ignored: taking the + /// `wtf_timers` lock on every inner-loop iteration of `tick()` is not + /// worth it, and the `drain_due_wtf_timers` call at the next + /// `get_timeout`/`drain_timers` is never more than one loop iteration + /// away once `tick()` returns. + pub(crate) fn has_due_regular_timer(&self) -> bool { + let Some(timer) = self.timers.peek() else { + return false; + }; + // SAFETY: `peek` returns a live heap node. + let next = unsafe { &(*timer).next }; + let next = Timespec { + sec: next.sec, + nsec: next.nsec, + }; + !next.greater(&Timespec::now(TimespecMockMode::ForceRealTime)) + } + /// Pop the next due timer. `now` is filled lazily on first call so we /// don't pay for `clock_gettime` when the heap is empty. fn next(&mut self, has_set_now: &mut bool, now: &mut Timespec) -> Option<*mut EventLoopTimer> { diff --git a/test/js/node/timers/node-timers.test.ts b/test/js/node/timers/node-timers.test.ts index 5ba41145a5af..f3e751f9d958 100644 --- a/test/js/node/timers/node-timers.test.ts +++ b/test/js/node/timers/node-timers.test.ts @@ -245,3 +245,83 @@ describe.each(["with", "without"])("setImmediate %s timers running", mode => { it("should defer microtasks when an exception is thrown in an immediate", async () => { expect(await bunRun(["run", path.join(import.meta.dir, "timers-immediate-exception-fixture.js")])).toSpawn(); }); + +test("chained fs.read callbacks yield to due timers", async () => { + // Node/libuv delivers thread-pool completions once per poll and runs the + // timers phase between polls, so a chain of fs.read callbacks that each + // submit the next read interleaves with an overdue setInterval. Bun used to + // keep re-draining thread-pool completions inside one EventLoop::tick(), + // which meant the chain ran to completion before any due timer fired. + const script = ` + const fs = require('fs'); + const fd = fs.openSync(process.execPath, 'r'); + let order = ''; + setInterval(() => { order += 'T'; }, 1).unref(); + setTimeout(() => { + (function go(i) { + if (i >= 20) { + const first = order.indexOf('R'); + const last = order.lastIndexOf('R'); + const between = order.slice(first, last + 1).split('T').length - 1; + console.log(JSON.stringify({ order, between })); + process.exit(0); + } + fs.read(fd, Buffer.alloc(1), 0, 1, 0, () => { + order += 'R'; + go(i + 1); + }); + // Spin so the 1ms interval is overdue and the thread-pool read has + // landed before the event loop decides whether to re-drain mid-tick. + // The spin is wall-clock, not a wait-for-condition: it constructs the + // state the yield check must observe. + const s = Date.now(); while (Date.now() - s < 2); + })(0); + }, 5); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", script], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const out = JSON.parse(stdout.trim()); + // Node fires ~N-1 intervals between N reads with this shape; the fix only + // needs to guarantee the chain yields at all, so assert at least one. + expect({ between: out.between > 0, order: out.order }).toEqual({ between: true, order: out.order }); + expect(exitCode).toBe(0); +}); + +test("chained fs.read callbacks yield to pending setImmediate", async () => { + // Same mid-tick re-drain: a setImmediate queued from the first read + // callback must run before the rest of the chain, matching Node's + // poll/check alternation. + const script = ` + const fs = require('fs'); + const fd = fs.openSync(process.execPath, 'r'); + let order = ''; + setTimeout(() => { + (function go(i) { + if (i >= 20) { console.log(JSON.stringify({ order })); process.exit(0); } + fs.read(fd, Buffer.alloc(1), 0, 1, 0, () => { + order += 'R'; + if (i === 0) setImmediate(() => { order += 'I'; }); + go(i + 1); + }); + const s = Date.now(); while (Date.now() - s < 2); + })(0); + }, 1); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", script], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const out = JSON.parse(stdout.trim()); + // Node: "RIRRRR...". Bun before the fix: "RRRR...RI" (or "RRRR...R" with + // the immediate dropped by process.exit). + expect(out.order.slice(0, 2)).toBe("RI"); + expect(exitCode).toBe(0); +}); From b5d3e9db88d8dc0c632fc79ab03070ba4e6c9cf5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 30 Jul 2026 18:22:03 +0000 Subject: [PATCH 2/6] Guard the due-timer probe on concurrent_tasks being non-empty Avoids a clock_gettime per inner-loop iteration of tick() when the timer heap is non-empty but nothing arrived from other threads (an HTTP server always has DateHeaderTimer armed). The yield decision only matters when there is something to re-drain. --- src/jsc/event_loop.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 822bfad2a607..4aff7a708d87 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -467,10 +467,19 @@ impl EventLoop { /// `tick_concurrent()` at the start of `tick()` is that once-per-iteration /// batch boundary; re-draining here is a throughput nicety that must yield /// when a timer is due or an immediate is pending. + /// + /// The yield probe is guarded on `concurrent_tasks` being non-empty so the + /// clock read in `__bun_has_due_timer` is only paid when there is actually + /// something to re-drain (an HTTP server always has `DateHeaderTimer` + /// armed, so an unguarded probe would add a `clock_gettime` to every + /// inner-loop iteration of `tick()`). fn tick_concurrent_unless_due(&mut self) { - if self.immediate_tasks.is_empty() && !__bun_has_due_timer() { - self.tick_concurrent(); + if !self.concurrent_tasks.is_empty() + && (!self.immediate_tasks.is_empty() || __bun_has_due_timer()) + { + return; } + self.tick_concurrent(); } /// Check whether refConcurrently has been called but the change has not yet been applied to the From a35828d84e6160eefef35942bf180ed406fbbcdd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 30 Jul 2026 18:24:58 +0000 Subject: [PATCH 3/6] Trim doc comments per comment-cop --- src/jsc/event_loop.rs | 23 ++++++----------------- src/runtime/dispatch.rs | 8 ++------ src/runtime/timer/mod.rs | 9 ++------- 3 files changed, 10 insertions(+), 30 deletions(-) diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 4aff7a708d87..295214dd68cc 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -191,9 +191,7 @@ unsafe extern "Rust" { /// `WTFTimer::run` — `timer` is an erased `*mut bun_runtime::timer::WTFTimer`. /// Defined in `bun_runtime::dispatch`. Link-time resolved. fn __bun_run_wtf_timer(timer: *mut (), vm: *mut VirtualMachine); - /// `timer::All::has_due_regular_timer` — peek the JS timer heap and - /// report whether its soonest entry is already due. Defined in - /// `bun_runtime::dispatch`. Link-time resolved. + /// `timer::All::has_due_regular_timer`. Defined in `bun_runtime::dispatch`. safe fn __bun_has_due_timer() -> bool; /// Tag-specific shutdown release for a queued-but-never-run task. Called /// from `release_queued_tasks_for_shutdown` (after `shutdown_for_exit`, @@ -459,20 +457,11 @@ impl EventLoop { let _ = self.tick_concurrent_with_count(); } - /// Re-drain the concurrent queue mid-tick only when nothing is waiting on - /// `tick()` to return. libuv delivers thread-pool completions once per - /// poll and runs the timers/check phases between polls, so Node interleaves - /// due `setInterval`/`setTimeout` (and `setImmediate`) with a chain of - /// `fs.read` callbacks that each submit the next read. The unconditional - /// `tick_concurrent()` at the start of `tick()` is that once-per-iteration - /// batch boundary; re-draining here is a throughput nicety that must yield - /// when a timer is due or an immediate is pending. - /// - /// The yield probe is guarded on `concurrent_tasks` being non-empty so the - /// clock read in `__bun_has_due_timer` is only paid when there is actually - /// something to re-drain (an HTTP server always has `DateHeaderTimer` - /// armed, so an unguarded probe would add a `clock_gettime` to every - /// inner-loop iteration of `tick()`). + /// Mid-tick re-drain, skipped when a `setImmediate` is pending or a JS + /// timer is due so `tick()` returns and `auto_tick*` can run them (Node + /// interleaves timers/check between thread-pool completion batches). The + /// `concurrent_tasks` guard keeps the clock-read probe off the path when + /// nothing arrived. fn tick_concurrent_unless_due(&mut self) { if !self.concurrent_tasks.is_empty() && (!self.immediate_tasks.is_empty() || __bun_has_due_timer()) diff --git a/src/runtime/dispatch.rs b/src/runtime/dispatch.rs index 13d7bad019d6..31a29a22c069 100644 --- a/src/runtime/dispatch.rs +++ b/src/runtime/dispatch.rs @@ -878,12 +878,8 @@ unsafe fn __bun_cancel_pending_immediate( } } -/// `__bun_has_due_timer` body — declared `extern "Rust"` in -/// `bun_jsc::event_loop`. `EventLoop::tick()` polls this to decide whether to -/// keep re-draining `concurrent_tasks` mid-tick or yield so `auto_tick*` can -/// run the timers phase (Node interleaves due timers between thread-pool -/// completion batches; without this a self-feeding `fs.read` chain starves -/// `setInterval`). +/// Declared `extern "Rust"` in `bun_jsc::event_loop`; `tick()`'s mid-tick +/// yield check. #[unsafe(no_mangle)] fn __bun_has_due_timer() -> bool { let all = crate::jsc_hooks::timer_all(); diff --git a/src/runtime/timer/mod.rs b/src/runtime/timer/mod.rs index 3af7f036602d..d70ffb806816 100644 --- a/src/runtime/timer/mod.rs +++ b/src/runtime/timer/mod.rs @@ -1013,13 +1013,8 @@ impl All { } } - /// Cheap "is the soonest JS timer already due" probe for - /// `EventLoop::tick()`'s mid-tick yield check. Reads the clock only when - /// the regular heap is non-empty. WTF timers are ignored: taking the - /// `wtf_timers` lock on every inner-loop iteration of `tick()` is not - /// worth it, and the `drain_due_wtf_timers` call at the next - /// `get_timeout`/`drain_timers` is never more than one loop iteration - /// away once `tick()` returns. + /// `EventLoop::tick()`'s mid-tick yield probe. WTF timers are skipped to + /// avoid the `wtf_timers` lock; they drain at the next `get_timeout`. pub(crate) fn has_due_regular_timer(&self) -> bool { let Some(timer) = self.timers.peek() else { return false; From 4e4bca91cbc8a9b875d318148867e9332187261f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:13:51 +0000 Subject: [PATCH 4/6] Address review: TOCTOU-safe immediate check; use crypto.pbkdf2 for cross-platform test coverage - Check immediate_tasks unconditionally (same-thread, no race) so a thread-pool push landing between concurrent_tasks.is_empty() and pop_batch() cannot be drained ahead of a pending setImmediate. Only the clock-read timer probe stays guarded on concurrent_tasks non-empty. - Switch the two tests from fs.read to crypto.pbkdf2: fs.read on Windows completes via libuv's callback into enqueue_task() (not the concurrent queue) so the original test never reached tick_concurrent_unless_due there. crypto.pbkdf2 goes through AnyTaskJob -> enqueue_task_concurrent on every platform. - Assert on the longest run of consecutive callbacks (<= 3) instead of total interleaves (>= 1). Under CPU contention the unfixed build breaks the chain occasionally so 'between > 0' could false-pass; the longest burst stays >= 9 there while the fixed build is always 1. --- src/jsc/event_loop.rs | 10 +++-- test/js/node/timers/node-timers.test.ts | 60 +++++++++++++------------ 2 files changed, 37 insertions(+), 33 deletions(-) diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 295214dd68cc..257fed3a0d09 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -461,11 +461,13 @@ impl EventLoop { /// timer is due so `tick()` returns and `auto_tick*` can run them (Node /// interleaves timers/check between thread-pool completion batches). The /// `concurrent_tasks` guard keeps the clock-read probe off the path when - /// nothing arrived. + /// nothing arrived; `immediate_tasks` is same-thread so it is checked + /// unconditionally (no TOCTOU against a late cross-thread push). fn tick_concurrent_unless_due(&mut self) { - if !self.concurrent_tasks.is_empty() - && (!self.immediate_tasks.is_empty() || __bun_has_due_timer()) - { + if !self.immediate_tasks.is_empty() { + return; + } + if !self.concurrent_tasks.is_empty() && __bun_has_due_timer() { return; } self.tick_concurrent(); diff --git a/test/js/node/timers/node-timers.test.ts b/test/js/node/timers/node-timers.test.ts index f3e751f9d958..b86d55617ec3 100644 --- a/test/js/node/timers/node-timers.test.ts +++ b/test/js/node/timers/node-timers.test.ts @@ -246,35 +246,37 @@ it("should defer microtasks when an exception is thrown in an immediate", async expect(await bunRun(["run", path.join(import.meta.dir, "timers-immediate-exception-fixture.js")])).toSpawn(); }); -test("chained fs.read callbacks yield to due timers", async () => { +test("chained thread-pool callbacks yield to due timers", async () => { // Node/libuv delivers thread-pool completions once per poll and runs the - // timers phase between polls, so a chain of fs.read callbacks that each - // submit the next read interleaves with an overdue setInterval. Bun used to - // keep re-draining thread-pool completions inside one EventLoop::tick(), - // which meant the chain ran to completion before any due timer fired. + // timers phase between polls, so a chain of callbacks that each submit the + // next job interleaves with an overdue setInterval. Bun used to keep + // re-draining thread-pool completions inside one EventLoop::tick(), so the + // chain ran to completion before any due timer fired. + // + // crypto.pbkdf2 is used (not fs.read) because its completion goes through + // enqueue_task_concurrent on every platform; fs.read on Windows goes through + // libuv's own callback and never reaches the re-drain this test covers. const script = ` - const fs = require('fs'); - const fd = fs.openSync(process.execPath, 'r'); + const crypto = require('crypto'); let order = ''; setInterval(() => { order += 'T'; }, 1).unref(); setTimeout(() => { (function go(i) { if (i >= 20) { - const first = order.indexOf('R'); - const last = order.lastIndexOf('R'); - const between = order.slice(first, last + 1).split('T').length - 1; - console.log(JSON.stringify({ order, between })); + const runs = order.match(/R+/g) || []; + const maxRun = Math.max(0, ...runs.map(r => r.length)); + console.log(JSON.stringify({ order, maxRun })); process.exit(0); } - fs.read(fd, Buffer.alloc(1), 0, 1, 0, () => { + crypto.pbkdf2('a', 'b', 1, 8, 'sha256', () => { order += 'R'; go(i + 1); }); - // Spin so the 1ms interval is overdue and the thread-pool read has + // Spin so the 1ms interval is overdue and the thread-pool job has // landed before the event loop decides whether to re-drain mid-tick. - // The spin is wall-clock, not a wait-for-condition: it constructs the - // state the yield check must observe. - const s = Date.now(); while (Date.now() - s < 2); + // Wall-clock, not wait-for-condition: it constructs the state the + // yield check must observe. + const s = Date.now(); while (Date.now() - s < 3); })(0); }, 5); `; @@ -286,29 +288,29 @@ test("chained fs.read callbacks yield to due timers", async () => { const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); const out = JSON.parse(stdout.trim()); - // Node fires ~N-1 intervals between N reads with this shape; the fix only - // needs to guarantee the chain yields at all, so assert at least one. - expect({ between: out.between > 0, order: out.order }).toEqual({ between: true, order: out.order }); + // Node and fixed Bun yield between every job (max consecutive R's = 1). + // Unfixed Bun runs the whole chain in one tick (max = 20); under CPU load + // the chain breaks up but the longest burst stays well above 3. + expect({ maxRunAtMost3: out.maxRun <= 3, order: out.order }).toEqual({ maxRunAtMost3: true, order: out.order }); expect(exitCode).toBe(0); }); -test("chained fs.read callbacks yield to pending setImmediate", async () => { - // Same mid-tick re-drain: a setImmediate queued from the first read - // callback must run before the rest of the chain, matching Node's - // poll/check alternation. +test("chained thread-pool callbacks yield to pending setImmediate", async () => { + // Same mid-tick re-drain: a setImmediate queued from the first callback + // must run before the rest of the chain, matching Node's poll/check + // alternation. const script = ` - const fs = require('fs'); - const fd = fs.openSync(process.execPath, 'r'); + const crypto = require('crypto'); let order = ''; setTimeout(() => { (function go(i) { if (i >= 20) { console.log(JSON.stringify({ order })); process.exit(0); } - fs.read(fd, Buffer.alloc(1), 0, 1, 0, () => { + crypto.pbkdf2('a', 'b', 1, 8, 'sha256', () => { order += 'R'; if (i === 0) setImmediate(() => { order += 'I'; }); go(i + 1); }); - const s = Date.now(); while (Date.now() - s < 2); + const s = Date.now(); while (Date.now() - s < 3); })(0); }, 1); `; @@ -320,8 +322,8 @@ test("chained fs.read callbacks yield to pending setImmediate", async () => { const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); const out = JSON.parse(stdout.trim()); - // Node: "RIRRRR...". Bun before the fix: "RRRR...RI" (or "RRRR...R" with - // the immediate dropped by process.exit). + // Node: "RIRRRR...". Bun before the fix: "RRRR...R" (immediate dropped by + // process.exit) or "RRRR...RI". expect(out.order.slice(0, 2)).toBe("RI"); expect(exitCode).toBe(0); }); From ee7b86413668be572792ce9dc64fa6227376ef27 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:22:44 +0000 Subject: [PATCH 5/6] Split maintenance from concurrent pop so yield only defers the pop tick_concurrent_with_count is now tick_concurrent_maintenance (ref-count delta, POSIX signal drain, imminent-GC) plus tick_concurrent_pop_batch. tick_concurrent_unless_due always runs the maintenance and gates only the pop, so the yield path no longer defers those side-effects. Also trim the test comments to durable rationale. --- src/jsc/event_loop.rs | 33 ++++++++++++++++--------- test/js/node/timers/node-timers.test.ts | 25 ++++--------------- 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 257fed3a0d09..f464d1773b35 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -457,20 +457,22 @@ impl EventLoop { let _ = self.tick_concurrent_with_count(); } - /// Mid-tick re-drain, skipped when a `setImmediate` is pending or a JS - /// timer is due so `tick()` returns and `auto_tick*` can run them (Node - /// interleaves timers/check between thread-pool completion batches). The - /// `concurrent_tasks` guard keeps the clock-read probe off the path when - /// nothing arrived; `immediate_tasks` is same-thread so it is checked - /// unconditionally (no TOCTOU against a late cross-thread push). + /// Mid-tick re-drain: the ref-count/signal/GC maintenance always runs; + /// only the `concurrent_tasks` pop is skipped when a `setImmediate` is + /// pending or a JS timer is due, so `tick()` returns and `auto_tick*` + /// can run them (Node interleaves timers/check between thread-pool + /// completion batches). `immediate_tasks` is same-thread and checked + /// first so a late cross-thread push cannot slip ahead of it; the + /// `concurrent_tasks` guard keeps the clock read off the path when + /// nothing arrived. fn tick_concurrent_unless_due(&mut self) { - if !self.immediate_tasks.is_empty() { - return; - } - if !self.concurrent_tasks.is_empty() && __bun_has_due_timer() { + self.tick_concurrent_maintenance(); + if !self.immediate_tasks.is_empty() + || (!self.concurrent_tasks.is_empty() && __bun_has_due_timer()) + { return; } - self.tick_concurrent(); + let _ = self.tick_concurrent_pop_batch(); } /// Check whether refConcurrently has been called but the change has not yet been applied to the @@ -492,7 +494,7 @@ impl EventLoop { } } - pub fn tick_concurrent_with_count(&mut self) -> usize { + fn tick_concurrent_maintenance(&mut self) { self.update_counts(); #[cfg(unix)] @@ -507,7 +509,14 @@ impl EventLoop { } self.run_imminent_gc_timer(); + } + + pub fn tick_concurrent_with_count(&mut self) -> usize { + self.tick_concurrent_maintenance(); + self.tick_concurrent_pop_batch() + } + fn tick_concurrent_pop_batch(&mut self) -> usize { let concurrent = self.concurrent_tasks.pop_batch(); let count = concurrent.count; if count == 0 { diff --git a/test/js/node/timers/node-timers.test.ts b/test/js/node/timers/node-timers.test.ts index b86d55617ec3..f2f1e2cba226 100644 --- a/test/js/node/timers/node-timers.test.ts +++ b/test/js/node/timers/node-timers.test.ts @@ -247,15 +247,9 @@ it("should defer microtasks when an exception is thrown in an immediate", async }); test("chained thread-pool callbacks yield to due timers", async () => { - // Node/libuv delivers thread-pool completions once per poll and runs the - // timers phase between polls, so a chain of callbacks that each submit the - // next job interleaves with an overdue setInterval. Bun used to keep - // re-draining thread-pool completions inside one EventLoop::tick(), so the - // chain ran to completion before any due timer fired. - // - // crypto.pbkdf2 is used (not fs.read) because its completion goes through - // enqueue_task_concurrent on every platform; fs.read on Windows goes through - // libuv's own callback and never reaches the re-drain this test covers. + // crypto.pbkdf2 completes via enqueue_task_concurrent on every platform + // (fs.read on Windows goes through libuv's callback and would not exercise + // the mid-tick re-drain path). const script = ` const crypto = require('crypto'); let order = ''; @@ -273,9 +267,7 @@ test("chained thread-pool callbacks yield to due timers", async () => { go(i + 1); }); // Spin so the 1ms interval is overdue and the thread-pool job has - // landed before the event loop decides whether to re-drain mid-tick. - // Wall-clock, not wait-for-condition: it constructs the state the - // yield check must observe. + // landed before the mid-tick re-drain decision. const s = Date.now(); while (Date.now() - s < 3); })(0); }, 5); @@ -288,17 +280,12 @@ test("chained thread-pool callbacks yield to due timers", async () => { const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); const out = JSON.parse(stdout.trim()); - // Node and fixed Bun yield between every job (max consecutive R's = 1). - // Unfixed Bun runs the whole chain in one tick (max = 20); under CPU load - // the chain breaks up but the longest burst stays well above 3. + // Due timers must break the chain every iteration; allow small jitter. expect({ maxRunAtMost3: out.maxRun <= 3, order: out.order }).toEqual({ maxRunAtMost3: true, order: out.order }); expect(exitCode).toBe(0); }); test("chained thread-pool callbacks yield to pending setImmediate", async () => { - // Same mid-tick re-drain: a setImmediate queued from the first callback - // must run before the rest of the chain, matching Node's poll/check - // alternation. const script = ` const crypto = require('crypto'); let order = ''; @@ -322,8 +309,6 @@ test("chained thread-pool callbacks yield to pending setImmediate", async () => const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); const out = JSON.parse(stdout.trim()); - // Node: "RIRRRR...". Bun before the fix: "RRRR...R" (immediate dropped by - // process.exit) or "RRRR...RI". expect(out.order.slice(0, 2)).toBe("RI"); expect(exitCode).toBe(0); }); From 3bb1da96b43b07264cd650963fe7226f0490fff7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:54:38 +0000 Subject: [PATCH 6/6] ci: retrigger