Only unregister actionReceiver when it was registered#6628
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a crash in AssistVoiceInteractionService where onShutdown() could attempt to unregister a broadcast receiver before it had been registered, based on Sentry reports of fast assistant switching/race conditions.
Changes:
- Make
actionReceivernullable and only unregister it when non-null - Instantiate/register the receiver during
onReady()rather than at property initialization - Add a regression test ensuring
onShutdown()beforeonReady()does not crash
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/src/main/kotlin/io/homeassistant/companion/android/assist/service/AssistVoiceInteractionService.kt | Guard receiver unregistration by tracking registration state via a nullable receiver reference |
| app/src/test/kotlin/io/homeassistant/companion/android/assist/service/AssistVoiceInteractionServiceTest.kt | Add regression coverage for calling onShutdown() before onReady() |
| /** Non-null only while the receiver is registered (between [onReady] and [onShutdown]). */ | ||
| private var actionReceiver: BroadcastReceiver? = null | ||
|
|
||
| override fun onReady() { | ||
| super.onReady() | ||
| isServiceReady = true | ||
| Timber.d("VoiceInteractionService is ready") | ||
| actionReceiver = object : BroadcastReceiver() { | ||
| override fun onReceive(context: Context, intent: Intent) { | ||
| handleAction(intent.action) | ||
| } | ||
| } | ||
| ContextCompat.registerReceiver( | ||
| this, | ||
| actionReceiver, |
There was a problem hiding this comment.
The new KDoc says actionReceiver is non-null only while the receiver is registered, but it’s assigned before ContextCompat.registerReceiver(...) is called. If registration were to throw, actionReceiver would remain non-null even though it was never registered, and onShutdown could still attempt to unregister it. Consider creating the receiver as a local val, registering it, and only then assigning it to actionReceiver (or adjust the KDoc to match the actual lifecycle).
There was a problem hiding this comment.
I think it is acceptable tbh if it crashes on registerReceiver the app crashes already.
| // Reproduces Sentry:ANDROID-27RA: onShutdown called before onReady should not throw | ||
| // IllegalArgumentException for unregistering a receiver that was never registered |
There was a problem hiding this comment.
Please remove the reference to the private Sentry issue or include a link to the PR/GitHub with the trace. This currently doesn't help the majority of contributors without access to Sentry. (I think the description after the Sentry reference is descriptive enough and it can be removed.)
Summary
Fix a crash from Sentry where we unregistered the
actionReceiverbefore we actually register it in onReady. I must be hard to reproduce like switching the service very quick from HA to Google for instance. This PR guard this behavior by only setting the attribute when it is registered.Checklist
Raw error from Sentry
IllegalArgumentException: Receiver not registered: io.homeassistant.companion.android.assist.service.AssistVoiceInteractionService$actionReceiver$1@ae8f1fb
Project: android
Date: 3/28/2026, 2:29:33 PM
Issue Summary
Unregistered Receiver Exception in Assist Service Shutdown
What's wrong: IllegalArgumentException during receiver unregistration in AssistVoiceInteractionService.
In the trace: Breadcrumbs show only device orientation changes; no direct link to the crash.
Possible cause: The actionReceiver was likely unregistered prematurely or twice during service shutdown.
Tags
Exception
Exception 1
Type: IllegalArgumentException
Value: Receiver not registered: io.homeassistant.companion.android.assist.service.AssistVoiceInteractionService$actionReceiver$1@ae8f1fb
Stacktrace