Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
22 changes: 19 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,20 @@ 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.
Comment thread
robobun marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
fn tick_concurrent_unless_due(&mut self) {
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
/// underlying event loop's `active` counter
pub fn has_pending_refs(&self) -> bool {
Expand Down Expand Up @@ -620,7 +636,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 +647,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
80 changes: 80 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,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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

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);
});
Loading