androidx / media

Jetpack Media3 support libraries for media use cases, including ExoPlayer, an extensible media player for Android
https://developer.android.com/media/media3
Apache License 2.0
1.71k stars 410 forks source link

java.lang.IllegalArgumentException: session is already released #422

Open PaulWoitaschek opened 1 year ago

PaulWoitaschek commented 1 year ago

Media3 Version

Media3 1.0.1

Devices that reproduce the issue

grafik grafik

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Not tested

Reproduction steps

  1. Rollout Voice 7.0.12: https://github.com/PaulWoitaschek/Voice/blob/7.0.12/playback/src/main/kotlin/voice/playback/session/PlaybackService.kt
  2. See crashes in Crashlytics

Expected result

It doesn't crash

Actual result

I'm seeing multilpe permutations of this crash.

The biggest one is this, which is strange because that version of Voice does not declare any MEDIA_BUTTON receiver.

Fatal Exception: java.lang.RuntimeException: Unable to start service voice.playback.session.PlaybackService@60513f1 with Intent { act=android.intent.action.MEDIA_BUTTON dat=androidx.media3.session.MediaSessionImpl:/2360112758 flg=0x10000000 cmp=de.ph1b.audiobook/voice.playback.session.PlaybackService (has extras) }: java.lang.IllegalArgumentException: session is already released
       at android.app.ActivityThread.handleServiceArgs(ActivityThread.java:4808)
       at android.app.ActivityThread.access$2100(ActivityThread.java:271)
       at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2163)
       at android.os.Handler.dispatchMessage(Handler.java:106)
       at android.os.Looper.loopOnce(Looper.java:210)
       at android.os.Looper.loop(Looper.java:299)
       at android.app.ActivityThread.main(ActivityThread.java:8250)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:556)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1045)

Caused by java.lang.IllegalArgumentException: session is already released
       at androidx.media3.common.util.Assertions.checkArgument(Assertions.java:1)
       at androidx.media3.session.MediaSessionService.addSession(MediaSessionService.java:10)
       at androidx.media3.session.MediaSessionService.onStartCommand(MediaSessionService.java:120)
       at android.app.ActivityThread.handleServiceArgs(ActivityThread.java:4790)
       at android.app.ActivityThread.access$2100(ActivityThread.java:271)
       at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2163)
       at android.os.Handler.dispatchMessage(Handler.java:106)
       at android.os.Looper.loopOnce(Looper.java:210)
       at android.os.Looper.loop(Looper.java:299)
       at android.app.ActivityThread.main(ActivityThread.java:8250)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:556)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1045)

Media

not media related

Bug Report

marcbaechinger commented 1 year ago

Thanks for your report! This is puzzling indeed.

that version of Voice does not declare any MEDIA_BUTTON receiver.

