Skip to content

Commit 92e1f62

Browse files
nift4nift4
committed
session: fix a deadlock due to onPlayRequested()
Issue: #1197
1 parent 1ca8dff commit 92e1f62

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

1103-
/* package */ boolean onPlayRequested() {
1103+
/* package */ ListenableFuture<Boolean> onPlayRequested() {
11041104
if (Looper.myLooper() != Looper.getMainLooper()) {
11051105
SettableFuture<Boolean> playRequested = SettableFuture.create();
1106-
mainHandler.post(() -> playRequested.set(onPlayRequested()));
1107-
try {
1108-
return playRequested.get();
1109-
} catch (InterruptedException | ExecutionException e) {
1110-
throw new IllegalStateException(e);
1111-
}
1106+
mainHandler.post(
1107+
() -> {
1108+
try {
1109+
playRequested.set(onPlayRequested().get());
1110+
} catch (ExecutionException | InterruptedException e) {
1111+
playRequested.setException(new IllegalStateException(e));
1112+
}
1113+
});
1114+
return playRequested;
11121115
}
11131116
if (this.mediaSessionListener != null) {
1114-
return this.mediaSessionListener.onPlayRequested(instance);
1117+
return Futures.immediateFuture(this.mediaSessionListener.onPlayRequested(instance));
11151118
}
1116-
return true;
1119+
return Futures.immediateFuture(true);
11171120
}
11181121

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

12071229
/* 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
@@ -749,15 +749,10 @@ public void playForControllerInfo(ControllerInfo controller, int sequenceNumber)
749749
controller,
750750
sequenceNumber,
751751
COMMAND_PLAY_PAUSE,
752-
sendSessionResultSuccess(
753-
player -> {
754-
@Nullable MediaSessionImpl impl = sessionImpl.get();
755-
if (impl == null || impl.isReleased()) {
756-
return;
757-
}
758-
impl.handleMediaControllerPlayRequest(
759-
controller, /* callOnPlayerInteractionFinished= */ false);
760-
}));
752+
sendSessionResultWhenReady(
753+
(session, theController, sequenceId) ->
754+
session.handleMediaControllerPlayRequest(
755+
theController, /* callOnPlayerInteractionFinished= */ false)));
761756
}
762757

763758
@Override

0 commit comments

Comments
 (0)