Skip to content

Commit 03b8a7d

Browse files
nift4nift4
committed
session: fix a deadlock due to onPlayRequested()
Issue: #1197
1 parent e3018e5 commit 03b8a7d

File tree

3 files changed

+130
-95
lines changed

3 files changed

+130
-95
lines changed

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

Lines changed: 105 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,20 +1066,23 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
10661066
});
10671067
}
10681068

1069-
/* package */ boolean onPlayRequested() {
1069+
/* package */ ListenableFuture<Boolean> onPlayRequested() {
10701070
if (Looper.myLooper() != Looper.getMainLooper()) {
10711071
SettableFuture<Boolean> playRequested = SettableFuture.create();
1072-
mainHandler.post(() -> playRequested.set(onPlayRequested()));
1073-
try {
1074-
return playRequested.get();
1075-
} catch (InterruptedException | ExecutionException e) {
1076-
throw new IllegalStateException(e);
1077-
}
1072+
mainHandler.post(
1073+
() -> {
1074+
try {
1075+
playRequested.set(onPlayRequested().get());
1076+
} catch (ExecutionException | InterruptedException e) {
1077+
playRequested.setException(new IllegalStateException(e));
1078+
}
1079+
});
1080+
return playRequested;
10781081
}
10791082
if (this.mediaSessionListener != null) {
1080-
return this.mediaSessionListener.onPlayRequested(instance);
1083+
return Futures.immediateFuture(this.mediaSessionListener.onPlayRequested(instance));
10811084
}
1082-
return true;
1085+
return Futures.immediateFuture(true);
10831086
}
10841087

10851088
/**
@@ -1090,83 +1093,102 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
10901093
*
10911094
* @param controller The controller requesting to play.
10921095
*/
1093-
/* package */ void handleMediaControllerPlayRequest(
1096+
/* package */ ListenableFuture<SessionResult> handleMediaControllerPlayRequest(
10941097
ControllerInfo controller, boolean callOnPlayerInteractionFinished) {
1095-
if (!onPlayRequested()) {
1096-
// Request denied, e.g. due to missing foreground service abilities.
1097-
return;
1098-
}
1099-
boolean hasCurrentMediaItem =
1100-
playerWrapper.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)
1101-
&& playerWrapper.getCurrentMediaItem() != null;
1102-
boolean canAddMediaItems =
1103-
playerWrapper.isCommandAvailable(COMMAND_SET_MEDIA_ITEM)
1104-
|| playerWrapper.isCommandAvailable(COMMAND_CHANGE_MEDIA_ITEMS);
1105-
ControllerInfo controllerForRequest = resolveControllerInfoForCallback(controller);
1106-
Player.Commands playCommand =
1107-
new Player.Commands.Builder().add(Player.COMMAND_PLAY_PAUSE).build();
1108-
if (hasCurrentMediaItem || !canAddMediaItems) {
1109-
// No playback resumption needed or possible.
1110-
if (!hasCurrentMediaItem) {
1111-
Log.w(
1112-
TAG,
1113-
"Play requested without current MediaItem, but playback resumption prevented by"
1114-
+ " missing available commands");
1115-
}
1116-
Util.handlePlayButtonAction(playerWrapper);
1117-
if (callOnPlayerInteractionFinished) {
1118-
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1119-
}
1120-
} else {
1121-
@Nullable
1122-
ListenableFuture<MediaItemsWithStartPosition> future =
1123-
checkNotNull(
1124-
callback.onPlaybackResumption(instance, controllerForRequest),
1125-
"Callback.onPlaybackResumption must return a non-null future");
1126-
Futures.addCallback(
1127-
future,
1128-
new FutureCallback<MediaItemsWithStartPosition>() {
1129-
@Override
1130-
public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) {
1131-
callWithControllerForCurrentRequestSet(
1132-
controllerForRequest,
1133-
() -> {
1134-
MediaUtils.setMediaItemsWithStartIndexAndPosition(
1135-
playerWrapper, mediaItemsWithStartPosition);
1136-
Util.handlePlayButtonAction(playerWrapper);
1137-
if (callOnPlayerInteractionFinished) {
1138-
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1139-
}
1140-
})
1141-
.run();
1098+
SettableFuture<SessionResult> sessionFuture = SettableFuture.create();
1099+
ListenableFuture<Boolean> playRequestedFuture = onPlayRequested();
1100+
playRequestedFuture.addListener(
1101+
() -> {
1102+
boolean playRequested;
1103+
try {
1104+
playRequested = playRequestedFuture.get();
1105+
} catch (ExecutionException | InterruptedException e) {
1106+
sessionFuture.setException(new IllegalStateException(e));
1107+
return;
1108+
}
1109+
if (!playRequested) {
1110+
// Request denied, e.g. due to missing foreground service abilities.
1111+
sessionFuture.set(new SessionResult(SessionResult.RESULT_ERROR_UNKNOWN));
1112+
return;
1113+
}
1114+
boolean hasCurrentMediaItem =
1115+
playerWrapper.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)
1116+
&& playerWrapper.getCurrentMediaItem() != null;
1117+
boolean canAddMediaItems =
1118+
playerWrapper.isCommandAvailable(COMMAND_SET_MEDIA_ITEM)
1119+
|| playerWrapper.isCommandAvailable(COMMAND_CHANGE_MEDIA_ITEMS);
1120+
ControllerInfo controllerForRequest = resolveControllerInfoForCallback(controller);
1121+
Player.Commands playCommand =
1122+
new Player.Commands.Builder().add(Player.COMMAND_PLAY_PAUSE).build();
1123+
if (hasCurrentMediaItem || !canAddMediaItems) {
1124+
// No playback resumption needed or possible.
1125+
if (!hasCurrentMediaItem) {
1126+
Log.w(
1127+
TAG,
1128+
"Play requested without current MediaItem, but playback resumption prevented by"
1129+
+ " missing available commands");
11421130
}
1143-
1144-
@Override
1145-
public void onFailure(Throwable t) {
1146-
if (t instanceof UnsupportedOperationException) {
1147-
Log.w(
1148-
TAG,
1149-
"UnsupportedOperationException: Make sure to implement"
1150-
+ " MediaSession.Callback.onPlaybackResumption() if you add a"
1151-
+ " media button receiver to your manifest or if you implement the recent"
1152-
+ " media item contract with your MediaLibraryService.",
1153-
t);
1154-
} else {
1155-
Log.e(
1156-
TAG,
1157-
"Failure calling MediaSession.Callback.onPlaybackResumption(): "
1158-
+ t.getMessage(),
1159-
t);
1160-
}
1161-
// Play as requested even if playback resumption fails.
1162-
Util.handlePlayButtonAction(playerWrapper);
1163-
if (callOnPlayerInteractionFinished) {
1164-
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1165-
}
1131+
Util.handlePlayButtonAction(playerWrapper);
1132+
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
1133+
if (callOnPlayerInteractionFinished) {
1134+
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
11661135
}
1167-
},
1168-
this::postOrRunOnApplicationHandler);
1169-
}
1136+
} else {
1137+
@Nullable
1138+
ListenableFuture<MediaItemsWithStartPosition> future =
1139+
checkNotNull(
1140+
callback.onPlaybackResumption(instance, controllerForRequest),
1141+
"Callback.onPlaybackResumption must return a non-null future");
1142+
Futures.addCallback(
1143+
future,
1144+
new FutureCallback<MediaItemsWithStartPosition>() {
1145+
@Override
1146+
public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) {
1147+
callWithControllerForCurrentRequestSet(
1148+
controllerForRequest,
1149+
() -> {
1150+
MediaUtils.setMediaItemsWithStartIndexAndPosition(
1151+
playerWrapper, mediaItemsWithStartPosition);
1152+
Util.handlePlayButtonAction(playerWrapper);
1153+
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
1154+
if (callOnPlayerInteractionFinished) {
1155+
onPlayerInteractionFinishedOnHandler(
1156+
controllerForRequest, playCommand);
1157+
}
1158+
})
1159+
.run();
1160+
}
1161+
1162+
@Override
1163+
public void onFailure(Throwable t) {
1164+
if (t instanceof UnsupportedOperationException) {
1165+
Log.w(
1166+
TAG,
1167+
"UnsupportedOperationException: Make sure to implement"
1168+
+ " MediaSession.Callback.onPlaybackResumption() if you add a media"
1169+
+ " button receiver to your manifest or if you implement the recent"
1170+
+ " media item contract with your MediaLibraryService.",
1171+
t);
1172+
} else {
1173+
Log.e(
1174+
TAG,
1175+
"Failure calling MediaSession.Callback.onPlaybackResumption(): "
1176+
+ t.getMessage(),
1177+
t);
1178+
}
1179+
// Play as requested even if playback resumption fails.
1180+
Util.handlePlayButtonAction(playerWrapper);
1181+
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
1182+
if (callOnPlayerInteractionFinished) {
1183+
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1184+
}
1185+
}
1186+
},
1187+
this::postOrRunOnApplicationHandler);
1188+
}
1189+
},
1190+
this::postOrRunOnApplicationHandler);
1191+
return sessionFuture;
11701192
}
11711193

