Delay unbinding the controller until pending commands are handled.

Once a controller bound to a MediaSessionService unbinds, this service
may be destroyed immediately if this was the last bound controller and
the service isn't started in the foreground.

At the time of releasing the controller, there may still be pending
commands that need to be handled by the session service. These commands
may cause the session service to post a foreground notification to
keep it alive. So to avoid the destruction of the service while these
commands are still handled, we need to keep the controller bound.

We also add a timeout in case the session tasks are never completed
by the session implementation. In case the controller is destroyed,
the unbinding happens automatically by the system.

PiperOrigin-RevId: 463802220
This commit is contained in:
tonihei 2022-07-28 11:10:36 +00:00
parent 7f08ab729a
commit ac42b593cc
7 changed files with 189 additions and 19 deletions

View File

@ -98,8 +98,10 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization;
* {@link MediaSession} in it. {@link MediaSessionService} will provide a session to connect.
*
* <p>When you're done, use {@link #releaseFuture(Future)} or {@link #release()} to clean up
* resources. This also helps session service to be destroyed when there's no controller associated
* with it.
* resources. This also helps the session service to be destroyed when there's no controller
* associated with it. Releasing the controller will still deliver all pending commands sent to the
* session and only unbind from the session service once these commands have been handled, or after
* a timeout of {@link #RELEASE_UNBIND_TIMEOUT_MS}.
*
* <h2 id="ThreadingModel">Threading Model</h2>
*
@ -161,6 +163,12 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization;
*/
public class MediaController implements Player {
/**
* The timeout for handling pending commands after calling {@link #release()}. If the timeout is
* reached, the controller is unbound from the session service even if commands are still pending.
*/
@UnstableApi public static final long RELEASE_UNBIND_TIMEOUT_MS = 30_000;
private static final String TAG = "MediaController";
private static final String WRONG_THREAD_ERROR_MESSAGE =

View File

@ -162,6 +162,8 @@ import org.checkerframework.checker.nullness.qual.NonNull;
public static final String TAG = "MCImplBase";
private static final long RELEASE_TIMEOUT_MS = 30_000;
private final MediaController instance;
protected final SequencedFutureManager sequencedFutureManager;
protected final MediaControllerStub controllerStub;
@ -315,14 +317,9 @@ import org.checkerframework.checker.nullness.qual.NonNull;
return;
}
released = true;
if (serviceConnection != null) {
context.unbindService(serviceConnection);
serviceConnection = null;
}
connectedToken = null;
flushCommandQueueHandler.release();
this.iSession = null;
controllerStub.destroy();
if (iSession != null) {
int seq = sequencedFutureManager.obtainNextSequenceNumber();
try {
@ -332,8 +329,16 @@ import org.checkerframework.checker.nullness.qual.NonNull;
// No-op.
}
}
sequencedFutureManager.release();
listeners.release();
sequencedFutureManager.lazyRelease(
RELEASE_TIMEOUT_MS,
() -> {
if (serviceConnection != null) {
context.unbindService(serviceConnection);
serviceConnection = null;
}
controllerStub.destroy();
});
}
@Override

View File

