diff --git a/app/src/main/java/eu/darken/capod/monitor/core/worker/MonitorService.kt b/app/src/main/java/eu/darken/capod/monitor/core/worker/MonitorService.kt index 7a5118ed..4319578d 100644 --- a/app/src/main/java/eu/darken/capod/monitor/core/worker/MonitorService.kt +++ b/app/src/main/java/eu/darken/capod/monitor/core/worker/MonitorService.kt @@ -190,8 +190,11 @@ class MonitorService : Service() { // would re-post the previous session's last content under its original timestamp — which is // how a stale "unknown device" frame survives a teardown/restart cycle. Rebuilding is not an // option here: this has to stay cheap enough to satisfy the obligation before DI is ready. + // + // Deliberately does NOT clear the cache here: a collector on Dispatchers.Default can post + // between the read below and any write, so clearing would discard a frame that is actually + // current. Invalidation happens where a new session is launched instead. val reusable = lastNotification?.takeIf { monitoringJob?.isActive == true } - if (reusable == null) lastNotification = null if (!promoteToForeground(reusable ?: MonitorNotifications.createEarlyNotification(this))) { stopSelf(startId) return START_NOT_STICKY @@ -214,6 +217,10 @@ class MonitorService : Service() { val generation = ++monitorGeneration monitorScope.coroutineContext.cancelChildren() + // A new session starts here, so the previous session's last frame must not be re-promoted + // into it. Safe at this point: the old collectors are cancelled and the new ones haven't run, + // so there is no current notification to discard. + lastNotification = null monitoringJob = monitorScope.launch { try { @@ -445,9 +452,11 @@ class MonitorService : Service() { log(TAG, WARN) { "Failed to cancel connected notification: ${e.message}" } } } - // The FGS notification can outlive stopSelf(), leaving whatever content was last posted - // stuck in the shade. notificationManager.cancel() alone is not enough while the - // notification is still bound to the foreground service, so detach it first. + // Defence in depth: AOSP reaps the FGS notification on destroy by itself (verified on + // API 36), but reports of a stuck ongoing notification on Samsung suggest that is not + // universal. Retract it explicitly. cancel() alone is not the foreground-service + // lifecycle operation and can be ignored while the notification is still bound, so + // detach first. Both calls are idempotent. try { stopForeground(Service.STOP_FOREGROUND_REMOVE) notificationManager.cancel(MonitorNotifications.NOTIFICATION_ID) diff --git a/app/src/test/java/eu/darken/capod/monitor/core/worker/MonitorServiceTest.kt b/app/src/test/java/eu/darken/capod/monitor/core/worker/MonitorServiceTest.kt index 5790cfbd..1b7975d5 100644 --- a/app/src/test/java/eu/darken/capod/monitor/core/worker/MonitorServiceTest.kt +++ b/app/src/test/java/eu/darken/capod/monitor/core/worker/MonitorServiceTest.kt @@ -138,8 +138,9 @@ class MonitorServiceTest { val stale = notification("stale") service.postPrimaryNotification(stale) - // Monitor session has ended — the cached frame no longer reflects anything current. - service.setField("monitoringJob", null) + // Monitor session has ended. Production never nulls monitoringJob, it leaves the completed + // job in place, so a finished Job is the faithful state here. + service.setField("monitoringJob", Job().apply { complete() }) service.onStartCommand(MonitorService.intent(context), 0, 1) shouldBe Service.START_STICKY @@ -147,6 +148,24 @@ class MonitorServiceTest { service.getField("lastNotification") shouldNotBeSameInstanceAs stale } + /** + * Invalidation is deliberately at session launch, not next to the promote: a collector on + * Dispatchers.Default can post between the read and a write there, so clearing at promote time + * could discard a frame that is genuinely current. + */ + @Test + fun `launching a new session drops the previous session's notification`() { + val service = createService() + service.readyForMonitoring() + service.postPrimaryNotification(notification("previous")) + + // forceStart bypasses the "already monitoring" early return, so a new session is launched. + service.onStartCommand(MonitorService.intent(context, forceStart = true), 0, 1) shouldBe + Service.START_STICKY + + service.getField("lastNotification").shouldBeNull() + } + /** * The FGS notification can outlive `stopSelf()`. Leaving it up strands whatever content was last * posted — including the unknown-device placeholder built before the first BLE scan batch landed.