Clear deferred-medium backlog: single-flight status write + example mounted guards

CHANGELOG.md

@@ -1,6 +1,8 @@
 ## Unreleased
 
-- Documentation fixes only; no API or behavior changes.
+- Overlapping `checkAndInstall` calls now report their own reason
+  ("A check is already running") in `CodePush.status` instead of surfacing the
+  other check's state.
 - `CodePushOverlay.bannerBuilder`: return a widget to show no banner
   (`const SizedBox.shrink()`) — it never accepted `null`. Keep side effects out
   of the builder; wire the handed `onRestart`/`onDismiss` to your UI instead.

README.md

@@ -245,7 +245,18 @@ CodePush.status.addListener(() {
 ```
 
 Values include: `init`, `Checking server...`, `Downloading patch...`,
-`Patch active`, `No update (204)`, `Restart to apply`, etc.
+`Patch active`, `No update (204)`, `Restart to apply`,
+`A check is already running`, etc.
+
+One ordering guarantee holds across those transitions: when `checkAndInstall`
+returns `false`, the reason is that check's **final** `status` write, so
+reading the value immediately after the `await` reliably explains the result.
+A concurrent (losing) call stamps `A check is already running` without
+disturbing the active check's terminal message. The guarantee is about which
+transition a check ends on — the value still does not stay put afterwards.
+The one exception is the iOS post-download load path, where the guard is
+released before the load completes: its writes are best-effort ordering,
+and a concurrent full check may overwrite them.
 
 Don't use this as an app-facing "a patch loaded" signal. `CodePushOverlay`
 latches the `Patch active` edge internally, but on that transition it re-keys

example/lib/main.dart

@@ -58,6 +58,11 @@ class _CodePushDemoState extends State<CodePushDemo> {
   Future<void> _loadStatus() async {
     final patched = await CodePush.isPatched;
     final patch = await CodePush.currentPatch;
+    // Guard every setState that follows an await: the overlay re-keys the
+    // app subtree when a patch activates (see the build-method note below),
+    // which disposes this State mid-await — a late setState would then
+    // throw in debug builds.
+    if (!mounted) return;
     setState(() {
       _isPatched = patched;
       _currentPatch = patch;
@@ -72,9 +77,15 @@ class _CodePushDemoState extends State<CodePushDemo> {
         appId: 'your-app-id',
         releaseVersion: '1.0.0+1',
         onUpdateReady: () {
+          // checkAndInstall invokes this synchronously, before the await
+          // below resumes — but the overlay can re-key this subtree while
+          // the check is in flight, so the callback can land on a
+          // disposed State. Hence the mounted guard.
+          if (!mounted) return;
           setState(() => _status = 'Patch installed! Restart to apply.');
         },
       );
+      if (!mounted) return;
       if (!installed) {
         // `false` is not just "no update" — checkAndInstall also returns false
         // for "a check is already running", a download failure, a hash
@@ -86,6 +97,7 @@ class _CodePushDemoState extends State<CodePushDemo> {
       }
       await _loadStatus();
     } on CodePushException catch (e) {
+      if (!mounted) return;
       setState(() => _status = 'Error: ${e.message}');
     }
   }
@@ -94,15 +106,18 @@ class _CodePushDemoState extends State<CodePushDemo> {
     setState(() => _status = 'Rolling back...');
     try {
       await CodePush.rollback();
+      if (!mounted) return;
       setState(() => _status = 'Rolled back. Restart to revert.');
       await _loadStatus();
     } on CodePushException catch (e) {
+      if (!mounted) return;
       setState(() => _status = 'Rollback failed: ${e.message}');
     } catch (e) {
       // The iOS Dart-side rollback deletes the resident patch file, which can
       // throw a FileSystemException (not a CodePushException) if it is already
       // gone or unreadable — catch it so the button never leaves an exception
       // unhandled.
+      if (!mounted) return;
       setState(() => _status = 'Rollback failed: $e');
     }
   }

lib/src/code_push.dart

@@ -163,6 +163,12 @@ abstract final class CodePush {
   /// `'Patch already installed'` at the next check, and again on every resume),
   /// so it is a fleeting TRANSITION, not a level you can poll.
   ///
+  /// One ordering guarantee holds across those transitions: when
+  /// [checkAndInstall] returns `false`, its reason was that check's final
+  /// write, so reading the value right after the `await` is reliable — the
+  /// guarantee is about WHICH transition a check ends on, not about the
+  /// value staying put afterwards.
+  ///
   /// Do NOT use it as an app-facing "a patch loaded" signal. [CodePushOverlay]
   /// latches the `'Patch active'` edge internally, but on that same transition
   /// it re-keys its child subtree — disposing any latch a widget under it holds
@@ -442,6 +448,16 @@ abstract final class CodePush {
   /// Checks the server for updates, downloads and installs if available.
   ///
   /// Returns `true` if a patch was installed (restart needed).
+  ///
+  /// Status contract: every `false` return leaves its reason as the FINAL
+  /// [status] write of that check, so reading [status] immediately after the
+  /// `await` reliably explains the result (a later check will overwrite it).
+  /// A concurrent call loses the single-flight guard: it stamps
+  /// `'A check is already running'` and returns `false` without disturbing
+  /// the active check's terminal message — except on the iOS post-download
+  /// load path, where the guard is released before the load completes, so
+  /// its writes are best-effort ordering rather than a guarantee (a
+  /// concurrent full check may overwrite them).
   static Future<bool> checkAndInstall({
     required String serverUrl,
     required String appId,
@@ -456,7 +472,18 @@ abstract final class CodePush {
     // is deliberately global (not per appId/channel): apps use a single
     // config, and a losing caller's `false` means "another check is
     // already running", not "no update".
-    if (_checkInFlight) return false;
+    //
+    // Write [status] before this early return too — checkAndInstall's
+    // contract is that every `false` return leaves its reason in [status],
+    // and without this write a losing caller would surface the OTHER
+    // check's in-flight state as if it were its own result. The in-flight
+    // check keeps overwriting [status] as it progresses, so this write is
+    // transient and cannot mask a 'Patch active' edge (notifications are
+    // synchronous, so that edge has already been delivered).
+    if (_checkInFlight) {
+      status.value = 'A check is already running';
+      return false;
+    }
     _checkInFlight = true;
     try {
       print('[CP] checkAndInstall start');
@@ -507,7 +534,6 @@ abstract final class CodePush {
       // any installed patch, so the fleet returns to the store baseline
       // within one check interval of the switch being flipped.
       if (data['ota_disabled'] == true) {
-        status.value = 'OTA disabled by server';
         try {
           // Unconditional: the rollback knows where each platform keeps
           // the patch (engine on Android/desktop, file removal on iOS
@@ -520,7 +546,12 @@ abstract final class CodePush {
           status.value =
               'OTA disabled by server — patch removed (restart to apply)';
         } catch (_) {
-          // Best-effort: a clean device has nothing to revert.
+          // Best-effort: a clean device has nothing to revert. The
+          // status is written AFTER the awaited rollback attempt (in
+          // both branches): a concurrent check() during that await
+          // stamps 'A check is already running', and the kill-switch
+          // signal must be what stands when we return.
+          status.value = 'OTA disabled by server';
         }
         return false;
       }
@@ -607,7 +638,6 @@ abstract final class CodePush {
       );
 
       if (actualEngineFingerprint == null) {
-        status.value = 'Incompatible baseline: engine has no code push support';
         await _reportIncompatibleBaseline(
           serverUrl: serverUrl,
           appId: appId,
@@ -617,6 +647,12 @@ abstract final class CodePush {
           expectedFingerprint: expectedEngineFingerprint,
           actualFingerprint: null,
         );
+        // Terminal status is written AFTER the awaited report: a
+        // concurrent check() during that POST stamps 'A check is
+        // already running', and this actionable message must be what
+        // stands when we return. It's an error state, not progress, so
+        // writing it last loses nothing.
+        status.value = 'Incompatible baseline: engine has no code push support';
         return false;
       }
 
@@ -627,8 +663,6 @@ abstract final class CodePush {
       if (expectedEngineFingerprint != null &&
           actualEngineFingerprint != 'unknown' &&
           expectedEngineFingerprint != actualEngineFingerprint) {
-        status.value = 'Incompatible baseline: engine ABI mismatch '
-            '($actualEngineFingerprint vs $expectedEngineFingerprint)';
         await _reportIncompatibleBaseline(
           serverUrl: serverUrl,
           appId: appId,
@@ -637,6 +671,11 @@ abstract final class CodePush {
           expectedFingerprint: expectedEngineFingerprint,
           actualFingerprint: actualEngineFingerprint,
         );
+        // After the await, so a concurrent check()'s guard write can't
+        // overwrite this terminal status (see the null-fingerprint
+        // branch above).
+        status.value = 'Incompatible baseline: engine ABI mismatch '
+            '($actualEngineFingerprint vs $expectedEngineFingerprint)';
         return false;
       }
 
@@ -1906,15 +1945,22 @@ abstract final class CodePush {
             'bytes=${container.length}',
           );
         }
-        status.value = 'Patch format is unexpected — rolling back. '
-            'Upgrade flutter_compile to the latest version and '
-            'rebuild the patch.';
         await _iosImmediateRollback(
           serverUrl: serverUrl,
           appId: appId,
           patchId: patchId,
           errorMessage: 'Patch format mismatch — rejected before load',
         );
+        // Written after the awaited rollback (file deletes + telemetry
+        // POST) so this upgrade hint is the last write this method
+        // makes. The single-flight guard is not held here (the install
+        // path returns this future without awaiting it, and cold-boot /
+        // reload callers never take the guard), so a concurrent full
+        // check can still overwrite it later — best-effort ordering,
+        // not a guarantee.
+        status.value = 'Patch format is unexpected — rolling back. '
+            'Upgrade flutter_compile to the latest version and '
+            'rebuild the patch.';
         return false;
       }
 
@@ -1993,7 +2039,6 @@ abstract final class CodePush {
         );
       }
       print('[CP] MODULE LOAD THREW ($origin) — $e');
-      status.value = 'Module error: $e — rolling back patch';
       // Real load failure. Delete immediately instead of waiting for
       // the three-strike auto-rollback.
       await _iosImmediateRollback(
@@ -2002,6 +2047,11 @@ abstract final class CodePush {
         patchId: patchId,
         errorMessage: 'Patch load threw $e — deleted immediately',
       );
+      // Terminal status goes after the awaited rollback so it is the
+      // last write this method makes. The single-flight guard is not
+      // held on this path, so a concurrent full check can still
+      // overwrite it later — best-effort ordering, not a guarantee.
+      status.value = 'Module error: $e — rolling back patch';
       return false;
     }
   }

test/hardening_test.dart

@@ -23,6 +23,8 @@ void main() {
   Duration offerDelay = Duration.zero;
   String? offeredHash;
   String? offeredUrlOverride;
+  String? offeredEngineFingerprint;
+  Future<void> Function()? onTelemetry;
 
   void serve() {
     server.listen((HttpRequest req) async {
@@ -36,9 +38,14 @@ void main() {
             'patch_available': true,
             'patch_id': 'p1',
             if (offeredHash != null) 'patch_hash': offeredHash,
+            if (offeredEngineFingerprint != null)
+              'engine_fingerprint': offeredEngineFingerprint,
             'patch_url':
                 offeredUrlOverride ?? 'http://127.0.0.1:${server.port}/patch',
           }));
+      } else if (req.uri.path.endsWith('/telemetry/client-error')) {
+        if (onTelemetry != null) await onTelemetry!();
+        req.response.statusCode = HttpStatus.ok;
       } else {
         downloads++;
         req.response.statusCode = HttpStatus.ok;
@@ -55,6 +62,8 @@ void main() {
     offerDelay = Duration.zero;
     offeredHash = null;
     offeredUrlOverride = null;
+    offeredEngineFingerprint = null;
+    onTelemetry = null;
     CodePush.debugResetBaselineHashCache();
     TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger
         .setMockMethodCallHandler(engineChannel, (MethodCall call) async {
@@ -169,6 +178,65 @@ void main() {
       expect(updateChecks, 1, reason: 'losers return before the network');
       expect(downloads, 1);
     });
+
+    test('a losing overlapped check writes its own reason to status',
+        () async {
+      // Contract: checkAndInstall leaves its reason in CodePush.status
+      // before EVERY false return — including the single-flight early
+      // return, which previously left the other check's in-flight state
+      // in place (so a caller surfacing status after `false` rendered a
+      // foreign operation's progress as its own result).
+      offeredHash = patchHash;
+      offerDelay = const Duration(milliseconds: 300);
+      serve();
+
+      // Both calls in the same synchronous turn: the winner latches the
+      // guard before its first await, and the loser's status write also
+      // lands before ITS first await — so no wall-clock sequencing is
+      // needed (a delay could even over-sleep past the offer window and
+      // turn the "loser" into a real second check).
+      final winner = check();
+      final loser = check();
+      expect(CodePush.status.value, 'A check is already running');
+      expect(await loser, isFalse);
+
+      await winner;
+    });
+
+    test(
+        'incompatible-baseline terminal status survives a concurrent check '
+        'during the telemetry POST', () async {
+      // Regression guard for the reordered terminal writes: the engine
+      // ABI-mismatch branch reports telemetry (awaited) BEFORE writing
+      // its terminal status. A concurrent check() arriving inside that
+      // await stamps 'A check is already running'; the terminal write
+      // must land AFTER the await so the actionable message is what
+      // stands when the winner returns. Moving that write back up next
+      // to its branch condition turns this test red.
+      offeredHash = patchHash;
+      offeredEngineFingerprint = 'flutter-9.9.9'; // mocked engine: 'abi'
+      Future<bool>? loser;
+      String? statusDuringReport;
+      onTelemetry = () async {
+        // The winner is parked on the awaited telemetry POST with the
+        // single-flight guard held — exactly the reordered-write window.
+        loser ??= check();
+        statusDuringReport = CodePush.status.value;
+      };
+      serve();
+
+      expect(await check(), isFalse);
+      expect(loser, isNotNull, reason: 'the telemetry hook must have fired');
+      expect(await loser, isFalse);
+      expect(statusDuringReport, 'A check is already running',
+          reason: 'the loser must have stamped its status inside the '
+              'window, or this test is not exercising the race');
+      expect(
+        CodePush.status.value,
+        startsWith('Incompatible baseline: engine ABI mismatch'),
+      );
+      expect(downloads, 0, reason: 'mismatch is refused before download');
+    });
   });
 
   group('download size cap', () {

test/ota_controls_test.dart

@@ -78,6 +78,43 @@ void main() {
       expect(CodePush.status.value, isNot(contains('patch removed')));
     });
 
+    test(
+        'clean device: kill-switch status survives a concurrent check '
+        'during the revert attempt', () async {
+      serveOtaDisabled();
+      Future<bool>? loser;
+      String? statusDuringRevert;
+      TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger
+          .setMockMethodCallHandler(engineChannel, (MethodCall call) async {
+        if (call.method == 'CodePush.rollback') {
+          // The winner is parked on the revert await with the
+          // single-flight guard held — exactly the window where a
+          // concurrent check stamps its loser status. The kill-switch
+          // branch must rewrite AFTER this await so the fleet-wide
+          // signal is what stands when the winner returns.
+          loser ??= check();
+          // Captured (not expect()ed) here: a TestFailure thrown inside
+          // this handler is swallowed by the kill-switch catch around
+          // the rollback await; the body assertion below fails loudly.
+          statusDuringRevert = CodePush.status.value;
+          return false; // Unpatched device: the revert attempt no-ops.
+        }
+        return null;
+      });
+
+      final installed = await check();
+
+      expect(installed, isFalse);
+      expect(loser, isNotNull);
+      expect(statusDuringRevert, contains('already running'),
+          reason: 'the loser must actually have stamped its status inside '
+              'the revert window — otherwise this test is vacuous');
+      expect(await loser, isFalse);
+      expect(CodePush.status.value, contains('OTA disabled'));
+      expect(CodePush.status.value, isNot(contains('patch removed')));
+      expect(CodePush.status.value, isNot(contains('already running')));
+    });
+
     test('patched device: reverts to the store baseline', () async {
       serveOtaDisabled();
       final engineCalls = <String>[];