Stop the playlist and the OTA checker when the Activity is destroyed

onDestroy already shuts down the wall and group controllers, and its comment says exactly why: those
Handlers are on the main looper, which outlives the Activity, so a surviving tick "would keep
broadcasting sync frames against the released player forever". Three other things on that same looper
were never stopped.

PlaylistController kept advancing after the Activity was gone. Every tick wrote the resume index and
emitted play_start/play_end through the still-live WebSocketService, so after any relaunch — the
"launch" command, Relauncher after OTA or boot, a re-pair, or a config change outside the ones the
manifest handles — two controllers were reporting playback for one screen. That inflates Total Plays
and Hours in Reports for that panel, and races over the resume position #234 depends on. Widget items
also re-entered showWidget on a WebView nobody owned any more.

UpdateChecker was never stopped either, and its install receiver was never unregistered:
installReceiverRegistered is per-instance, so each recreate added another checker polling
/api/update/check and another receiver for INSTALL_COMPLETE. N of those turns one
STATUS_PENDING_USER_ACTION into N confirm dialogs stacked over customer content, and concurrent
checkers race in tryPackageInstaller — which starts by abandoning ALL of the app's installer
sessions, so one can abandon another's staged session mid-flight and the update never completes.
shutdown() now does both, and the receiver is held so it can actually be unregistered.

The Activity's own posted callbacks (the 30s failure-check loop among them) are cleared too.

134 Android JVM tests green. The effect is a leak and a duplicate reporting stream rather than a
wrong value on a screen, so it is verified by reading the lifecycle rather than by a unit test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
This commit is contained in:
Claude 2026-07-30 21:03:19 -05:00
parent 6e9a1f9711
commit acadb4c1f4
2 changed files with 35 additions and 0 deletions

View file

@ -1328,6 +1328,19 @@ class MainActivity : AppCompatActivity() {
override fun onDestroy() {
remoteStreaming = false
// Everything below this line exists for the same reason the wall/group shutdown does, and
// was missing: these Handlers are on the MAIN LOOPER, which outlives the Activity.
//
// PlaylistController kept advancing after the Activity was destroyed. Each tick wrote the
// resume index and emitted play_start/play_end through the still-live WebSocketService, so
// after a relaunch (the "launch" command, Relauncher after OTA/boot, a re-pair, or a config
// change outside the ones we handle) TWO controllers were reporting playback for one screen
// — inflating Total Plays and Hours in Reports, and racing over the resume position that
// #234 relies on. Widget items also re-entered showWidget on a WebView nobody owned.
if (::playlistController.isInitialized) playlistController.stop()
if (::updateChecker.isInitialized) updateChecker.shutdown()
// The 30s failure-check loop and anything else this Activity posted.
handler.removeCallbacksAndMessages(null)
// Kill the wall/group leader tick BEFORE releasing media. The Handler is on the main looper
// (outlives this Activity), so a surviving tick would keep broadcasting sync frames against
// the released player forever — the zombie-leader / split-brain / garbage-position leak.

View file

@ -38,6 +38,8 @@ class UpdateChecker(private val context: Context) {
private val CHECK_INTERVAL = 30 * 60 * 1000L
private var installReceiverRegistered = false
// Held so shutdown() can unregister it; without a handle the receiver outlives the Activity.
private var installReceiver: BroadcastReceiver? = null
// #139: report OTA status to the dashboard (device:log, tag "ota"). Wired by MainActivity
// to WebSocketService.sendLog; null until then. Read lazily so binding order doesn't matter.
@ -92,6 +94,7 @@ class UpdateChecker(private val context: Context) {
@Suppress("UnspecifiedRegisterReceiverFlag") context.registerReceiver(receiver, filter)
}
installReceiverRegistered = true
installReceiver = receiver
}
fun startPeriodicCheck() {
@ -113,6 +116,25 @@ class UpdateChecker(private val context: Context) {
checkTimer = null
}
/**
* Full teardown for an Activity that is going away.
*
* stopPeriodicCheck alone leaves the install receiver registered against a dead Context, and
* installReceiverRegistered is per-instance so each Activity recreate produced another
* checker polling /api/update/check and another receiver for INSTALL_COMPLETE. N of those means
* one STATUS_PENDING_USER_ACTION fires N confirm dialogs over customer content, and concurrent
* checkers race in tryPackageInstaller, which begins by abandoning ALL of this app's installer
* sessions so one can abandon another's staged session mid-flight and the update never lands.
*/
fun shutdown() {
stopPeriodicCheck()
if (installReceiverRegistered) {
installReceiver?.let { r -> try { context.unregisterReceiver(r) } catch (_: Throwable) { /* already gone */ } }
installReceiver = null
installReceiverRegistered = false
}
}
/**
* [forced] = an operator pressed "force update" on this specific device, rather than the
* 30-minute timer firing. A forced run differs in three ways, all because a human aimed it at