Skip to content

Commit 4c339e4

Browse files
committed
session: fix a deadlock due to onPlayRequested()
Issue: #1197
1 parent d1dcb26 commit 4c339e4

File tree

3 files changed

+131
-96
lines changed

3 files changed

+131
-96
lines changed

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

Lines changed: 106 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,20 +1113,23 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
11131113
});
11141114
}
11151115

1116-
/* package */ boolean onPlayRequested() {
1116+
/* package */ ListenableFuture<Boolean> onPlayRequested() {
11171117
if (Looper.myLooper() != Looper.getMainLooper()) {
11181118
SettableFuture<Boolean> playRequested = SettableFuture.create();
1119-
mainHandler.post(() -> playRequested.set(onPlayRequested()));
1120-
try {
1121-
return playRequested.get();
1122-
} catch (InterruptedException | ExecutionException e) {
1123-
throw new IllegalStateException(e);
1124-
}
1119+
mainHandler.post(
1120+
() -> {
1121+
try {
1122+
playRequested.set(onPlayRequested().get());
1123+
} catch (ExecutionException | InterruptedException e) {
1124+
playRequested.setException(new IllegalStateException(e));
1125+
}
1126+
});
1127+
return playRequested;
11251128
}
11261129
if (this.mediaSessionListener != null) {
1127-
return this.mediaSessionListener.onPlayRequested(instance);
1130+
return Futures.immediateFuture(this.mediaSessionListener.onPlayRequested(instance));
11281131
}
1129-
return true;
1132+
return Futures.immediateFuture(true);
11301133
}
11311134

11321135
/**
@@ -1137,84 +1140,103 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
11371140
*
11381141
* @param controller The controller requesting to play.
11391142
*/
1140-
/* package */ void handleMediaControllerPlayRequest(
1143+
/* package */ ListenableFuture<SessionResult> handleMediaControllerPlayRequest(
11411144
ControllerInfo controller, boolean callOnPlayerInteractionFinished) {
1142-
if (!onPlayRequested()) {
1143-
// Request denied, e.g. due to missing foreground service abilities.
1144-
return;
1145-
}
1146-
boolean hasCurrentMediaItem =
1147-
playerWrapper.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)
1148-
&& playerWrapper.getCurrentMediaItem() != null;
1149-
boolean canAddMediaItems =
1150-
playerWrapper.isCommandAvailable(COMMAND_SET_MEDIA_ITEM)
1151-
|| playerWrapper.isCommandAvailable(COMMAND_CHANGE_MEDIA_ITEMS);
1152-
ControllerInfo controllerForRequest = resolveControllerInfoForCallback(controller);
1153-
Player.Commands playCommand =
1154-
new Player.Commands.Builder().add(Player.COMMAND_PLAY_PAUSE).build();
1155-
if (hasCurrentMediaItem || !canAddMediaItems) {
1156-
// No playback resumption needed or possible.
1157-
if (!hasCurrentMediaItem) {
1158-
Log.w(
1159-
TAG,
1160-
"Play requested without current MediaItem, but playback resumption prevented by"
1161-
+ " missing available commands");
1162-
}
1163-
Util.handlePlayButtonAction(playerWrapper);
1164-
if (callOnPlayerInteractionFinished) {
1165-
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1166-
}
1167-
} else {
1168-
@Nullable
1169-
ListenableFuture<MediaItemsWithStartPosition> future =
1170-
checkNotNull(
1171-
callback.onPlaybackResumption(
1172-
instance, controllerForRequest, /* isForPlayback= */ true),
1173-
"Callback.onPlaybackResumption must return a non-null future");
1174-
Futures.addCallback(
1175-
future,
1176-
new FutureCallback<MediaItemsWithStartPosition>() {
1177-
@Override
1178-
public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) {
1179-
callWithControllerForCurrentRequestSet(
1180-
controllerForRequest,
1181-
() -> {
1182-
MediaUtils.setMediaItemsWithStartIndexAndPosition(
1183-
playerWrapper, mediaItemsWithStartPosition);
1184-
Util.handlePlayButtonAction(playerWrapper);
1185-
if (callOnPlayerInteractionFinished) {
1186-
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1187-
}
1188-
})
1189-
.run();
1145+
SettableFuture<SessionResult> sessionFuture = SettableFuture.create();
1146+
ListenableFuture<Boolean> playRequestedFuture = onPlayRequested();
1147+
playRequestedFuture.addListener(
1148+
() -> {
1149+
boolean playRequested;
1150+
try {
1151+
playRequested = playRequestedFuture.get();
1152+
} catch (ExecutionException | InterruptedException e) {
1153+
sessionFuture.setException(new IllegalStateException(e));
1154+
return;
1155+
}
1156+
if (!playRequested) {
1157+
// Request denied, e.g. due to missing foreground service abilities.
1158+
sessionFuture.set(new SessionResult(SessionResult.RESULT_ERROR_UNKNOWN));
1159+
return;
1160+
}
1161+
boolean hasCurrentMediaItem =
1162+
playerWrapper.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)
1163+
&& playerWrapper.getCurrentMediaItem() != null;
1164+
boolean canAddMediaItems =
1165+
playerWrapper.isCommandAvailable(COMMAND_SET_MEDIA_ITEM)
1166+
|| playerWrapper.isCommandAvailable(COMMAND_CHANGE_MEDIA_ITEMS);
1167+
ControllerInfo controllerForRequest = resolveControllerInfoForCallback(controller);
1168+
Player.Commands playCommand =
1169+
new Player.Commands.Builder().add(Player.COMMAND_PLAY_PAUSE).build();
1170+
if (hasCurrentMediaItem || !canAddMediaItems) {
1171+
// No playback resumption needed or possible.
1172+
if (!hasCurrentMediaItem) {
1173+
Log.w(
1174+
TAG,
1175+
"Play requested without current MediaItem, but playback resumption prevented by"
1176+
+ " missing available commands");
11901177
}
1191-
1192-
@Override
1193-
public void onFailure(Throwable t) {
1194-
if (t instanceof UnsupportedOperationException) {
1195-
Log.w(
1196-
TAG,
1197-
"UnsupportedOperationException: Make sure to implement"
1198-
+ " MediaSession.Callback.onPlaybackResumption() if you add a"
1199-
+ " media button receiver to your manifest or if you implement the recent"
1200-
+ " media item contract with your MediaLibraryService.",
1201-
t);
1202-
} else {
1203-
Log.e(
1204-
TAG,
1205-
"Failure calling MediaSession.Callback.onPlaybackResumption(): "
1206-
+ t.getMessage(),
1207-
t);
1208-
}
1209-
// Play as requested even if playback resumption fails.
1210-
Util.handlePlayButtonAction(playerWrapper);
1211-
if (callOnPlayerInteractionFinished) {
1212-
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1213-
}
1178+
Util.handlePlayButtonAction(playerWrapper);
1179+
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
1180+
if (callOnPlayerInteractionFinished) {
1181+
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
12141182
}
1215-
},
1216-
this::postOrRunOnApplicationHandler);
1217-
}
1183+
} else {
1184+
@Nullable
1185+
ListenableFuture<MediaItemsWithStartPosition> future =
1186+
checkNotNull(
1187+
callback.onPlaybackResumption(
1188+
instance, controllerForRequest, /* isForPlayback= */ true),
1189+
"Callback.onPlaybackResumption must return a non-null future");
1190+
Futures.addCallback(
1191+
future,
1192+
new FutureCallback<MediaItemsWithStartPosition>() {
1193+
@Override
1194+
public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) {
1195+
callWithControllerForCurrentRequestSet(
1196+
controllerForRequest,
1197+
() -> {
1198+
MediaUtils.setMediaItemsWithStartIndexAndPosition(
1199+
playerWrapper, mediaItemsWithStartPosition);
1200+
Util.handlePlayButtonAction(playerWrapper);
1201+
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
1202+
if (callOnPlayerInteractionFinished) {
1203+
onPlayerInteractionFinishedOnHandler(
1204+
controllerForRequest, playCommand);
1205+
}
1206+
})
1207+
.run();
1208+
}
1209+
1210+
@Override
1211+
public void onFailure(Throwable t) {
1212+
if (t instanceof UnsupportedOperationException) {
1213+
Log.w(
1214+
TAG,
1215+
"UnsupportedOperationException: Make sure to implement"
1216+
+ " MediaSession.Callback.onPlaybackResumption() if you add a media"
1217+
+ " button receiver to your manifest or if you implement the recent"
1218+
+ " media item contract with your MediaLibraryService.",
1219+
t);
1220+
} else {
1221+
Log.e(
1222+
TAG,
1223+
"Failure calling MediaSession.Callback.onPlaybackResumption(): "
1224+
+ t.getMessage(),
1225+
t);
1226+
}
1227+
// Play as requested even if playback resumption fails.
1228+
Util.handlePlayButtonAction(playerWrapper);
1229+
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
1230+
if (callOnPlayerInteractionFinished) {
1231+
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1232+
}
1233+
}
1234+
},
1235+
this::postOrRunOnApplicationHandler);
1236+
}
1237+
},
1238+
this::postOrRunOnApplicationHandler);
1239+
return sessionFuture;
12181240
}
12191241

