Skip to content

Commit

Permalink
Fully update internal state before invoking periodic-timer callback (#89
Browse files Browse the repository at this point in the history
)

Fixes #88.

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 calls `flushTimers` or `elapse`, 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 of `FakeTimer._fire` is
immediately followed by `flushMicrotasks()`, this means calling
other `FakeAsync` methods from the timer callback is no different
from doing so in a subsequent microtask.

Moreover, when running timers from `flushTimers`, if after the
`flushMicrotasks` call this turns out to be the last timer to run,
then `flushTimers` will return with no further updates to the state.
So when the timer callback is invoked (in that case), the whole
`FakeAsync` state must already be in a state that `flushTimers`
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 that `elapse` would
be happy to leave things in is that `_elapsingTo` is still set.
That field affects only `elapse` and `elapseBlocking`; and those
are both designed to handle being called from within `elapse`.
  • Loading branch information
gnprice authored Oct 4, 2024
1 parent 591e89f commit 62fa2d9
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 1 deletion.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
## 1.3.2-wip

* Require Dart 3.3
* Fix bug where a `flushTimers` or `elapse` call from within
the callback of a periodic timer would immediately invoke
the same timer.

## 1.3.1

Expand Down
2 changes: 1 addition & 1 deletion lib/fake_async.dart
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,9 @@ class FakeTimer implements Timer {
assert(isActive);
_tick++;
if (isPeriodic) {
_nextCall += duration;
// ignore: avoid_dynamic_calls
_callback(this);
_nextCall += duration;
} else {
cancel();
// ignore: avoid_dynamic_calls
Expand Down
17 changes: 17 additions & 0 deletions test/fake_async_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,23 @@ void main() {
expect(ticks, [1, 2]);
});
});

test('should update periodic timer state before invoking callback', () {
// Regression test for: https://github.com/dart-lang/fake_async/issues/88
FakeAsync().run((async) {
final log = <String>[];
Timer.periodic(const Duration(seconds: 2), (timer) {
log.add('periodic ${timer.tick}');
async.elapse(Duration.zero);
});
Timer(const Duration(seconds: 3), () {
log.add('single');
});

async.flushTimers(flushPeriodicTimers: false);
expect(log, ['periodic 1', 'single']);
});
});
});

group('clock', () {
Expand Down

0 comments on commit 62fa2d9

Please sign in to comment.