Move pending timers between the virtual and real event loops#2
Open
spuun wants to merge 2 commits into
Open
Conversation
Instead of raising PendingTimersError when the control block exits with virtual timers still pending, track each pending timer together with the virtual time remaining until it would have fired (wake_at - virtual_now) and re-attach it to the real event loop with that remaining duration. Parked sleeping fibers and select timeouts then wake up later in real time; control returns immediately. Removes PendingTimersError (the abstract Error base remains). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fibers already sleeping or waiting on a select timeout on the real event loop when control begins are now adopted into virtual time, the mirror image of the exit-side re-attach. At control start every execution context's event loop is scanned (Fiber::ExecutionContext.each); sleep and select-timeout events are removed from the real Crystal::EventLoop::Polling timer heap and re-registered on the virtual clock, so they wake when virtual time is advanced past their deadline instead of in real time. Adopted sleeps carry an on_wake proc that marks the real (stack-allocated) sleep event timed out before enqueuing, avoiding the event loop's "manually resumed before the timer expired" guard; this proc is honored on both the advance path and the exit-side re-attach path. Polling builds only (kqueue/epoll). IO-operation timeouts are left on the real loop since they are tied to a live IO wait. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR makes TimeControl.control non-destructive at both boundaries by (1) adopting pre-existing real event-loop sleep/select-timeout timers into virtual time at control entry, and (2) re-attaching any still-pending virtual timers back onto the real event loop at control exit instead of raising an error.
Changes:
- Remove
PendingTimersErrorand track pending timers with remaining time so they can be rescheduled on exit. - Add Polling-only traversal/extraction of real event-loop timers to adopt existing sleeps/select timeouts into the virtual clock.
- Add/adjust specs and docs to cover adoption and exit-side re-attach behavior (including an isolated execution context case).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/time_control/errors.cr |
Removes PendingTimersError (breaking API change). |
src/time_control/core_ext/crystal/event_loop/timers.cr |
Adds read-only traversal over the Polling timer heap for adoption. |
src/time_control/core_ext/crystal/event_loop/polling.cr |
Extracts pending sleep/select-timeout timers from Polling loops and wires adoption across execution contexts. |
src/time_control/context.cr |
Adds on_wake, pending-timer tracking, adoption helpers, and exit-side rescheduling. |
src/time_control.cr |
Wires timer adoption at entry and pending-timer rescheduling at exit. |
spec/time_control_spec.cr |
Replaces pending-timer error spec with re-attach specs; adds adoption specs for sleep/select timeout. |
spec/time_control_mt_spec.cr |
Adds MT coverage for adopting a pre-existing sleep in an isolated execution context. |
README.md |
Documents adoption behavior and the new pending-timer exit semantics. |
AGENTS.md |
Updates architecture notes and public API list to reflect new behavior and removed error class. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+93
to
+98
| # Adopts a fiber that was already sleeping on the real event loop when | ||
| # control started. *wake_at* is the event's real monotonic deadline, which | ||
| # equals the virtual deadline because both clocks share their origin at | ||
| # control start. *on_wake* marks the real (stack-allocated) sleep event | ||
| # timed out before enqueuing the fiber. | ||
| def adopt_sleep(fiber : Fiber, wake_at : Time::Instant, on_wake : Proc(Nil)) : Nil |
Comment on lines
+106
to
+110
| # Adopts a fiber that was already waiting on a `select` timeout on the real | ||
| # event loop when control started. Reuses the native select-timeout wake and | ||
| # cancel paths. | ||
| def adopt_select_timeout(fiber : Fiber, wake_at : Time::Instant) : Nil | ||
| notify = @timers_mutex.synchronize do |
Comment on lines
+178
to
+182
| # Re-attaches any timers that were still pending when control stopped to the | ||
| # real event loop, each with the virtual time that remained until it would | ||
| # have fired. Must be called after control has stopped (i.e. while time is no | ||
| # longer being intercepted) so that `sleep` uses the real event loop. | ||
| def reschedule_pending_timers : Nil |
Comment on lines
105
to
+109
| ensure | ||
| @@context = nil | ||
| ctx.try &.stop | ||
| isolated.try &.wait | ||
| if ctx && ctx.leaked_timer_count > 0 | ||
| raise PendingTimersError.new(ctx.leaked_timer_count) | ||
| end | ||
| ctx.try &.reschedule_pending_timers |
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
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.
Makes
TimeControl.controlnon-destructive at both boundaries: fibers that are mid-sleep(or waiting on aselecttimeout) when control starts are pulled into virtual time, and any that are still pending when control ends are handed back to the real event loop. Previously, leftover timers raisedPendingTimersError, and fibers already sleeping beforecontrolbegan were invisible to the virtual clock (they woke in real wall-clock time).Exit side — re-attach pending timers to the real event loop
Instead of raising
PendingTimersErrorwhen the block exits with timers still pending, each pending timer is re-attached to the real event loop with the virtual time that remained until it would have fired (wake_at - virtual_now). The parked fibers wake up later in real time;controlreturns immediately.PendingTimersError(the abstractTimeControl::Errorbase remains). This is a breaking API change.Entry side — adopt pre-existing real-loop timers into virtual time
At control start, every execution context's event loop is scanned (
Fiber::ExecutionContext.each), and itsSleep/SelectTimeoutevents are removed from the realCrystal::EventLoop::Pollingtimer heap and re-registered on the virtual clock — so they wake when virtual time is advanced past their deadline rather than in real time. The two clocks share an origin at control start, so the event's realwake_atmaps directly to the virtual deadline.on_wakeproc that marks the real (stack-allocated) sleep event timed out before enqueuing, avoiding the event loop's "manually resumed before the timer expired" guard. The proc is honored on both the advance path and the exit-side re-attach path.Scope limits (by design)
Files
src/time_control/context.cr— pending-timer tracking,on_wake,adopt_sleep/adopt_select_timeout.src/time_control/core_ext/crystal/event_loop/timers.cr(new) — read-only traversal of the pairing-heap timer structure.src/time_control/core_ext/crystal/event_loop/polling.cr— extract-and-re-register driver.src/time_control/errors.cr— dropPendingTimersError.src/time_control.cr— wire adoption intocontrol.Testing
Isolatedcontext).spec/time_control_spec.crpasses 8/8 across stress runs; the MT adoption test passes 6/6 in isolation.🤖 Generated with Claude Code