Add only: filter to restrict interception to matching fibers#1
Open
dentarg wants to merge 1 commit into
Open
Conversation
In a process with background fibers (housekeeping loops, tickers), intercepting every sleep is not practical: any fiber that re-arms a periodic sleep while time is controlled freezes and leaks a pending timer, failing the control block with `PendingTimersError`. `TimeControl.control(only: /^worker/)` restricts interception to fibers whose name matches; sleeps and timeouts of all other fibers stay on the real event loop. Note that real timer deadlines are still measured against the virtual clock while it is controlled, so a non-matching fiber's timer registered inside the block fires late by up to the advanced amount — harmless for periodic housekeeping, but not full isolation. `PendingTimersError` now also names the fibers whose timers were left pending, which makes the leak source obvious in a process with many fibers.
There was a problem hiding this comment.
Pull request overview
Adds an only: regex filter to TimeControl.control so virtual timer interception can be limited to matching fibers, reducing PendingTimersError noise/leaks in apps that run background/housekeeping fibers. It also enhances PendingTimersError to include fiber names for easier leak attribution.
Changes:
- Add
only : Regex?plumbing toTimeControl.control/Context, and gate sleep/select/IO-timeout interception viaContext#controls?(fiber). - Improve
PendingTimersErrorto expose#fiber_namesand include fiber names in the raised message. - Add specs and documentation describing the new filter and the updated pending-timer error.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/time_control/errors.cr | Extends PendingTimersError to carry fiber names and builds a more informative error message. |
| src/time_control/core_ext/fiber.cr | Gates select-timeout virtualization on whether the current fiber is controlled. |
| src/time_control/core_ext/crystal/event_loop/polling.cr | Gates IO-timeout wakeup timer virtualization based on only: filter. |
| src/time_control/core_ext/crystal/event_loop.cr | Gates sleep virtualization based on only: filter. |
| src/time_control/context.cr | Adds only: filter storage and controls? predicate; tracks leaked timers by fiber name. |
| src/time_control.cr | Adds only: to the public API overloads and raises PendingTimersError with names. |
| spec/time_control_spec.cr | Adds coverage for fiber-name reporting and only: behavior. |
| README.md | Documents only: usage and pending-timer fiber naming. |
| AGENTS.md | Updates the public API note to mention the only: filter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+19
to
+21
| def initialize(@fiber_names : Array(String)) | ||
| @count = @fiber_names.size | ||
| super("#{@count} timer(s) were still pending when the control block exited: #{@fiber_names.join(", ")}") |
Comment on lines
+45
to
+47
| # leak, raising `PendingTimersError` at block exit. Note that real timers | ||
| # are still measured against the virtual clock while it is controlled, so | ||
| # an unfiltered fiber's timer can fire late by up to the advanced amount. |
Comment on lines
+230
to
+234
| Note that while time is controlled the real event loop also measures its | ||
| timer deadlines against the virtual clock, so a non-matching fiber's timer | ||
| registered inside the block fires once virtual time advances past it or | ||
| after the block exits — late by up to the advanced amount. For periodic | ||
| housekeeping work this is harmless, but it is not full isolation. |
Comment on lines
+241
to
+248
| it "leaves timers of non-matching fibers on the real event loop" do | ||
| TimeControl.control(only: /^worker/) do |_controller| | ||
| spawn(name: "housekeeping") { sleep 1.second } | ||
| Fiber.yield | ||
| # Exits cleanly: the housekeeping timer is real, not a leaked | ||
| # virtual timer. | ||
| end | ||
| end |
Comment on lines
+250
to
+255
| it "does not control unnamed fibers" do | ||
| TimeControl.control(only: /^worker/) do |_controller| | ||
| spawn { sleep 1.second } | ||
| Fiber.yield | ||
| end | ||
| end |
Contributor
|
#2 may make this unnecessary. |
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.
In a process with background fibers (housekeeping loops, tickers), intercepting every sleep is not practical: any fiber that re-arms a periodic sleep while time is controlled freezes and leaks a pending timer, failing the control block with
PendingTimersError.TimeControl.control(only: /^worker/)restricts interception to fibers whose name matches; sleeps and timeouts of all other fibers stay on the real event loop. Note that real timer deadlines are still measured against the virtual clock while it is controlled, so a non-matching fiber's timer registered inside the block fires late by up to the advanced amount — harmless for periodic housekeeping, but not full isolation.PendingTimersErrornow also names the fibers whose timers were left pending, which makes the leak source obvious in a process with many fibers.