fix(stop-detail): single-flight departures observe on resume - #166
Closed
ai-tiro wants to merge 1 commit into
Closed
fix(stop-detail): single-flight departures observe on resume#166ai-tiro wants to merge 1 commit into
ai-tiro wants to merge 1 commit into
Conversation
Bringing the app to the foreground on a stop-detail screen intermittently fired TWO departures requests ~66 ms apart instead of one (issue #162). Root cause: startObserving() used a plain observeJob?.cancel() and then immediately launched the new collection. cancel() only *requests* cooperative cancellation, so the previous observe coroutine could still be parked inside the suspending fetch and fire a departures request at the same moment the fresh collection subscribed — an intermittent race with a small window. Fix: the new job now cancelAndJoin()s the prior job before subscribing to observeDepartures, so the old subscription is fully torn down first and exactly one fetch starts per resume. The prior Job is captured before reassigning observeJob, so the join never targets the coroutine it runs in (no self-join deadlock). The genuine first start has no prior job and subscribes immediately; the 30s polling cadence, background-pause behaviour, and the previous stacked fix's Loading-retention / idempotent-header contracts are all preserved. Adds a single-flight unit test asserting a pause->resume cycle leaves exactly one live collector and applies a single tick once. Co-Authored-By: ai-tiro <ai-tiro@jfx.ac> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Debug APK: app-debug-6d270f330b44b03f2478c1ede6cf6dd64a1d5993.apk (built from Requires GitHub login. Artifact expires after 3 days. |
This was referenced Jun 18, 2026
Collaborator
Author
|
Superseded by #179, which combined these commits and has merged to master. Closing as superseded. |
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.
Closes #162.
What I did
StopDetailViewModel.startObserving()nowcancelAndJoin()s the prior observe job before the new coroutine subscribes toobserveDepartures, instead of firing a cooperativecancel()and immediately launching the new collection.Jobis captured into a local beforeobserveJobis reassigned, so the new coroutine joins the old job and never self-joins (no deadlock).observedKeyssize grows by one per resume, not two) and a single tick is applied once.Why / what I discovered
The bug (issue #162): foregrounding on a stop-detail screen intermittently fired two departures requests ~66 ms apart.
repeatOnLifecycle(RESUMED)callsstartObserving()on every Pause→Resume. The old code didobserveJob?.cancel()then launched the new collection synchronously.Job.cancel()only requests cooperative cancellation — the previously-paused-but-not-yet-stopped observe coroutine could still be parked inside the suspending fetch and emit/fetch at the same instant the fresh collection subscribed. Small window, hence intermittent (the issue saw it once at09:06:11.558/.624, and a later cycle fired only once).cancelAndJoin()closes the window deterministically: the old subscription is fully torn down before the new one starts, so exactly one fetch begins per resume.I confirmed the race is real (not already neutralised by the #161 Loading-retention change): that change only stops the duplicate from flickering the skeleton — it does not stop the duplicate network fetch. So a genuine production fix was warranted, not just hardening.
What I considered / didn't do
refresh()and the legitimate resume-after-real-pause both want a fresh collection, so an "already active → no-op" guard would suppress wanted fetches.cancelAndJoinkeeps one-fetch-per-resume while still honouring every deliberate restart.stopObserving()(plaincancel()is correct there — nothing races a teardown) or to the 30s cadence / background-pause / pagination paths.Testing
./gradlew :feature:stop-detail:compileDebugKotlin :feature:stop-detail:testDebugUnitTest— green, 39 tests, 0 failures. The previous fix'sissue 161tests stay green.resume single-flights the observe … issue 162asserts single-flight at the ViewModel seam.repeatOnLifecyclelifecycle-driven instrumentation test (realLifecycleOwnerPause→Resume timing) is heavier than the seam test and not added here; the ViewModel-level single-flight assertion covers the production contract. No one-way-door changes.🤖 Generated with Claude Code