Skip to content

Commit 74872d1

Browse files
toniheicopybara-github
authored andcommitted
Try to officially disconnect controller in more cases
In cases where we intentionally ignore a connection request, we don't currently attempt to call onDisconnected on the controller in all cases. This means such controllers may not be aware they are ignored. Even if some cases only happen with malformed or malicious connection attempts, it's still useful to try and disconnect the connection officially. PiperOrigin-RevId: 785814501
1 parent b82be5b commit 74872d1

File tree

5 files changed

+66
-73
lines changed

5 files changed

+66
-73
lines changed

libraries/session/src/main/java/androidx/media3/session/MediaSession.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2264,7 +2264,7 @@ default void onPeriodicSessionPositionInfoChanged(
22642264

22652265
// Mostly matched with MediaController.ControllerCallback
22662266

2267-
default void onDisconnected(int seq) throws RemoteException {}
2267+
default void onDisconnected(int seq) {}
22682268

22692269
default void setCustomLayout(int seq, List<CommandButton> layout) throws RemoteException {}
22702270

libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,11 +1012,7 @@ private ControllerInfo tryGetController(RemoteUserInfo remoteUserInfo) {
10121012
/* maxCommandsForMediaItems= */ 0);
10131013
MediaSession.ConnectionResult connectionResult = sessionImpl.onConnectOnHandler(controller);
10141014
if (!connectionResult.isAccepted) {
1015-
try {
1016-
controllerCb.onDisconnected(/* seq= */ 0);
1017-
} catch (RemoteException e) {
1018-
// Controller may have died prematurely.
1019-
}
1015+
controllerCb.onDisconnected(/* seq= */ 0);
10201016
return null;
10211017
}
10221018
connectedControllersManager.addController(
@@ -1306,7 +1302,7 @@ public void onAvailableCommandsChangedFromPlayer(int seq, Player.Commands availa
13061302
}
13071303

13081304
@Override
1309-
public void onDisconnected(int seq) throws RemoteException {
1305+
public void onDisconnected(int seq) {
13101306
// Calling MediaSessionCompat#release() is already done in release().
13111307
}
13121308

@@ -1728,11 +1724,7 @@ public ConnectionTimeoutHandler(
17281724
public void handleMessage(Message msg) {
17291725
ControllerInfo controller = (ControllerInfo) msg.obj;
17301726
if (connectedControllersManager.isConnected(controller)) {
1731-
try {
1732-
checkStateNotNull(controller.getControllerCb()).onDisconnected(/* seq= */ 0);
1733-
} catch (RemoteException e) {
1734-
// Controller may have died prematurely.
1735-
}
1727+
checkStateNotNull(controller.getControllerCb()).onDisconnected(/* seq= */ 0);
17361728
connectedControllersManager.removeController(controller);
17371729
}
17381730
}

libraries/session/src/main/java/androidx/media3/session/MediaSessionService.java

Lines changed: 16 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import android.os.Handler;
3535
import android.os.IBinder;
3636
import android.os.Looper;
37-
import android.os.RemoteException;
3837
import androidx.annotation.CallSuper;
3938
import androidx.annotation.GuardedBy;
4039
import androidx.annotation.IntDef;
@@ -836,26 +835,21 @@ public void connect(
836835
@Nullable IMediaController caller, @Nullable Bundle connectionRequestBundle) {
837836
if (caller == null || connectionRequestBundle == null) {
838837
// Malformed call from potentially malicious controller.
839-
// No need to notify that we're ignoring call.
838+
SessionUtil.disconnectIMediaController(caller);
840839
return;
841840
}
842841
ConnectionRequest request;
843842
try {
844843
request = ConnectionRequest.fromBundle(connectionRequestBundle);
845844
} catch (RuntimeException e) {
846845
// Malformed call from potentially malicious controller.
847-
// No need to notify that we're ignoring call.
848846
Log.w(TAG, "Ignoring malformed Bundle for ConnectionRequest", e);
847+
SessionUtil.disconnectIMediaController(caller);
849848
return;
850849
}
851850
@Nullable MediaSessionService mediaSessionService = serviceReference.get();
852851
if (mediaSessionService == null) {
853-
try {
854-
caller.onDisconnected(/* seq= */ 0);
855-
} catch (RemoteException e) {
856-
// Controller may be died prematurely.
857-
// Not an issue because we'll ignore it anyway.
858-
}
852+
SessionUtil.disconnectIMediaController(caller);
859853
return;
860854
}
861855
int callingPid = Binder.getCallingPid();
@@ -872,7 +866,7 @@ public void connect(
872866
handler.post(
873867
() -> {
874868
pendingControllers.remove(caller);
875-
boolean shouldNotifyDisconnected = true;
869+
boolean connected = false;
876870
try {
877871
@Nullable MediaSessionService service = serviceReference.get();
878872
if (service == null) {
@@ -889,30 +883,19 @@ public void connect(
889883
request.connectionHints,
890884
request.maxCommandsForMediaItems);
891885

892-
@Nullable MediaSession session;
893-
try {
894-
session = service.onGetSession(controllerInfo);
895-
if (session == null) {
896-
return;
897-
}
898-
899-
service.addSession(session);
900-
shouldNotifyDisconnected = false;
901-
902-
session.handleControllerConnectionFromService(caller, controllerInfo);
903-
} catch (Exception e) {
904-
// Don't propagate exception in service to the controller.
905-
Log.w(TAG, "Failed to add a session to session service", e);
886+
@Nullable MediaSession session = service.onGetSession(controllerInfo);
887+
if (session == null) {
888+
return;
906889
}
890+
service.addSession(session);
891+
session.handleControllerConnectionFromService(caller, controllerInfo);
892+
connected = true;
893+
} catch (Exception e) {
894+
// Don't propagate exception in service to the controller.
895+
Log.w(TAG, "Failed to add a session to session service", e);
907896
} finally {
908-
// Trick to call onDisconnected() in one place.
909-
if (shouldNotifyDisconnected) {
910-
try {
911-
caller.onDisconnected(/* seq= */ 0);
912-
} catch (RemoteException e) {
913-
// Controller may be died prematurely.
914-
// Not an issue because we'll ignore it anyway.
915-
}
897+
if (!connected) {
898+
SessionUtil.disconnectIMediaController(caller);
916899
}
917900
}
918901
});
@@ -925,11 +908,7 @@ public void release() {
925908
serviceReference.clear();
926909
handler.removeCallbacksAndMessages(null);
927910
for (IMediaController controller : pendingControllers) {
928-
try {
929-
controller.onDisconnected(/* seq= */ 0);
930-
} catch (RemoteException e) {
931-
// Ignore. We're releasing.
932-
}
911+
SessionUtil.disconnectIMediaController(controller);
933912
}
934913
pendingControllers.clear();
935914
}

libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -467,16 +467,12 @@ private int maybeCorrectMediaItemIndex(
467467
@SuppressWarnings("UngroupedOverloads") // Overload belongs to AIDL interface that is separated.
468468
public void connect(@Nullable IMediaController caller, @Nullable ControllerInfo controllerInfo) {
469469
if (caller == null || controllerInfo == null) {
470+
SessionUtil.disconnectIMediaController(caller);
470471
return;
471472
}
472473
@Nullable MediaSessionImpl sessionImpl = this.sessionImpl.get();
473474
if (sessionImpl == null || sessionImpl.isReleased()) {
474-
try {
475-
caller.onDisconnected(/* seq= */ 0);
476-
} catch (RemoteException e) {
477-
// Controller may be died prematurely.
478-
// Not an issue because we'll ignore it anyway.
479-
}
475+
SessionUtil.disconnectIMediaController(caller);
480476
return;
481477
}
482478
pendingControllers.add(controllerInfo);
@@ -596,12 +592,7 @@ public void connect(@Nullable IMediaController caller, @Nullable ControllerInfo
596592
}
597593
} finally {
598594
if (!connected) {
599-
try {
600-
caller.onDisconnected(/* seq= */ 0);
601-
} catch (RemoteException e) {
602-
// Controller may be died prematurely.
603-
// Not an issue because we'll ignore it anyway.
604-
}
595+
SessionUtil.disconnectIMediaController(caller);
605596
}
606597
}
607598
});
@@ -613,21 +604,13 @@ public void release() {
613604
connectedControllersManager.removeController(controller);
614605
ControllerCb cb = controller.getControllerCb();
615606
if (cb != null) {
616-
try {
617-
cb.onDisconnected(/* seq= */ 0);
618-
} catch (RemoteException e) {
619-
// Ignore. We're releasing.
620-
}
607+
cb.onDisconnected(/* seq= */ 0);
621608
}
622609
}
623610
for (ControllerInfo controller : pendingControllers) {
624611
ControllerCb cb = controller.getControllerCb();
625612
if (cb != null) {
626-
try {
627-
cb.onDisconnected(/* seq= */ 0);
628-
} catch (RemoteException e) {
629-
// Ignore. We're releasing.
630-
}
613+
cb.onDisconnected(/* seq= */ 0);
631614
}
632615
}
633616
pendingControllers.clear();
@@ -2226,8 +2209,8 @@ public void onSearchResultChanged(
22262209
}
22272210

22282211
@Override
2229-
public void onDisconnected(int sequenceNumber) throws RemoteException {
2230-
iController.onDisconnected(sequenceNumber);
2212+
public void onDisconnected(int sequenceNumber) {
2213+
SessionUtil.disconnectIMediaController(iController);
22312214
}
22322215

22332216
@Override
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright 2025 The Android Open Source Project
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package androidx.media3.session;
17+
18+
import android.os.RemoteException;
19+
import androidx.annotation.Nullable;
20+
21+
/** Static utilities for the session module. */
22+
/* package */ class SessionUtil {
23+
24+
private SessionUtil() {}
25+
26+
/**
27+
* Disconnect a {@link IMediaController}, ignoring remote exceptions in case the controller is
28+
* broken or already died.
29+
*/
30+
public static void disconnectIMediaController(@Nullable IMediaController controller) {
31+
try {
32+
if (controller != null) {
33+
controller.onDisconnected(/* seq= */ 0);
34+
}
35+
} catch (RemoteException e) {
36+
// Intentionally ignored. Controller may have died already or is malformed.
37+
}
38+
}
39+
}

0 commit comments

Comments
 (0)