11721194
private void setLegacyMediaButtonPreferences(

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

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -391,9 +391,27 @@ public void onPrepareFromUri(@Nullable Uri mediaUri, @Nullable Bundle extras) {
391391
public void onPlay() {
392392
dispatchSessionTaskWithPlayerCommand(
393393
COMMAND_PLAY_PAUSE,
394-
controller ->
395-
sessionImpl.handleMediaControllerPlayRequest(
396-
controller, /* callOnPlayerInteractionFinished= */ true),
394+
controller -> {
395+
ListenableFuture<SessionResult> resultFuture =
396+
sessionImpl.handleMediaControllerPlayRequest(
397+
controller, /* callOnPlayerInteractionFinished= */ true);
398+
Futures.addCallback(
399+
resultFuture,
400+
new FutureCallback<SessionResult>() {
401+
@Override
402+
public void onSuccess(SessionResult result) {
403+
if (result.resultCode != RESULT_SUCCESS) {
404+
Log.w(TAG, "onPlay() failed: " + result + " (from: " + controller + ")");
405+
}
406+
}
407+
408+
@Override
409+
public void onFailure(Throwable t) {
410+
Log.e(TAG, "Unexpected exception in onPlay() of " + controller, t);
411+
}
412+
},
413+
MoreExecutors.directExecutor());
414+
},
397415
sessionCompat.getCurrentControllerInfo(),
398416
/* callOnPlayerInteractionFinished= */ false);
399417
}

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -738,15 +738,10 @@ public void playForControllerInfo(ControllerInfo controller, int sequenceNumber)
738738
controller,
739739
sequenceNumber,
740740
COMMAND_PLAY_PAUSE,
741-
sendSessionResultSuccess(
742-
player -> {
743-
@Nullable MediaSessionImpl impl = sessionImpl.get();
744-
if (impl == null || impl.isReleased()) {
745-
return;
746-
}
747-
impl.handleMediaControllerPlayRequest(
748-
controller, /* callOnPlayerInteractionFinished= */ false);
749-
}));
741+
sendSessionResultWhenReady(
742+
(session, theController, sequenceId) ->
743+
session.handleMediaControllerPlayRequest(
744+
theController, /* callOnPlayerInteractionFinished= */ false)));
750745
}
751746

752747
@Override

0 commit comments

Comments
 (0)