diff --git a/app/src/main/kotlin/io/homeassistant/companion/android/assist/service/AssistVoiceInteractionService.kt b/app/src/main/kotlin/io/homeassistant/companion/android/assist/service/AssistVoiceInteractionService.kt index 0601410d784..20e8b19f427 100644 --- a/app/src/main/kotlin/io/homeassistant/companion/android/assist/service/AssistVoiceInteractionService.kt +++ b/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) diff --git a/app/src/test/kotlin/io/homeassistant/companion/android/assist/service/AssistVoiceInteractionServiceTest.kt b/app/src/test/kotlin/io/homeassistant/companion/android/assist/service/AssistVoiceInteractionServiceTest.kt index 3691fd7b1be..d1ab002f1c8 100644 --- a/app/src/test/kotlin/io/homeassistant/companion/android/assist/service/AssistVoiceInteractionServiceTest.kt +++ b/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() + 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)