This repository has been archived by the owner on Oct 18, 2024. It is now read-only.
Fully update internal state before invoking periodic-timer callback #89
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 dart-lang/test#2319.
Currently when a periodic timer's callback gets invoked, its
_nextCall
is still the time of the current call, not the next one. If the timer callback itself callsflushTimers
orelapse
, this causes the same timer to immediately get called again.Fortunately the fix is easy: update
_nextCall
just before invoking_callback
, instead of just after.To work through why this is a complete fix (and doesn't leave further bugs of this kind still to be fixed):
After this fix, the call to the timer's callback is a tail call from
FakeTimer._fire
. Because the call site ofFakeTimer._fire
is immediately followed byflushMicrotasks()
, this means calling otherFakeAsync
methods from the timer callback is no different from doing so in a subsequent microtask.Moreover, when running timers from
flushTimers
, if after theflushMicrotasks
call this turns out to be the last timer to run, thenflushTimers
will return with no further updates to the state. So when the timer callback is invoked (in that case), the wholeFakeAsync
state must already be in a state thatflushTimers
would have been happy to leave it in. (And there's no special cleanup that it does only after a non-last timer.)Similarly, when running timers from
elapse
(the only other possibility), the only difference from a state thatelapse
would be happy to leave things in is that_elapsingTo
is still set. That field affects onlyelapse
andelapseBlocking
; and those are both designed to handle being called from withinelapse
.Contribution guidelines:
dart format
.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.