Fix app doesn't go into reconnection mode in some cases#320
Merged
Conversation
**Root cause** — two problems combining to suppress reconnection: 1. **Race condition** (`CameraStreamerService` sets `NOT_STREAMING` ~4ms before `isStreamingFlow` emits `false`). The old code read `streamStatus.value` as `previousStatus`, so by the time the `isStreamingFlow` observer fired, the status was already `NOT_STREAMING` → `wasStreaming = false` → reconnect skipped silently. 2. **Silent socket close** — when you turn WiFi off in Android Settings, the OS closes the SRT UDP socket cleanly (no error), so no `ClosedException` is emitted to `throwableFlow`. The `isStreamingFlow` observer was the *only* remaining reconnect path — but bug #1 broke it. **Fix** — in the `isStreamingFlow` observer, replaced the unreliable `previousStatus` check with a `var previouslyStreaming` local variable that tracks the actual previous value of the flow itself. It's initialized from `isStreamingFlow.value` (handles already-streaming case on attach), set to `true` when `isStreaming=true` fires, used as `wasStreaming` when `isStreaming=false` fires, then reset to `false`. This is completely independent of the service's status updates, so the race condition can no longer suppress reconnection. --- The diff is clean and minimal. Let me analyze potential behavioral changes: **Scenarios that now work differently:** | Scenario | Old behavior | New behavior | Concern? | |----------|-------------|-------------|----------| | WiFi off → SRT socket silently closed | `wasStreaming=false` → no reconnect | `wasStreaming=true` → reconnect triggers | **Desired fix** | | Service observer fires `NOT_STREAMING` before `isStreamingFlow=false` | Race → reconnection suppressed | Race irrelevant, tracked independently | **Desired fix** | **Scenarios that remain unchanged:** 1. **User taps Stop** — `userStoppedManually=true` is set *before* anything else happens, so the `wasStreaming` check never reaches the reconnect branch. No change. 2. **Already reconnecting** — `isReconnecting=true` guard remains, prevents duplicate triggers. No change. 3. **Stream that never started** (`isStreamingFlow` was never `true`) — `previouslyStreaming` initializes from `isStreamingFlow.value` which is `false`, and is never set to `true`, so `wasStreaming=false` → goes to "Stream never started" path. No change. 4. **Service rebind after process death** — `previouslyStreaming` re-initializes from current `isStreamingFlow.value`. If the stream was running, it picks up `true`; if not, `false`. Same semantics as before. **One edge case worth noting:** If `isStreamingFlow` emits `true` → `false` → `false` rapidly (e.g., flicker), the first `false` sets `previouslyStreaming = false`, so the second `false` correctly gets `wasStreaming=false`. No double-reconnect. Safe. **Verdict:** The fix is safe. The only behavioral change is that the reconnection now correctly triggers when WiFi drops silently — no other paths are affected.
5e1f7c8 to
a4ed89b
Compare
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.
Root cause — two problems combining to suppress reconnection:
Race condition (
CameraStreamerServicesetsNOT_STREAMING~4ms beforeisStreamingFlowemitsfalse). The old code readstreamStatus.valueaspreviousStatus, so by the time theisStreamingFlowobserver fired, the status was alreadyNOT_STREAMING→wasStreaming = false→ reconnect skipped silently.Silent socket close — when you turn WiFi off in Android Settings, the OS closes the SRT UDP socket cleanly (no error), so no
ClosedExceptionis emitted tothrowableFlow. TheisStreamingFlowobserver was the only remaining reconnect path — but bug Foreground service #1 broke it.Fix — in the
isStreamingFlowobserver, replaced the unreliablepreviousStatuscheck with avar previouslyStreaminglocal variable that tracks the actual previous value of the flow itself. It's initialized fromisStreamingFlow.value(handles already-streaming case on attach), set totruewhenisStreaming=truefires, used aswasStreamingwhenisStreaming=falsefires, then reset tofalse. This is completely independent of the service's status updates, so the race condition can no longer suppress reconnection.The diff is clean and minimal. Let me analyze potential behavioral changes:
Scenarios that now work differently:
wasStreaming=false→ no reconnectwasStreaming=true→ reconnect triggersNOT_STREAMINGbeforeisStreamingFlow=falseScenarios that remain unchanged:
User taps Stop —
userStoppedManually=trueis set before anything else happens, so thewasStreamingcheck never reaches the reconnect branch. No change.Already reconnecting —
isReconnecting=trueguard remains, prevents duplicate triggers. No change.Stream that never started (
isStreamingFlowwas nevertrue) —previouslyStreaminginitializes fromisStreamingFlow.valuewhich isfalse, and is never set totrue, sowasStreaming=false→ goes to "Stream never started" path. No change.Service rebind after process death —
previouslyStreamingre-initializes from currentisStreamingFlow.value. If the stream was running, it picks uptrue; if not,false. Same semantics as before.One edge case worth noting:
If
isStreamingFlowemitstrue→false→falserapidly (e.g., flicker), the firstfalsesetspreviouslyStreaming = false, so the secondfalsecorrectly getswasStreaming=false. No double-reconnect. Safe.Verdict: The fix is safe. The only behavioral change is that the reconnection now correctly triggers when WiFi drops silently — no other paths are affected.