Skip to content

Commit d70bfc0

Browse files
nift4nift4
committed
session: fix a deadlock due to onPlayRequested()
Issue: #1197
1 parent 2f242a0 commit d70bfc0

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
@@ -1099,20 +1099,23 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
10991099
});
11001100
}
11011101

1102-
/* package */ boolean onPlayRequested() {
1102+
/* package */ ListenableFuture<Boolean> onPlayRequested() {
11031103
if (Looper.myLooper() != Looper.getMainLooper()) {
11041104
SettableFuture<Boolean> playRequested = SettableFuture.create();
1105-
mainHandler.post(() -> playRequested.set(onPlayRequested()));
1106-
try {
1107-
return playRequested.get();
1108-
} catch (InterruptedException | ExecutionException e) {
1109-
throw new IllegalStateException(e);
1110-
}
1105+
mainHandler.post(
1106+
() -> {
1107+
try {
1108+
playRequested.set(onPlayRequested().get());
1109+
} catch (ExecutionException | InterruptedException e) {
1110+
playRequested.setException(new IllegalStateException(e));
1111+
}
1112+
});
1113+
return playRequested;
11111114
}
11121115
if (this.mediaSessionListener != null) {
1113-
return this.mediaSessionListener.onPlayRequested(instance);
1116+
return Futures.immediateFuture(this.mediaSessionListener.onPlayRequested(instance));
11141117
}
1115-
return true;
1118+
return Futures.immediateFuture(true);
11161119
}
11171120

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

12061228
/* 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
@@ -607,9 +607,27 @@ public void onPrepareFromUri(@Nullable Uri mediaUri, @Nullable Bundle extras) {
607607
public void onPlay() {
608608
dispatchSessionTaskWithPlayerCommand(
609609
COMMAND_PLAY_PAUSE,
610-
controller ->
611-
sessionImpl.handleMediaControllerPlayRequest(
612-
controller, /* callOnPlayerInteractionFinished= */ true),
610+
controller -> {
611+
ListenableFuture<SessionResult> resultFuture =
612+
sessionImpl.handleMediaControllerPlayRequest(
613+
controller, /* callOnPlayerInteractionFinished= */ true);
614+
Futures.addCallback(
615+
resultFuture,
616+
new FutureCallback<SessionResult>() {
617+
@Override
618+
public void onSuccess(SessionResult result) {
619+
if (result.resultCode != RESULT_SUCCESS) {
620+
Log.w(TAG, "onPlay() failed: " + result + " (from: " + controller + ")");
621+
}
622+
}
623+
624+
@Override
625+
public void onFailure(Throwable t) {
626+
Log.e(TAG, "Unexpected exception in onPlay() of " + controller, t);
627+
}
628+
},
629+
MoreExecutors.directExecutor());
630+
},
613631
sessionCompat.getCurrentControllerInfo(),
614632
/* callOnPlayerInteractionFinished= */ false);
615633
}

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -761,15 +761,10 @@ public void playForControllerInfo(ControllerInfo controller, int sequenceNumber)
761761
controller,
762762
sequenceNumber,
763763
COMMAND_PLAY_PAUSE,
764-
sendSessionResultSuccess(
765-
player -> {
766-
@Nullable MediaSessionImpl impl = sessionImpl.get();
767-
if (impl == null || impl.isReleased()) {
768-
return;
769-
}
770-
impl.handleMediaControllerPlayRequest(
771-
controller, /* callOnPlayerInteractionFinished= */ false);
772-
}));
764+
sendSessionResultWhenReady(
765+
(session, theController, sequenceId) ->
766+
session.handleMediaControllerPlayRequest(
767+
theController, /* callOnPlayerInteractionFinished= */ false)));
773768
}
774769

775770
@Override

0 commit comments

Comments
 (0)