12201242
/* package */ void triggerPlayerInfoUpdate() {

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

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -849,9 +849,27 @@ public ConnectedControllersManager<RemoteUserInfo> getConnectedControllersManage
849849
private void dispatchSessionTaskWithPlayRequest() {
850850
dispatchSessionTaskWithPlayerCommand(
851851
COMMAND_PLAY_PAUSE,
852-
controller ->
853-
sessionImpl.handleMediaControllerPlayRequest(
854-
controller, /* callOnPlayerInteractionFinished= */ true),
852+
controller -> {
853+
ListenableFuture<SessionResult> resultFuture =
854+
sessionImpl.handleMediaControllerPlayRequest(
855+
controller, /* callOnPlayerInteractionFinished= */ true);
856+
Futures.addCallback(
857+
resultFuture,
858+
new FutureCallback<SessionResult>() {
859+
@Override
860+
public void onSuccess(SessionResult result) {
861+
if (result.resultCode != RESULT_SUCCESS) {
862+
Log.w(TAG, "onPlay() failed: " + result + " (from: " + controller + ")");
863+
}
864+
}
865+
866+
@Override
867+
public void onFailure(Throwable t) {
868+
Log.e(TAG, "Unexpected exception in onPlay() of " + controller, t);
869+
}
870+
},
871+
MoreExecutors.directExecutor());
872+
},
855873
sessionCompat.getCurrentControllerInfo(),
856874
/* callOnPlayerInteractionFinished= */ false);
857875
}

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -770,15 +770,10 @@ public void playForControllerInfo(ControllerInfo controller, int sequenceNumber)
770770
controller,
771771
sequenceNumber,
772772
COMMAND_PLAY_PAUSE,
773-
sendSessionResultSuccess(
774-
player -> {
775-
@Nullable MediaSessionImpl impl = sessionImpl.get();
776-
if (impl == null || impl.isReleased()) {
777-
return;
778-
}
779-
impl.handleMediaControllerPlayRequest(
780-
controller, /* callOnPlayerInteractionFinished= */ false);
781-
}));
773+
sendSessionResultWhenReady(
774+
(session, theController, sequenceId) ->
775+
session.handleMediaControllerPlayRequest(
776+
theController, /* callOnPlayerInteractionFinished= */ false)));
782777
}
783778

784779
@Override

0 commit comments

Comments
 (0)