All these crashes are on devices with Android 12 and below (at least if sonstige (1) isn't Android 13). It looks to me as this is a PendingIntent sent from a notification that arrives in onStartCommand. I think this part is working as intended.

The exception is thrown because the session is already released. Which is a bit strange when looking at your service implementation (which, as an aside, looks quite slick with the injection approach :). It doesn't seem that you ever call session.release() except onTaskRemoved and onDestroy() is called. Which should terminate the service immediately and together with releasing the session the notification is removed.

It's quite odd, that the session that is returned here is then, two lines below when passed to addSession(session) already released.

From your code (that looks correct to me), an explanation would be that there is a race between onTaskRemoved() and receiving the onStartCommand call with the media button. However, this is a long shot and I don't know how this manually should be performed by a user.

It may be cause by a memory leak of the service, that is fixed since 1.0.2 (see #346).

So what I would try:

(1) You can check with canary leak whether your current app exhibits the memory leak problem. This still wouldn't explain why there is still a notification when the session is released, but this would be interesting to now. If that's the case upgrading to 1.0.2 is an option.

(2) If my above guessing is true, you could try to change onGetSession in your service to return null when session.isReleased() returns true.

I think that this could be a workaround. We could even do this check ourselves in MediaSessionService. Although without better understanding why this happens, this is a bit of a guess work.

Can you give me an idea on how many users are affected (proportionally to all users)?

Sorry for not being able to get a better answer for now,

PaulWoitaschek commented 1 year ago

Thanks for your help!

(1) You can check with canary leak whether your current app exhibits the memory leak problem. This still wouldn't explain why there is still a notification when the session is released, but this would be interesting to now. If that's the case upgrading to 1.0.2 is an option.

I added leakcanary but it did not detect any leaks so far.

(2) If my above guessing is true, you could try to change onGetSession in your service to return null when session.isReleased() returns true.

I can't do that as it's package private.

Can you give me an idea on how many users are affected (proportionally to all users)?

Voice 7.0.12 (the one where this was originally reported)

Voice 7.0.14

PaulWoitaschek commented 1 year ago

Okay, entering the badlands now: https://github.com/PaulWoitaschek/Voice/commit/773a4066ec0b8450f80d0b458c3fd89be3f644dd

PaulWoitaschek commented 1 year ago

@marcbaechinger This actually solved the crash!

mhelder commented 9 months ago

Just dropping a comment here to maybe add some additional information.

We're seeing the same crash logged in Crashlytics using media3-1.2.0. Exact same stack trace as posted up top, albeit some different line numbers in various places. Some stats:

Please note that the Samsung + Android 14 stats may be a coincidence/market reflection of our user base and their locality.

Crashlytics also labels this as "early crash", saying: 92% of crash events for this issue happened in the first 5 seconds of a user's session.

We're still investigating repro steps. In the meantime, I was wondering if there were any new insights in what could be causing this issue? I could blindly apply the suggested workaround to see if the issue goes away of course, but I as @marcbaechinger suggested: it'd be good to have a better understanding why this happens. Given that the aforementioned workaround requires reflection, I can't help but wonder if this should maybe be patched in the media framework instead?

ksvslk commented 2 months ago

Also happening with Media3 1.4.0

androidx.media3.session.MediaSessionService.addSession java.lang.IllegalArgumentException

  1. All Samsung so far
  2. All Android 14
  3. All background (NOT user-perceived) - not sure if users are seeing the crash dialog or not
Exception java.lang.RuntimeException:
  at android.app.ActivityThread.handleServiceArgs (ActivityThread.java:5286)
  at android.app.ActivityThread.-$$Nest$mhandleServiceArgs
  at android.app.ActivityThread$H.handleMessage (ActivityThread.java:2531)
  at android.os.Handler.dispatchMessage (Handler.java:106)
  at android.os.Looper.loopOnce (Looper.java:230)
  at android.os.Looper.loop (Looper.java:319)
  at android.app.ActivityThread.main (ActivityThread.java:8919)
  at java.lang.reflect.Method.invoke
  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:578)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1103)
Caused by java.lang.IllegalArgumentException: session is already released
  at androidx.media3.common.util.Assertions.checkArgument (Assertions.java:55)
  at androidx.media3.session.MediaSessionService.addSession (MediaSessionService.java:264)
  at androidx.media3.session.MediaSessionService.onStartCommand (MediaSessionService.java:429)
  at android.app.ActivityThread.handleServiceArgs (ActivityThread.java:5268)

EDIT - more info:

RuntimeException
Unable to start service my.package.MyMediaSessionService@40aca3s with Intent {act=android.intent.action.MEDIA_BUTTON dat=androidx.media3.session.MediaSessionImpl:/...cmp=my.package/MyMediaSessionService (has extras) }: java.lang.IllegalArgumentException: session is already released

Implementation:

EDIT 2 (solved):

To reproduce - only Android 14 and Samsung:

  1. Play something
  2. Put app to the background
  3. Open notifications tray
  4. Hit pause
  5. Remove task/app from recents
  6. Observe crash (with or without the "This app has a bug..." dialogue)

Fix: Add session = null to the onTaskRemoved Maybe it should be added to: https://developer.android.com/media/media3/session/background-playback