From 62fa2d9ae2142bc1e111fe35bf301c982bef4adb Mon Sep 17 00:00:00 2001
From: Greg Price <gnprice@gmail.com>
Date: Fri, 4 Oct 2024 16:04:55 -0700
Subject: [PATCH] Fully update internal state before invoking periodic-timer
 callback (#89)

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`.
---
 CHANGELOG.md              |  3 +++
 lib/fake_async.dart       |  2 +-
 test/fake_async_test.dart | 17 +++++++++++++++++
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 61c8b85..5c7dbd0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
 
diff --git a/lib/fake_async.dart b/lib/fake_async.dart
index 415cb96..b4eea85 100644
--- a/lib/fake_async.dart
+++ b/lib/fake_async.dart
@@ -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
diff --git a/test/fake_async_test.dart b/test/fake_async_test.dart
index 7aefc5f..463eecd 100644
--- a/test/fake_async_test.dart
+++ b/test/fake_async_test.dart
@@ -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', () {