From dbebd279c41abb18e168fbebbb269feccfd8879b Mon Sep 17 00:00:00 2001 From: ibaker Date: Mon, 19 Jul 2021 18:05:47 +0100 Subject: [PATCH] Avoid DefaultDrmSessionManager releasing too many session references Before this fix, if DefaultDrmSessionManager.release() was called while there was at least one 'external' session reference still active (i.e. session.referenceCount > 1) then the manager will release it's reference immediately but when the session's reference count subsequently drops to 1 (due to external references being released) the manager will schedule a task to release its internal reference *again*. This change fixes the problem by only scheduling the timed release if the manager is unreleased. This ensures that the internal references are only released once. Issue: #9193 PiperOrigin-RevId: 385580741 --- RELEASENOTES.md | 4 ++ .../drm/DefaultDrmSessionManager.java | 6 ++- .../drm/DefaultDrmSessionManagerTest.java | 38 +++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 80205b8e74..c2d8e4734d 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -45,6 +45,10 @@ ([#9106](https://github.com/google/ExoPlayer/issues/9106). * DRM: * Allow repeated provisioning in `DefaultDrmSession(Manager)`. + * Fix a crash due to `DefaultDrmSessionManager.release()` incorrectly + releasing too many keep-alive `DefaultDrmSession` references, resulting + in `DefaultDrmSession.release()` throwing an `IllegalStateException` + ([#9193](https://github.com/google/ExoPlayer/issues/9193)). * Metadata: * Fix handling of emsg messages with an unset duration ([#9123](https://github.com/google/ExoPlayer/issues/9123)). diff --git a/library/core/src/main/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManager.java b/library/core/src/main/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManager.java index 093257e870..5680627b32 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManager.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManager.java @@ -911,8 +911,10 @@ public class DefaultDrmSessionManager implements DrmSessionManager { @Override public void onReferenceCountDecremented(DefaultDrmSession session, int newReferenceCount) { - if (newReferenceCount == 1 && sessionKeepaliveMs != C.TIME_UNSET) { - // Only the internal keep-alive reference remains, so we can start the timeout. + if (newReferenceCount == 1 && prepareCallsCount > 0 && sessionKeepaliveMs != C.TIME_UNSET) { + // Only the internal keep-alive reference remains, so we can start the timeout. We only + // do this if the manager isn't released, because a released manager has already released + // all its internal session keep-alive references. keepaliveSessions.add(session); checkNotNull(playbackHandler) .postAtTime( diff --git a/library/core/src/test/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManagerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManagerTest.java index 0b9f560546..4dac826ce2 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManagerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManagerTest.java @@ -222,6 +222,44 @@ public class DefaultDrmSessionManagerTest { exoMediaDrm.release(); } + // https://github.com/google/ExoPlayer/issues/9193 + @Test(timeout = 10_000) + public void + managerReleasedBeforeSession_keepaliveEnabled_managerOnlyReleasesOneKeepaliveReference() + throws Exception { + FakeExoMediaDrm.LicenseServer licenseServer = + FakeExoMediaDrm.LicenseServer.allowingSchemeDatas(DRM_SCHEME_DATAS); + FakeExoMediaDrm exoMediaDrm = new FakeExoMediaDrm.Builder().build(); + DrmSessionManager drmSessionManager = + new DefaultDrmSessionManager.Builder() + .setUuidAndExoMediaDrmProvider(DRM_SCHEME_UUID, new AppManagedProvider(exoMediaDrm)) + .setSessionKeepaliveMs(10_000) + .build(/* mediaDrmCallback= */ licenseServer); + + drmSessionManager.prepare(); + DrmSession drmSession = + checkNotNull( + drmSessionManager.acquireSession( + /* playbackLooper= */ checkNotNull(Looper.myLooper()), + /* eventDispatcher= */ null, + FORMAT_WITH_DRM_INIT_DATA)); + waitForOpenedWithKeys(drmSession); + + // Release the manager (there's still an explicit reference to the session from acquireSession). + // This should immediately release the manager's internal keepalive session reference. + drmSessionManager.release(); + assertThat(drmSession.getState()).isEqualTo(DrmSession.STATE_OPENED_WITH_KEYS); + + // Ensure the manager doesn't release a *second* keepalive session reference after the timer + // expires. + ShadowLooper.idleMainLooper(10, SECONDS); + assertThat(drmSession.getState()).isEqualTo(DrmSession.STATE_OPENED_WITH_KEYS); + + // Release the explicit session reference. + drmSession.release(/* eventDispatcher= */ null); + assertThat(drmSession.getState()).isEqualTo(DrmSession.STATE_RELEASED); + } + @Test(timeout = 10_000) public void maxConcurrentSessionsExceeded_allKeepAliveSessionsEagerlyReleased() throws Exception { ImmutableList secondSchemeDatas =