Prevent unregistering actionReceiver twice and handle weird lifecycle

app/src/main/kotlin/io/homeassistant/companion/android/assist/service/AssistVoiceInteractionService.kt

@@ -68,11 +68,21 @@ class AssistVoiceInteractionService : VoiceInteractionService() {
     }
     private var isServiceReady = false
 
+    /** One-way latch set once the service has begun tearing down (via [onShutdown] or [onDestroy]). */
+    private var isTornDown = false
+
     /** Non-null only while the receiver is registered (between [onReady] and [onShutdown]). */
     private var actionReceiver: BroadcastReceiver? = null
 
     override fun onReady() {
         super.onReady()
+        if (isTornDown) {
+            // The system can deliver onReady even after onShutdown/onDestroy while it is winding the
+            // service down. Registering a receiver on the dying context would later crash in
+            // onShutdown, so ignore this spurious signal
+            Timber.w("Ignoring onReady delivered after shutdown")
+            return
+        }
         isServiceReady = true
         Timber.d("VoiceInteractionService is ready")
         actionReceiver = object : BroadcastReceiver() {
@@ -102,14 +112,32 @@ class AssistVoiceInteractionService : VoiceInteractionService() {
 
     override fun onShutdown() {
         super.onShutdown()
+        isTornDown = true
         isServiceReady = false
         Timber.d("VoiceInteractionService is shutting down")
-        actionReceiver?.let(::unregisterReceiver)
-        actionReceiver = null
+        actionReceiver?.let { receiver ->
+            actionReceiver = null
+            try {
+                unregisterReceiver(receiver)
+            } catch (e: IllegalArgumentException) {
+                // On some devices the framework tears down the receiver registration before
+                // onShutdown() runs while the service instance (and this field) survives. There is
+                // no API to query whether a receiver is still registered, so swallowing this is the
+                // documented way to handle the resulting "Receiver not registered" error
+                Timber.w(e, "Action receiver was already unregistered")
+            }
+        }
         // Don't use stopListening() as it launches a coroutine that may not complete before cancel
         serviceScope.cancel()
     }
 
+    override fun onDestroy() {
+        // onShutdown is not guaranteed to run before onDestroy, so latch teardown here too to keep
+        // a late onReady from re-initializing an already-destroyed instance
+        isTornDown = true
+        super.onDestroy()
+    }
+
     override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
         // Fallback for commands delivered via startService() when the service is already running
         handleAction(intent?.action)

app/src/test/kotlin/io/homeassistant/companion/android/assist/service/AssistVoiceInteractionServiceTest.kt

@@ -328,6 +328,62 @@ class AssistVoiceInteractionServiceTest {
         service.onShutdown()
     }
 
+    @Test
+    fun `Given service shut down when onReady delivered again then do not register receiver`() = runTest {
+        service.onReady()
+        advanceUntilIdle()
+        service.onShutdown()
+        advanceUntilIdle()
+
+        service.onReady()
+        advanceUntilIdle()
+
+        assertTrue(ACTION_START_LISTENING !in getRegisteredReceiverActions())
+    }
+
+    @Test
+    fun `Given service shut down when onReady delivered and wake word enabled then do not start listening`() = runTest {
+        coEvery { assistConfigManager.isWakeWordEnabled() } returns true
+        coEvery { assistConfigManager.getSelectedWakeWordModel() } returns microWakeWordModelConfigs[0]
+
+        service.onShutdown()
+        advanceUntilIdle()
+
+        service.onReady()
+        advanceUntilIdle()
+
+        coVerify(exactly = 0) { wakeWordListener.start(any(), any()) }
+    }
+
+    @Test
+    fun `Given service destroyed when onReady delivered again then do not register receiver`() = runTest {
+        service.onDestroy()
+
+        service.onReady()
+        advanceUntilIdle()
+
+        assertTrue(ACTION_START_LISTENING !in getRegisteredReceiverActions())
+    }
+
+    @Test
+    fun `Given receiver already unregistered when onShutdown then do not crash`() = runTest {
+        service.onReady()
+        advanceUntilIdle()
+
+        // Simulate the framework removing the registration out from under the service
+        val application = ApplicationProvider.getApplicationContext<Application>()
+        val receiver = Shadows.shadowOf(application).registeredReceivers
+            .first { wrapper ->
+                (0 until wrapper.intentFilter.countActions())
+                    .any { wrapper.intentFilter.getAction(it) == ACTION_START_LISTENING }
+            }
+            .broadcastReceiver
+        application.unregisterReceiver(receiver)
+
+        // Should not throw
+        service.onShutdown()
+    }
+
     @Test
     fun `Given context when startListening then send START_LISTENING broadcast with package`() {
         assertAction(ACTION_START_LISTENING, AssistVoiceInteractionService::startListening)