Harden realtime foreground service lifecycle - #112
Conversation
- Skip the periodic worker's foreground promotion when the realtime service is already running, so its backstop pass no longer posts a duplicate ongoing status notification - Guard startForegroundService against ForegroundServiceStartNotAllowedException so a late viewModelScope apply() from the background falls back to the periodic backstop instead of crashing - Give the ongoing status notification a contentIntent into MainActivity - Hold a timeout-bounded partial wake lock around each realtime pass so an in-flight transfer does not stall when the screen turns off
in-jun
left a comment
There was a problem hiding this comment.
Reviewed against all four referenced issues and the surrounding lifecycle code. This is clean and well-scoped — each change maps to one issue and stays within app/sync.
#103 — duplicate notification. Gating the worker's setForeground() on !isRealtimeServiceEnabled() reads the same KEY_REALTIME flag apply() writes, so the redundant 1002 notification is suppressed in REALTIME while PERIODIC keeps its extended budget. Correct.
#80 — dead tap target. contentIntent on the shared status notification reuses the immutable PendingIntent pattern from SyncAlertNotifier; request code 1001 doesn't collide with the 2000+ alert codes, and the intents are equal anyway. Good that the worker's 1002 notification becomes tappable too.
#79 — wake lock. Timeout-bounded acquire after the network-gate early return, released in finally behind the isHeld guard so the timeout auto-release can't trigger an under-lock. Passes are serialized by the collect, and cancellation still runs the finally, so there's no leak on shutdown.
#17 — background FGS start. The guard swallows only ForegroundServiceStartNotAllowedException on API 31+ and rethrows everything else, with the SDK_INT check short-circuiting ahead of the is check (safe on older APIs). This also restores deletePair's forgetAncestors cleanup, which the uncaught throw used to skip.
One behavioral note, not blocking: in REALTIME mode where the service was refused a background start, the backstop worker now also skips promotion and runs a long pass under the default budget. That's an acceptable degradation given the engine is crash-safe and recordIfStoppedMidPass surfaces a persistently stopped pair.
LGTM.
Cleans up the realtime foreground service and periodic backstop worker around notifications, wake locks, and background start restrictions.
sync_statusnotification (1001), so the worker promoting itself posted a second, redundant "Syncing files" notification (1002) on every backstop pass; it now checks the realtime flag and only promotes in PERIODIC mode where the long-pass execution budget is actually needed.contentIntent. The persistent notification that REALTIME users always see in the shade was a dead tap target; it now deep-links intoMainActivity, reusing the immutablePendingIntentpatternSyncAlertNotifieralready uses.PARTIAL_WAKE_LOCKaround each realtime pass. A foreground service keeps the process and network alive but does not stop CPU suspend once the screen turns off, so an in-flight transfer could freeze mid-pass until the next wakeup; the wake lock (released in afinally) keeps the pass running. This also puts the already-declaredWAKE_LOCKpermission to use.startForegroundService()againstForegroundServiceStartNotAllowedException.apply()runs insideviewModelScope, which outlives the Activity, so a lateawaitLoaded()/sync-lock wait could start the service from the background and crash on Android 12+; it now falls back to the WorkManager periodic backstop, matching the guard the boot path already has.Fixes #103
Fixes #80
Fixes #79
Fixes #17