loliball/media3
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix missing notification of session release

Browse Source 
A session may not notify its release back to the controller
when the controller is connecting while the session is releasing.
Here are issues in detail:
  - MediaSession doesn't respond to controller's connection
    request when it's released.
  - MediaSession discards incoming connection requests when it's
    released. The requests are only kept in the application
    looper, but the looper is cleared when the session is
    released.

This CL fixes the above issues, and fixes some flaky tests.

PiperOrigin-RevId: 419756158
This commit is contained in:
jaewan 2022-01-05 07:27:57 +00:00 committed by tonihei
parent f7e88d7c82
commit bb81c4969d
4 changed files with 157 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java
View File

@ -313,7 +313,7 @@ import org.checkerframework.checker.initialization.qual.Initialized;
if (broadcastReceiver != null) {
context.unregisterReceiver(broadcastReceiver);
}
dispatchRemoteControllerTaskWithoutReturn(ControllerCb::onDisconnected);
sessionStub.release();
}
public PlayerWrapper getPlayerWrapper() {
@ -604,7 +604,10 @@ import org.checkerframework.checker.initialization.qual.Initialized;
// 0 is OK for legacy controllers, because they didn't have sequence numbers.
seq = 0;
}
task.run(checkStateNotNull(controller.getControllerCb()), seq);
ControllerCb cb = controller.getControllerCb();
if (cb != null) {
task.run(cb, seq);
}
} catch (DeadObjectException e) {
onDeadObjectException(controller, e);
} catch (RemoteException e) {
@ -637,7 +640,10 @@ import org.checkerframework.checker.initialization.qual.Initialized;
// Tell that operation is successful, although we don't know the actual result.
future = Futures.immediateFuture(new SessionResult(SessionResult.RESULT_SUCCESS));
}
task.run(checkStateNotNull(controller.getControllerCb()), seq);
ControllerCb cb = controller.getControllerCb();
if (cb != null) {
task.run(cb, seq);
}
return future;
} catch (DeadObjectException e) {
onDeadObjectException(controller, e);

121
libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java
View File

@ -81,9 +81,12 @@ import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.MoreExecutors;
import java.lang.ref.WeakReference;
import java.util.Collections;
import java.util.Deque;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CancellationException;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
/**
@ -96,19 +99,17 @@ import java.util.concurrent.ExecutionException;
private static final String TAG = "MediaSessionStub";
private final Object lock;
private final WeakReference<MediaSessionImpl> sessionImpl;
private final MediaSessionManager sessionManager;
private final ConnectedControllersManager<IBinder> connectedControllersManager;
private final Set<ControllerInfo> pendingControllers;
public MediaSessionStub(MediaSessionImpl sessionImpl) {
// Initialize default values.
lock = new Object();
// Initialize members with params.
this.sessionImpl = new WeakReference<>(sessionImpl);
sessionManager = MediaSessionManager.getSessionManager(sessionImpl.getContext());
connectedControllersManager = new ConnectedControllersManager<>(sessionImpl);
pendingControllers = Collections.newSetFromMap(new ConcurrentHashMap<>());
}
public ConnectedControllersManager<IBinder> getConnectedControllersManager() {
@ -331,50 +332,58 @@ import java.util.concurrent.ExecutionException;
connectionHints);
@Nullable MediaSessionImpl sessionImpl = this.sessionImpl.get();
if (sessionImpl == null || sessionImpl.isReleased()) {
try {
caller.onDisconnected(/* seq= */ 0);
} catch (RemoteException e) {
// Controller may be died prematurely.
// Not an issue because we'll ignore it anyway.
}
return;
}
pendingControllers.add(controllerInfo);
postOrRun(
sessionImpl.getApplicationHandler(),
() -> {
if (sessionImpl.isReleased()) {
return;
}
IBinder callbackBinder =
checkStateNotNull((Controller2Cb) controllerInfo.getControllerCb())
.getCallbackBinder();
MediaSession.ConnectionResult connectionResult =
sessionImpl.onConnectOnHandler(controllerInfo);
// Don't reject connection for the request from trusted app.
// Otherwise server will fail to retrieve session's information to dispatch
// media keys to.
boolean accept = connectionResult.isAccepted || controllerInfo.isTrusted();
if (accept) {
boolean connected = false;
try {
pendingControllers.remove(controllerInfo);
if (sessionImpl.isReleased()) {
return;
}
IBinder callbackBinder =
checkStateNotNull((Controller2Cb) controllerInfo.getControllerCb())
.getCallbackBinder();
MediaSession.ConnectionResult connectionResult =
sessionImpl.onConnectOnHandler(controllerInfo);
// Don't reject connection for the request from trusted app.
// Otherwise server will fail to retrieve session's information to dispatch
// media keys to.
if (!connectionResult.isAccepted && !controllerInfo.isTrusted()) {
return;
}
if (!connectionResult.isAccepted) {
// For trusted apps, send non-null allowed commands to keep
// connection.
// For the accepted controller, send non-null allowed commands to keep connection.
connectionResult =
MediaSession.ConnectionResult.accept(
SessionCommands.EMPTY, Player.Commands.EMPTY);
}
SequencedFutureManager sequencedFutureManager;
synchronized (lock) {
if (connectedControllersManager.isConnected(controllerInfo)) {
Log.w(
TAG,
"Controller "
+ controllerInfo
+ " has sent connection"
+ " request multiple times");
}
connectedControllersManager.addController(
callbackBinder,
controllerInfo,
connectionResult.availableSessionCommands,
connectionResult.availablePlayerCommands);
sequencedFutureManager =
checkStateNotNull(
connectedControllersManager.getSequencedFutureManager(controllerInfo));
if (connectedControllersManager.isConnected(controllerInfo)) {
Log.w(
TAG,
"Controller "
+ controllerInfo
+ " has sent connection"
+ " request multiple times");
}
connectedControllersManager.addController(
callbackBinder,
controllerInfo,
connectionResult.availableSessionCommands,
connectionResult.availablePlayerCommands);
sequencedFutureManager =
checkStateNotNull(
connectedControllersManager.getSequencedFutureManager(controllerInfo));
// If connection is accepted, notify the current state to the controller.
// It's needed because we cannot call synchronous calls between
// session/controller.
@ -399,22 +408,48 @@ import java.util.concurrent.ExecutionException;
try {
caller.onConnected(
sequencedFutureManager.obtainNextSequenceNumber(), state.toBundle());
connected = true;
} catch (RemoteException e) {
// Controller may be died prematurely.
}
sessionImpl.onPostConnectOnHandler(controllerInfo);
} else {
try {
caller.onDisconnected(0);
} catch (RemoteException e) {
// Controller may be died prematurely.
// Not an issue because we'll ignore it anyway.
} finally {
if (!connected) {
try {
caller.onDisconnected(/* seq= */ 0);
} catch (RemoteException e) {
// Controller may be died prematurely.
// Not an issue because we'll ignore it anyway.
}
}
}
});
}
public void release() {
List<ControllerInfo> controllers = connectedControllersManager.getConnectedControllers();
for (ControllerInfo controller : controllers) {
ControllerCb cb = controller.getControllerCb();
if (cb != null) {
try {
cb.onDisconnected(/* seq= */ 0);
} catch (RemoteException e) {
// Ignore. We're releasing.
}
}
}
for (ControllerInfo controller : pendingControllers) {
ControllerCb cb = controller.getControllerCb();
if (cb != null) {
try {
cb.onDisconnected(/* seq= */ 0);
} catch (RemoteException e) {
// Ignore. We're releasing.
}
}
}
}
//////////////////////////////////////////////////////////////////////////////////////////////
// AIDL methods for session overrides
//////////////////////////////////////////////////////////////////////////////////////////////

16
libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java
View File

@ -83,6 +83,7 @@ import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Rule;
@ -172,6 +173,21 @@ public class MediaControllerListenerTest {
assertThat(controller).isNotNull();
}
@Test
public void connection_toReleasedSession() throws Exception {
CountDownLatch latch = new CountDownLatch(1);
SessionToken token = remoteSession.getToken();
remoteSession.release();
ListenableFuture<MediaController> controllerFuture =
new MediaController.Builder(context, token)
.setApplicationLooper(threadTestRule.getHandler().getLooper())
.buildAsync();
controllerFuture.addListener(() -> latch.countDown(), threadTestRule.getHandler()::post);
latch.await(TIMEOUT_MS, MILLISECONDS);
Assert.assertThrows(
ExecutionException.class, () -> controllerFuture.get(/* timeout= */ 0, MILLISECONDS));
}
@Test
public void connection_sessionReleased() throws Exception {
CountDownLatch latch = new CountDownLatch(1);

54
libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaSessionAndControllerTest.java
View File

@ -22,14 +22,21 @@ import static com.google.common.truth.Truth.assertThat;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import android.content.Context;
import android.os.Handler;
import android.os.Looper;
import androidx.media3.common.Player;
import androidx.media3.common.util.Util;
import androidx.media3.test.session.common.HandlerThreadTestRule;
import androidx.media3.test.session.common.MainLooperTestRule;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.filters.LargeTest;
import com.google.common.util.concurrent.ListenableFuture;
import java.util.concurrent.CancellationException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;
import org.junit.Before;
import org.junit.ClassRule;
@ -100,6 +107,53 @@ public class MediaSessionAndControllerTest {
});
}
/** Tests to ensure that disconnection is notified. */
@Test
public void connecting_whileSessionIsReleasing_notified() throws Exception {
MockPlayer player =
new MockPlayer.Builder()
.setApplicationLooper(threadTestRule.getHandler().getLooper())
.build();
Handler mainHandler = new Handler(Looper.getMainLooper());
Executor mainExecutor = (runnable) -> Util.postOrRun(mainHandler, runnable);
for (int i = 0; i < 100; i++) {
int idx = i;
MediaSession session =
sessionTestRule.ensureReleaseAfterTest(
new MediaSession.Builder(context, player).setId(TAG).build());
CountDownLatch latch = new CountDownLatch(1);
mainHandler.post(
() -> {
ListenableFuture<MediaController> controllerFuture =
new MediaController.Builder(context, session.getToken())
.setListener(
new MediaController.Listener() {
@Override
public void onDisconnected(MediaController controller) {
latch.countDown();
}
})
.buildAsync();
controllerFuture.addListener(
() -> {
try {
MediaController controller =
controllerFuture.get(/* timeout=* */ 0, MILLISECONDS);
assertThat(controller).isNotNull();
} catch (ExecutionException
| InterruptedException
| CancellationException
| TimeoutException e) {
latch.countDown();
}
},
mainExecutor);
});
threadTestRule.getHandler().postAndSync(session::release);
assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
}
}
@Test
public void connect_withTheSameLooper_connectsInTheSameLooperIteration() throws Exception {
// This may hang if controller cannot be connected immediately in a looper iteration.