fix(ext/napi): wake the event loop at the next uv_timer deadline - #36559
Merged
Conversation
A native `uv_timer_t` started through the N-API/libuv compat layer only fired when some unrelated activity happened to wake the event loop, rather than near its own deadline. With no other wake source a 1s timer could take ~30s to fire. `UvLoopInner` timers are driven by the phase-based event loop: Phase 1 fires every timer whose deadline has passed at the top of each tick. But nothing armed a wakeup *at* the next timer deadline. An active (ref'd) timer keeps the loop alive via `has_uv_alive_handles`, so `poll_event_loop_inner` returns `Poll::Pending`, but the re-wake conditions don't cover uv timers and no sleep is scheduled — so the task is only re-polled when another event fires. In the worst case (a timer as the sole pending work) that was the runtime's ~30s fallback poll. Mirror libuv's `uv__next_timeout`: expose the earliest pending timer deadline from `uv_compat` and, when the loop is otherwise idle, arm a `UserTimer` (the same "wake me at time T" primitive JS timers use) for it. The armed deadline is cached so the sleep is only recreated when the earliest deadline changes, and an already-elapsed deadline re-polls immediately so Phase 1 fires it on the next tick. The `napi uv timer callback fires` test previously passed but took ~30s; it now asserts the callback fires well within its deadline, turning it into a real regression guard (it fails, not just runs slowly, if this regresses). Fixes #36454
MutableSleep::change installed a new timer without clearing a leftover "ready" flag from the previous timer. The next poll_ready would observe that stale flag and force one spurious event-loop re-poll after every re-arm (e.g. each time a repeating uv timer reschedules). Clear the flag when installing a new timer; the immediate-deadline poll below still re-sets it so a due timer fires promptly.
main refactored MutableSleep's fired-state from a Cell<bool> into an atomic field on the shared wake_state. Update the re-arm reset added by this branch to use wake_state.ready.store(false, Release) so it compiles against the merged tree.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #36454.
Problem
A native
uv_timer_tstarted through the N-API / libuv compat layer does not fire near its requested deadline unless some other activity wakes the event loop. With no deliberate wake source, a 1s timer could take ~30s to fire (the reproduction in the issue shows exactly this).The
UvLoopInnertimers are driven by the phase-based event loop inpoll_event_loop_inner: Phase 1 fires every timer whose deadline has already passed, at the top of each tick. But nothing ever armed a wakeup at the next timer's deadline:has_uv_alive_handles, so the poll returnsPoll::Pending(good — the process doesn't exit).When a uv timer is the only pending work, the loop just sits until the runtime's ~30s fallback poll — hence the delay.
This blocks addons that rely on libuv timers firing on time (e.g.
@sentry/profiling-node, which uses a repeatinguv_timerfor sampling).Fix
Mirror libuv's
uv__next_timeout:UvLoopInner::next_timeout()returns the earliest pending timer's absolute deadline and the delay until it fires.poll_event_loop_inner, once the loop is otherwise going to park, arm aUserTimer(the same "wake me at time T" primitive the JS timer subsystem already uses) for that deadline. When the sleep fires, the task is re-polled and Phase 1 fires the timer.Details:
uv_timer_wake_deadline, so the sleep is only recreated when the earliest deadline actually changes (no per-tick churn).BTreeSetborrow + aCellcheck), comparable to thehas_alive_handlescalls already made each tick.Test
The existing
napi uv timer callback firestest passed before but took ~30s. It now recordsperformance.now()around the timer and asserts the callback fires well within its 5ms deadline, so a regression fails loudly instead of merely running slowly.FAILED (30s)—uv timer fired after 30103ms.ok (14–26ms).cargo test -p deno_core(447 tests) is green.