Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IllegalStateException when releasing DefaultDrmSession #9392

Closed
fpitters opened this issue Sep 6, 2021 · 8 comments
Closed

IllegalStateException when releasing DefaultDrmSession #9392

fpitters opened this issue Sep 6, 2021 · 8 comments

Comments

@fpitters
Copy link

fpitters commented Sep 6, 2021

We are seeing a weird crash (on Firebase Crashlytics) after bumping to 2.14.2 (from 2.13.2) with the following stacktrace:

Fatal Exception: java.lang.IllegalStateException
       at com.google.android.exoplayer2.util.Assertions.checkState(Assertions.java:84)
       at com.google.android.exoplayer2.drm.DefaultDrmSession.release(DefaultDrmSession.java:308)
       at com.google.android.exoplayer2.drm.DefaultDrmSessionManager$PreacquiredSessionReference.lambda$release$1(DefaultDrmSessionManager.java:1015)
       at com.google.android.exoplayer2.drm.DefaultDrmSessionManager$PreacquiredSessionReference.lambda$release$1$DefaultDrmSessionManager$PreacquiredSessionReference(DefaultDrmSessionManager.java:3)
       at com.google.android.exoplayer2.drm.-$$Lambda$DefaultDrmSessionManager$PreacquiredSessionReference$nYc-XCZkgUmlTVMJKF9VQKrj-NU.run(-.java:3)
       at android.os.Handler.handleCallback(Handler.java:761)
       at android.os.Handler.dispatchMessage(Handler.java:98)
       at android.os.Looper.loop(Looper.java:156)
       at android.os.HandlerThread.run(HandlerThread.java:61)

We haven't been able to reproduce it locally.

The incidence is low, few ~5 out of 25K a day and not device specific, happening across all brands. Although Huawei devices have 40% of the share.

We use Dash with Widevine encryption.

I thought this issue was related: #9193 but it might be is not.

We'll update the status in the upcoming weeks once we move to 2.15.0.

Thanks

@icbaker
Copy link
Collaborator

icbaker commented Sep 9, 2021

The stack trace provided indicates that a PreacquiredSessionReference is trying to release an underlying DefaultDrmSession that's already fully released (referenceCount == 0).

I've spent some time studying the code in DefaultDrmSessionManager and I haven't come up with a sequence of operations that would allow this to happen. Every PreacquiredSessionReference instance 'holds' a single reference count of its underlying DefaultDrmSession and nobody should be releasing that reference except the PreacquiredSessionReference (and it contains logic to ensure it only releases it at most once). Of course there might be a bug somewhere, but without being able to reproduce it (which is understandable because it sounds like a rare race condition) it's hard to do anything except comb through the code by hand.

I wonder if you could provide some extra info about how your app configures DRM playback that might help guide me towards an answer:

  • Are you using MediaItem's DRM fields or passing MediaSource instances directly to the player?
  • Are you directly instantiating DefaultDrmSessionManager and/or providing a custom DrmSessionManagerProvider to MediaSourceFactory?
  • If you're directly instantiating DefaultDrmSessionManager do you ever directly call prepare() or release() on it? (separately to any calls from ExoPlayer library code).
  • Do you customise or disable the DefaultDrmSessionManager.Builder#setSessionKeepaliveTimeoutMs() property?

@google-oss-bot
Copy link
Collaborator

Hey @fpitters. We need more information to resolve this issue but there hasn't been an update in 14 weekdays. I'm marking the issue as stale and if there are no new updates in the next 7 days I will close it automatically.

If you have more information that will help us get to the bottom of this, just add a comment!

@Luisemtza
Copy link

Hello, we have the same problem. We are using version 2.15 and we cannot replicate the problem, we can only see it through Firebase Crashlytics

com.google.android.exoplayer2.util.Assertions.checkState (Assertions.java:84)
com.google.android.exoplayer2.drm.DefaultDrmSession.release (DefaultDrmSession.java:314)
com.google.android.exoplayer2.drm.DefaultDrmSessionManager$PreacquiredSessionReference.lambda$release$1 (DefaultDrmSessionManager.java:1017)
com.google.android.exoplayer2.drm.DefaultDrmSessionManager$PreacquiredSessionReference.lambda$release$1$DefaultDrmSessionManager$PreacquiredSessionReference (DefaultDrmSessionManager.java)
com.google.android.exoplayer2.drm.-$$Lambda$DefaultDrmSessionManager$PreacquiredSessionReference$nYc-XCZkgUmlTVMJKF9VQKrj-NU.run (-.java:2)
android.os.Handler.handleCallback (Handler.java:938)
android.os.Handler.dispatchMessage (Handler.java:99)
android.os.Looper.loop (Looper.java:250)
android.os.HandlerThread.run (HandlerThread.java:67)

