Skip to content

Commit

Permalink
Avoid DefaultDrmSessionManager releasing too many session references
Browse files Browse the repository at this point in the history
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

#minor-release

PiperOrigin-RevId: 385580741
  • Loading branch information
icbaker committed Jul 20, 2021
1 parent 135eb3f commit 22ab148
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 2 deletions.
6 changes: 6 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@
using a `ForwardingPlayer` that overrides `getSeekBackIncrement`,
`seekBack`, `getSeekForwardIncrement` and `seekForward`. The rewind and
fast forward buttons can be disabled by using a `ForwardingPlayer` that
`seekBack`, `getSeekForwardIncrement` and `seekForward`. The
corresponding buttons can be disabled by using a `ForwardingPlayer` that
removes `COMMAND_SEEK_BACK` and `COMMAND_SEEK_FORWARD` from the
available commands.
* Update `DefaultControlDispatcher` `getRewindIncrementMs` and
Expand Down Expand Up @@ -147,6 +149,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)).
* PlayerNotificationManager:
* Add `PendingIntent.FLAG_IMMUTABLE` flag to BroadcastReceiver to support
Android 12.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -914,8 +914,10 @@ public void onReferenceCountIncremented(DefaultDrmSession session, int newRefere

@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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,44 @@ public void managerRelease_mediaDrmNotReleasedUntilLastSessionReleased() throws
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<DrmInitData.SchemeData> secondSchemeDatas =
Expand Down

0 comments on commit 22ab148

Please sign in to comment.