@ -29,6 +29,7 @@ import androidx.media3.common.util.Log;
import androidx.media3.session.MediaLibraryService.LibraryParams;
import java.lang.ref.WeakReference;
import java.util.List;
import org.checkerframework.checker.nullness.qual.NonNull;
/* package */ class MediaControllerStub extends IMediaController.Stub {
@ -41,7 +42,7 @@ import java.util.List;
}
@Override
public void onSessionResult(int seq, Bundle sessionResultBundle) {
public void onSessionResult(int sequenceNum, Bundle sessionResultBundle) {
SessionResult result;
try {
result = SessionResult.CREATOR.fromBundle(sessionResultBundle);
@ -52,11 +53,11 @@ import java.util.List;
// Don't post setting future result so the result can be obtained on the application looper.
// For an example, {@code MediaController.setRating(rating).get()} wouldn't return if the
// result is posted.
dispatchControllerTask(controller -> controller.setFutureResult(seq, result));
setControllerFutureResult(sequenceNum, result);
}
@Override
public void onLibraryResult(int seq, Bundle libraryResultBundle) {
public void onLibraryResult(int sequenceNum, Bundle libraryResultBundle) {
LibraryResult<?> result;
try {
result = LibraryResult.UNKNOWN_TYPE_CREATOR.fromBundle(libraryResultBundle);
@ -67,8 +68,7 @@ import java.util.List;
// Don't post setting future result so the result can be obtained on the application looper.
// For an example, {@code MediaBrowser.getLibraryRoot(params).get()} wouldn't return if the
// result is posted.
dispatchControllerTask(
(ControllerTask<MediaBrowserImplBase>) browser -> browser.setFutureResult(seq, result));
setControllerFutureResult(sequenceNum, result);
}
@Override
@ -246,16 +246,15 @@ import java.util.List;
controller.clear();
}
private <T extends MediaControllerImplBase> void dispatchControllerTask(ControllerTask<T> task) {
private <T extends @NonNull Object> void setControllerFutureResult(
int sequenceNum, T futureResult) {
long token = Binder.clearCallingIdentity();
try {
@Nullable MediaControllerImplBase controller = this.controller.get();
if (controller == null || controller.isReleased()) {
if (controller == null) {
return;
}
@SuppressWarnings("unchecked")
T castedController = (T) controller;
task.run(castedController);
controller.setFutureResult(sequenceNum, futureResult);
} finally {
Binder.restoreCallingIdentity(token);
}

View File

@ -15,10 +15,14 @@
*/
package androidx.media3.session;
import static androidx.media3.common.util.Assertions.checkNotNull;
import android.os.Handler;
import androidx.annotation.GuardedBy;
import androidx.annotation.Nullable;
import androidx.collection.ArrayMap;
import androidx.media3.common.util.Log;
import androidx.media3.common.util.Util;
import com.google.common.util.concurrent.AbstractFuture;
import com.google.common.util.concurrent.ListenableFuture;
import java.util.ArrayList;
@ -39,6 +43,17 @@ import org.checkerframework.checker.nullness.qual.NonNull;
@GuardedBy("lock")
private final ArrayMap<Integer, SequencedFuture<?>> seqToFutureMap;
@GuardedBy("lock")
@Nullable
private Runnable pendingLazyReleaseCallback;
@GuardedBy("lock")
@Nullable
private Handler releaseCallbackHandler;
@GuardedBy("lock")
private boolean isReleased;
public SequencedFutureManager() {
lock = new Object();
seqToFutureMap = new ArrayMap<>();
@ -66,7 +81,11 @@ import org.checkerframework.checker.nullness.qual.NonNull;
synchronized (lock) {
int seq = obtainNextSequenceNumber();
SequencedFuture<T> result = SequencedFuture.create(seq, resultWhenClosed);
seqToFutureMap.put(seq, result);
if (isReleased) {
result.setWithTheValueOfResultWhenClosed();
} else {
seqToFutureMap.put(seq, result);
}
return result;
}
}
@ -94,20 +113,42 @@ import org.checkerframework.checker.nullness.qual.NonNull;
+ result.getClass());
}
}
if (pendingLazyReleaseCallback != null && seqToFutureMap.isEmpty()) {
release();
}
}
}
public void release() {
List<SequencedFuture<?>> pendingResults;
synchronized (lock) {
isReleased = true;
pendingResults = new ArrayList<>(seqToFutureMap.values());
seqToFutureMap.clear();
if (pendingLazyReleaseCallback != null) {
checkNotNull(releaseCallbackHandler).post(pendingLazyReleaseCallback);
pendingLazyReleaseCallback = null;
releaseCallbackHandler = null;
}
}
for (SequencedFuture<?> result : pendingResults) {
result.setWithTheValueOfResultWhenClosed();
}
}
public void lazyRelease(long timeoutMs, Runnable releaseCallback) {
synchronized (lock) {
Handler releaseCallbackHandler = Util.createHandlerForCurrentLooper();
this.releaseCallbackHandler = releaseCallbackHandler;
pendingLazyReleaseCallback = releaseCallback;
if (seqToFutureMap.isEmpty()) {
release();
} else {
releaseCallbackHandler.postDelayed(this::release, timeoutMs);
}
}
}
public static final class SequencedFuture<T extends @NonNull Object> extends AbstractFuture<T> {
private final int sequenceNumber;

View File

@ -38,6 +38,7 @@ dependencies {
implementation project(modulePrefix + 'lib-session')
implementation project(modulePrefix + 'test-session-common')
implementation 'androidx.media:media:' + androidxMediaVersion
androidTestImplementation project(modulePrefix + 'lib-exoplayer')
androidTestImplementation 'androidx.test.ext:junit:' + androidxTestJUnitVersion
androidTestImplementation 'androidx.test.ext:truth:' + androidxTestTruthVersion
androidTestImplementation 'androidx.test:core:' + androidxTestCoreVersion

View File

@ -25,8 +25,12 @@ import static org.junit.Assert.assertThrows;
import android.content.ComponentName;
import android.content.Context;
import android.os.Bundle;
import android.os.Handler;
import android.os.Looper;
import androidx.media3.common.MediaItem;
import androidx.media3.common.Player;
import androidx.media3.common.util.ConditionVariable;
import androidx.media3.exoplayer.ExoPlayer;
import androidx.media3.session.MediaSession.ControllerInfo;
import androidx.media3.test.session.common.HandlerThreadTestRule;
import androidx.media3.test.session.common.MainLooperTestRule;
@ -34,11 +38,14 @@ import androidx.media3.test.session.common.TestUtils;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.filters.MediumTest;
import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.SettableFuture;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicBoolean;
import org.junit.After;
import org.junit.Before;
import org.junit.ClassRule;
@ -79,6 +86,65 @@ public class MediaSessionServiceTest {
TestServiceRegistry.getInstance().cleanUp();
}
@Test
public void controllerRelease_keepsControllerBoundUntilCommandsHandled() throws Exception {
TestServiceRegistry testServiceRegistry = TestServiceRegistry.getInstance();
ConditionVariable mediaItemsAdded = new ConditionVariable();
AtomicBoolean controllerBoundWhenMediaItemsAdded = new AtomicBoolean(false);
testServiceRegistry.setOnGetSessionHandler(
controllerInfo -> {
// Save bound state at the point where media items are added and listeners are informed.
MockMediaSessionService service =
(MockMediaSessionService) testServiceRegistry.getServiceInstance();
Player player = new ExoPlayer.Builder(service).build();
player.addListener(
new Player.Listener() {
@Override
public void onEvents(Player player, Player.Events events) {
if (events.contains(Player.EVENT_TIMELINE_CHANGED)
&& !player.getCurrentTimeline().isEmpty()) {
controllerBoundWhenMediaItemsAdded.set(service.hasBoundController());
mediaItemsAdded.open();
}
}
});
// Add short delay for resolving media items.
return new MediaSession.Builder(service, player)
.setCallback(
new MediaSession.Callback() {
@Override
public ListenableFuture<List<MediaItem>> onAddMediaItems(
MediaSession mediaSession,
ControllerInfo controller,
List<MediaItem> mediaItems) {
SettableFuture<List<MediaItem>> future = SettableFuture.create();
MediaItem playableItem =
mediaItems.get(0).buildUpon().setUri("https://test.test").build();
new Handler(Looper.myLooper())
.postDelayed(
() -> future.set(ImmutableList.of(playableItem)),
/* delayMillis= */ 500);
return future;
}
})
.build();
});
RemoteMediaController controller =
controllerTestRule.createRemoteController(
token, /* waitForConnection= */ true, /* connectionHints= */ null);
MockMediaSessionService service =
(MockMediaSessionService) testServiceRegistry.getServiceInstance();
// Add items and release controller immediately.
controller.addMediaItem(new MediaItem.Builder().setMediaId("media_id").build());
controller.release();
// Assert controller is still bound when command is fully handled and unbound after that.
mediaItemsAdded.block(TIMEOUT_MS);
assertThat(controllerBoundWhenMediaItemsAdded.get()).isEqualTo(true);
service.blockUntilAllControllersUnbind(TIMEOUT_MS);
}
/**
* Tests whether {@link MediaSessionService#onGetSession(ControllerInfo)} is called when
* controller tries to connect, with the proper arguments.

View File

@ -17,11 +17,17 @@ package androidx.media3.session;
import static androidx.media3.test.session.common.CommonConstants.SUPPORT_APP_PACKAGE_NAME;
import android.content.Intent;
import android.os.HandlerThread;
import android.os.IBinder;
import android.text.TextUtils;
import androidx.annotation.Nullable;
import androidx.media3.common.util.ConditionVariable;
import androidx.media3.common.util.UnstableApi;
import androidx.media3.common.util.Util;
import androidx.media3.session.MediaSession.ControllerInfo;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicInteger;
/** A mock MediaSessionService */
@UnstableApi
@ -29,8 +35,37 @@ public class MockMediaSessionService extends MediaSessionService {
/** ID of the session that this service will create. */
public static final String ID = "TestSession";
private final AtomicInteger boundControllerCount;
public MediaSession session;
private HandlerThread handlerThread;
private ConditionVariable allControllersUnbound;
public MockMediaSessionService() {
boundControllerCount = new AtomicInteger(/* initialValue= */ 0);
allControllersUnbound = new ConditionVariable();
allControllersUnbound.open();
}
/** Returns whether at least one controller is bound to this service. */
public boolean hasBoundController() {
return !allControllersUnbound.isOpen();
}
/**
* Blocks until all bound controllers unbind.
*
* @param timeoutMs The block timeout in milliseconds.
* @throws TimeoutException If the block timed out.
* @throws InterruptedException If the block was interrupted.
*/
public void blockUntilAllControllersUnbind(long timeoutMs)
throws TimeoutException, InterruptedException {
if (!allControllersUnbound.block(timeoutMs)) {
throw new TimeoutException();
}
}
@Override
public void onCreate() {
@ -40,6 +75,21 @@ public class MockMediaSessionService extends MediaSessionService {
handlerThread.start();
}
@Override
public IBinder onBind(@Nullable Intent intent) {
boundControllerCount.incrementAndGet();
allControllersUnbound.close();
return super.onBind(intent);
}
@Override
public boolean onUnbind(Intent intent) {
if (boundControllerCount.decrementAndGet() == 0) {
allControllersUnbound.open();
}
return super.onUnbind(intent);
}
@Override
public void onDestroy() {
super.onDestroy();