Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions src/jsc/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ 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`. 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`,
/// before `destructOnExit`) for every entry left in `self.tasks`.
Expand Down Expand Up @@ -455,6 +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).
Comment thread
robobun marked this conversation as resolved.
Outdated
fn tick_concurrent_unless_due(&mut self) {
if !self.immediate_tasks.is_empty() {
return;
}
if !self.concurrent_tasks.is_empty() && __bun_has_due_timer() {
return;
}
self.tick_concurrent();
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

/// 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 {
Expand Down Expand Up @@ -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
Expand All @@ -631,15 +649,15 @@ impl EventLoop {
self.entered_event_loop_count -= 1;
return;
}
self.tick_concurrent();
self.tick_concurrent_unless_due();
if self.tasks.readable_length() > 0 {
continue;
}
break;
}

while self.tick_with_count(ctx) > 0 {
self.tick_concurrent();
self.tick_concurrent_unless_due();
}

self.global_ref().handle_rejected_promises();
Expand Down
13 changes: 13 additions & 0 deletions src/runtime/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,19 @@ unsafe fn __bun_cancel_pending_immediate(
}
}

/// Declared `extern "Rust"` in `bun_jsc::event_loop`; `tick()`'s mid-tick
/// yield check.
Comment thread
robobun marked this conversation as resolved.
#[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.
///
Expand Down
15 changes: 15 additions & 0 deletions src/runtime/timer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,21 @@ impl All {
}
}

/// `EventLoop::tick()`'s mid-tick yield probe. WTF timers are skipped to
/// avoid the `wtf_timers` lock; they drain at the next `get_timeout`.
Comment thread
robobun marked this conversation as resolved.
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> {
Expand Down
82 changes: 82 additions & 0 deletions test/js/node/timers/node-timers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,3 +245,85 @@ 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 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.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
//
// 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 crypto = require('crypto');
let order = '';
setInterval(() => { order += 'T'; }, 1).unref();
setTimeout(() => {
(function go(i) {
if (i >= 20) {
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);
}
crypto.pbkdf2('a', 'b', 1, 8, 'sha256', () => {
order += 'R';
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.
const s = Date.now(); while (Date.now() - s < 3);
})(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 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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

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 = '';
setTimeout(() => {
(function go(i) {
if (i >= 20) { console.log(JSON.stringify({ order })); process.exit(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 < 3);
})(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...R" (immediate dropped by
// process.exit) or "RRRR...RI".
expect(out.order.slice(0, 2)).toBe("RI");
expect(exitCode).toBe(0);
});
Loading