fix(polling): drop stale in-flight check results on cancel/resubscribe - #148
Open
mem-5514-tahara wants to merge 3 commits into
Open
fix(polling): drop stale in-flight check results on cancel/resubscribe#148mem-5514-tahara wants to merge 3 commits into
mem-5514-tahara wants to merge 3 commits into
Conversation
If a listener cancels while a connectivity check is still running, that check's result belonged to the old subscription. Without a guard, it could still get emitted to a brand-new subscriber that started its own fresh check moments later, or restart the polling timer even though nobody is listening anymore. Track a _subscriptionVersion counter, bumped only when a listener cancels. _maybeEmitStatusUpdate snapshots it before the check starts and skips the emit (and the next timer reschedule) if it changed while the check was in-flight.
OutdatedGuy
requested changes
Aug 18, 2026
mem-5514-tahara
added a commit
to mem-5514-tahara/internet_connection_checker_plus
that referenced
this pull request
Aug 18, 2026
Removing _generation entirely (previous commit) turned out to be wrong: unlike _cancelGeneration, this guard protects backoff-specific mutable state (_currentBackoffDelay/_backoffNeedsReset), which only exists in this branch. Without it, a setIntervalAndResetTimer call (or a cancel) racing an in-flight failing check silently undoes the reset it just applied — the next failure incorrectly grows the delay instead of starting over at the fresh initial delay. Confirmed via a reproduction: removing the guard makes the added regression test fail (checkCount climbs from 3 to 5, and the delay grows to 100ms instead of staying at the reset 50ms). Renamed from _generation to _timerVersion with a plainer doc comment, per review feedback. _cancelGeneration (renamed _subscriptionVersion) remains split out as OutdatedGuy#148, since that one has no backoff dependency.
Move the hasListener check right after the await so isStale no longer needs to recheck it, and drop comments already covered by the _subscriptionVersion doc comment.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds subscription-version tracking to discard connectivity results from cancelled subscriptions.
Changes:
- Guards status emissions against stale in-flight checks.
- Adds cancellation and resubscription regression tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
lib/src/internet_connection.dart |
Adds stale-result detection during polling. |
test/internet_connection_test.dart |
Tests cancellation during in-flight checks. |
Suppressed comments (1)
lib/src/internet_connection.dart:304
- The version is advanced only after awaiting trigger-stream cleanup. If that subscription has asynchronous cancellation, a replacement listener can attach and the old connectivity check can finish while the version is still unchanged, allowing the stale result to be emitted to the replacement subscriber. Invalidate the generation synchronously before the first
await.
Future<void> _handleStatusChangeCancel() async {
await _triggerSubscription?.cancel();
_triggerSubscription = null;
_subscriptionVersion++;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+288
to
+290
| final isStale = _subscriptionVersion != previousSubVersion; | ||
|
|
||
| if (!isStale && _lastStatus != currentStatus) { |
Author
There was a problem hiding this comment.
Fixed — a version mismatch now returns immediately, skipping both emission and rescheduling.
A stale check still reached the unconditional Timer(...) at the end of _maybeEmitStatusUpdate, overwriting _timerHandle with an untracked timer while the fresh subscription's own timer kept running unreferenced. Return immediately on a version mismatch so stale work skips both emission and rescheduling. Also bump _subscriptionVersion synchronously before the first await in _handleStatusChangeCancel, so a resubscribe that races the in-flight trigger-subscription cancellation can't observe the stale version.
Author
|
Hi @OutdatedGuy, just a gentle follow-up on this PR as well! The Copilot review feedback regarding early return on stale checks has been applied, and all tests/checks are green. Whenever you have time to take a look at both #148 and #139, please let me know. Thanks! |
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.
Summary
Split out from #139 per review feedback, with
_cancelGenerationrenamed to_subscriptionVersion.If a listener cancels its subscription while a connectivity check is still running, that check's result belongs to the old subscription. Without a guard:
This adds a
_subscriptionVersioncounter, bumped only when a listener cancels._maybeEmitStatusUpdatesnapshots it before the check starts and skips both the emit and the next timer reschedule if it changed while the check was in-flight.Test plan
dart analyze— no issuesdart format --set-exit-if-changed— cleandart test— all tests pass, including two new regression tests: