Fixes exception when audioSwitch.stop() gets called before start()

[pulakdp]

On upgrade of livekit SDK from 2.24.0 to 2.25.2, I observed a fresh crash with the following stack trace:

Fatal Exception: java.lang.IllegalArgumentException: attempt to call removeOnCommunicationDeviceChangedListener on an unregistered listener
       at android.media.CallbackUtil.removeListener(CallbackUtil.java:185)
       at android.media.CallbackUtil$LazyListenerManager.removeListener(CallbackUtil.java:294)
       at android.media.AudioManager.removeOnCommunicationDeviceChangedListener(AudioManager.java:9806)
       at com.twilio.audioswitch.scanners.CommunicationDeviceScanner.stop(CommunicationDeviceScanner.kt:61)
       at com.twilio.audioswitch.AbstractAudioSwitch.closeListeners(AbstractAudioSwitch.java:383)
       at com.twilio.audioswitch.AbstractAudioSwitch.stop(AbstractAudioSwitch.java:261)
       at io.livekit.android.audio.AudioSwitchHandler.stop$lambda$4(AudioSwitchHandler.kt:266)
       at android.os.Handler.handleCallback(Handler.java:1014)
       at android.os.Handler.dispatchMessage(Handler.java:102)
       at android.os.Looper.loopOnce(Looper.java:250)
       at android.os.Looper.loop(Looper.java:340)
       at android.os.HandlerThread.run(HandlerThread.java:108)

This happens because LiveKit now uses CommDeviceAudioSwitch for Android 12 and above which unconditionally calls removeOnCommunicationDeviceChangedListener() without checking if the listener was ever registered. Since start() and stop() are called using same Handler, in some cases this leads to a race condition where start() is not yet called before stop() executes leading to this crash. Added simple boolean guardrail to avoid this.

I have added some tests which demonstrates the issue when guardrail is not there.

[changeset-bot]

🦋 Changeset detected

Latest commit: 0324455

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
client-sdk-android	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[davidliu]

Hmm, AudioSwitch itself has its own state management which shouldn't call through to the underlying cleanup if it hasn't started yet. Simply calling stop() before start() does not result in a crash. The race condition you added by using reflection to change the state to STARTED in your test doesn't seem possible in production unless you're also using reflection to alter the inner state of AudioSwitch in production?

[pulakdp]

I was not using reflection to alter states but at the same time I was not able to repro the issue. I saw a good number of crashes due to this specially on Vivo devices. However, I saw your PR davidliu/audioswitch#9 is a better fix directly at the site of exception. I'll try version 2.25.3 and report back if this persists. Thanks for the fix.