Are you using MediaItem's DRM fields or passing MediaSource instances directly to the player?

We are using MediaSource

Are you directly instantiating DefaultDrmSessionManager and/or providing a custom DrmSessionManagerProvider to MediaSourceFactory?

We are directly instantiating DefaultDrmSessionManager

If you're directly instantiating DefaultDrmSessionManager do you ever directly call prepare() or release() on it? (separately to any calls from ExoPlayer library code).

No, We don't call directly this methods.

Do you customise or disable the DefaultDrmSessionManager.Builder#setSessionKeepaliveTimeoutMs() property?

No, We don't are setting any value.

I hope it can help with the solution.

@icbaker
Copy link
Collaborator

icbaker commented Sep 29, 2021

@Luisemtza Are you able to post the code snippet you use to instantiate your DefaultDrmSessionManager?

Do you instantiate a new DefaultDrmSessionManager for every MediaSource or re-use them? If you re-use them, do you ever re-use them across multiple Player instances?

@Luisemtza
Copy link

Luisemtza commented Sep 29, 2021

Hi @icbaker, of corse:

private DefaultDrmSessionManager getDefaultDrmSessionManager(
    DefaultHttpDataSourceFactory dataSourceFactory,
    ConfigPlayer configPlayer,
    String phonogramId) {
  DefaultDrmSessionManager.Builder builder = new DefaultDrmSessionManager.Builder();
  if (configPlayer != null && configPlayer.forceUseLV3()) {
    builder.setUuidAndExoMediaDrmProvider(
        C.WIDEVINE_UUID,
        uuid -> {
          try {
	    FrameworkMediaDrm mediaDrm = FrameworkMediaDrm.newInstance(uuid);
	    mediaDrm.setPropertyString(WidevineUtils.SECURITY_LEVEL, WidevineUtils.LEVEL_3);
	    return mediaDrm;
	} catch (UnsupportedDrmException e) {
	  return null;
        }
      });
  }
  return builder.build(new DrmCallback(dataSourceFactory, configPlayer, phonogramId, true));
}

DrmCallBack is our imlpementation of MediaDrmCallback.

We are instantiate a new DefaultDrmSessionManager each MediaSource.

Regards

@icbaker
Copy link
Collaborator

icbaker commented Sep 30, 2021

Thanks! That all looks pretty normal, nothing suspicious jumps out that would relate to the exception in this issue (which suggests there is a bug somewhere in DefaultDrmSessionManager, we just can't find it).


Aside: This is not correct - you shouldn't be returning null from ExoMediaDrm.Provider#acquireExoMediaDrm. If this line ever executes it will almost immediately generate an NPE inside DefaultDrmSessionManager:

} catch (UnsupportedDrmException e) {
  return null;
}

You should return a DummyExoMediaDrm like FrameworkMediaDrm#DEFAULT_PROVIDER does (in fact you could rewrite your implementation to just wrap a call to FrameworkMediaDrm#DEFAULT_PROVIDER to avoid having to handle this exception yourself).

ojw28 pushed a commit that referenced this issue Oct 18, 2021
Issue: #9392 reports occasional IllegalStateExceptions from release()
in crashlytics,`with no way to reproduce locally. It seems likely there
is a bug somewhere in DRM handling, and ideally we would find that and
fix it.

However we haven't been able to find the problem, and in the meantime
these exceptions cause the entire app to crash. Although this is
arguably useful from a debugging perspective, it's obviously a poor
experience for developers and users, since all we're actually trying to
do is release the session, so maybe we shouldn't strictly care that it's
already released?

This change replaces the exception with an error log, which might be a
useful debugging hint if we see other DRM unexpected behaviour due to
references to released sessions being held for too long.

PiperOrigin-RevId: 403942546
@google-oss-bot
Copy link
Collaborator

Hey @fpitters. We need more information to resolve this issue but there hasn't been an update in 14 weekdays. I'm marking the issue as stale and if there are no new updates in the next 7 days I will close it automatically.

If you have more information that will help us get to the bottom of this, just add a comment!

@icbaker
Copy link
Collaborator

icbaker commented Oct 20, 2021

The change linked above stops throwing an exception in this case (logs an error instead). I'm going to close this issue, even though this isn't quite as satisfactory as finding & fixing the actual underlying bug.

@icbaker icbaker closed this as completed Oct 20, 2021
@google google locked and limited conversation to collaborators Dec 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants