`IllegalStateException` when receiving media button event in the background

naveensingh commented 12 months ago

Version

Media3 pre-release (alpha, beta or RC not in this list)

More version details

Version 1.2.0-alpha02

Devices that reproduce the issue

Reproducible on all Android versions, check the screenshot in the Actual result section

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Not tested

Reproduction steps

Play any audio file.
Pause playback.
Close app.
Force stop the app.
Resume playback using a bluetooth headset.
Watch the session service crash.

Expected result

The app starts playback successfully.

Actual result

Stacktrace:

FATAL EXCEPTION: main
Process: com.simplemobiletools.musicplayer.debug, PID: 2305
java.lang.RuntimeException: Unable to start service com.simplemobiletools.musicplayer.playback.PlaybackService@6bb0831 with Intent { act=android.intent.action.MEDIA_BUTTON flg=0x10000010 cmp=com.simplemobiletools.musicplayer.debug/com.simplemobiletools.musicplayer.playback.PlaybackService (has extras) }: java.lang.IllegalStateException: Future was expected to be done: androidx.media3.session.MediaControllerHolder@e2fdcd8[status=PENDING]
    at android.app.ActivityThread.handleServiceArgs(ActivityThread.java:5261)
    at android.app.ActivityThread.-$$Nest$mhandleServiceArgs(Unknown Source:0)
    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2447)
    at android.os.Handler.dispatchMessage(Handler.java:106)
    at android.os.Looper.loopOnce(Looper.java:226)
    at android.os.Looper.loop(Looper.java:313)
    at android.app.ActivityThread.main(ActivityThread.java:8762)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:604)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1067)
Caused by: java.lang.IllegalStateException: Future was expected to be done: androidx.media3.session.MediaControllerHolder@e2fdcd8[status=PENDING]
    at com.google.common.base.Preconditions.checkState(Preconditions.java:590)
    at com.google.common.util.concurrent.Futures.getDone(Futures.java:1147)
    at androidx.media3.session.MediaNotificationManager.getConnectedControllerForSession(MediaNotificationManager.java:325)
    at androidx.media3.session.MediaNotificationManager.onMediaButtonEvent(MediaNotificationManager.java:143)
    at androidx.media3.session.MediaSessionService.onStartCommand(MediaSessionService.java:431)
    at android.app.ActivityThread.handleServiceArgs(ActivityThread.java:5243)
    at android.app.ActivityThread.-$$Nest$mhandleServiceArgs(Unknown Source:0)
    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2447)
    at android.os.Handler.dispatchMessage(Handler.java:106)
    at android.os.Looper.loopOnce(Looper.java:226)
    at android.os.Looper.loop(Looper.java:313)
    at android.app.ActivityThread.main(ActivityThread.java:8762)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:604)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1067)

Crash statistics:

data

Media

Not applicable.

Bug Report

[ ] You will email the zip file produced by adb bugreport to android-media-github@google.com after filing this issue.

naveensingh commented 12 months ago

@tonihei (tagging you to make sure the issue isn't ignored/passed silently into the final 1.2.0 release)

ffd7bb5639f3c25b877f3689eadf07bdbfdee2b0 seems to be the commit that introduced this issue.

tonihei commented 12 months ago

Thanks for reporting! We just reworked this part of the code to make it more robust (for reasons unrelated to this IllegalStateException). See https://github.com/androidx/media/commit/64bd3bcad3fa4b0e433b16d583456920afad3ce2 (from yesterday) that removed the code path listed above.

@marcbaechinger Do you reckon the "not-yet-connected-controller" issue that was reported here could somehow still be a problem after the recent change? I guess not anymore since we are not directly using a controller for this now, right?

marcbaechinger commented 12 months ago

Yes, agreed this should be fixed in the next release for several reasons (including the CL regarding adding MediaSession.Callback.onMediaButtonEvent() that I just sent for internal review).

After these changes, the strongest reason why this can't happen anymore is that a) we use a fallback if the media notification controller is not connected. This fallback would also prevent crashes if the media notification controller is not connected at all, because then the event would go the same path through sessionCompat.getMediaController().dispatchMediaButtonEvent() as it was before. So the worst that can happen would be to receive the event through the platform session as before.

For b) the change in internal review also post a Runnable to the application thread of the session from within onStartCommand. Only that runnable in the app thread then calls MediaSessionImpl.onMediaButtonEvent. Hence, attempting to get the ControllerInfo of the media notification controller happens in a new Looper event (no postOrRun) on the app thread. When this runnable is posted, the media notification controller is already connecting on the application thread and adds the ControllerInfo to the ConnectedControllersManager.

I'm still trying to repro with beta-01 and will comment again here in the issue when the second CL currently in review landed here on the main branch.

androidx / media