Skip to content

Commit 21bfd94

Browse files
nift4nift4
committed
session: fix a deadlock due to onPlayRequested()
Issue: #1197
1 parent 1918a25 commit 21bfd94

File tree

3 files changed

+124
-95
lines changed

3 files changed

+124
-95
lines changed

Diff for: ‎libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java

+105-83
Original file line numberDiff line numberDiff line change
@@ -938,20 +938,23 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
938938
});
939939
}
940940

941-
/* package */ boolean onPlayRequested() {
941+
/* package */ ListenableFuture<Boolean> onPlayRequested() {
942942
if (Looper.myLooper() != Looper.getMainLooper()) {
943943
SettableFuture<Boolean> playRequested = SettableFuture.create();
944-
mainHandler.post(() -> playRequested.set(onPlayRequested()));
945-
try {
946-
return playRequested.get();
947-
} catch (InterruptedException | ExecutionException e) {
948-
throw new IllegalStateException(e);
949-
}
944+
mainHandler.post(
945+
() -> {
946+
try {
947+
playRequested.set(onPlayRequested().get());
948+
} catch (ExecutionException | InterruptedException e) {
949+
playRequested.setException(new IllegalStateException(e));
950+
}
951+
});
952+
return playRequested;
950953
}
951954
if (this.mediaSessionListener != null) {
952-
return this.mediaSessionListener.onPlayRequested(instance);
955+
return Futures.immediateFuture(this.mediaSessionListener.onPlayRequested(instance));
953956
}
954-
return true;
957+
return Futures.immediateFuture(true);
955958
}
956959

957960
/**
@@ -962,83 +965,102 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
962965
*
963966
* @param controller The controller requesting to play.
964967
*/
965-
/* package */ void handleMediaControllerPlayRequest(
968+
/* package */ ListenableFuture<SessionResult> handleMediaControllerPlayRequest(
966969
ControllerInfo controller, boolean callOnPlayerInteractionFinished) {
967-
if (!onPlayRequested()) {
968-
// Request denied, e.g. due to missing foreground service abilities.
969-
return;
970-
}
971-
boolean hasCurrentMediaItem =
972-
playerWrapper.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)
973-
&& playerWrapper.getCurrentMediaItem() != null;
974-
boolean canAddMediaItems =
975-
playerWrapper.isCommandAvailable(COMMAND_SET_MEDIA_ITEM)
976-
|| playerWrapper.isCommandAvailable(COMMAND_CHANGE_MEDIA_ITEMS);
977-
ControllerInfo controllerForRequest = resolveControllerInfoForCallback(controller);
978-
Player.Commands playCommand =
979-
new Player.Commands.Builder().add(Player.COMMAND_PLAY_PAUSE).build();
980-
if (hasCurrentMediaItem || !canAddMediaItems) {
981-
// No playback resumption needed or possible.
982-
if (!hasCurrentMediaItem) {
983-
Log.w(
984-
TAG,
985-
"Play requested without current MediaItem, but playback resumption prevented by"
986-
+ " missing available commands");
987-
}
988-
Util.handlePlayButtonAction(playerWrapper);
989-
if (callOnPlayerInteractionFinished) {
990-
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
991-
}
992-
} else {
993-
@Nullable
994-
ListenableFuture<MediaItemsWithStartPosition> future =
995-
checkNotNull(
996-
callback.onPlaybackResumption(instance, controllerForRequest),
997-
"Callback.onPlaybackResumption must return a non-null future");
998-
Futures.addCallback(
999-
future,
1000-
new FutureCallback<MediaItemsWithStartPosition>() {
1001-
@Override
1002-
public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) {
1003-
callWithControllerForCurrentRequestSet(
1004-
controllerForRequest,
1005-
() -> {
1006-
MediaUtils.setMediaItemsWithStartIndexAndPosition(
1007-
playerWrapper, mediaItemsWithStartPosition);
1008-
Util.handlePlayButtonAction(playerWrapper);
1009-
if (callOnPlayerInteractionFinished) {
1010-
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1011-
}
1012-
})
1013-
.run();
970+
SettableFuture<SessionResult> sessionFuture = SettableFuture.create();
971+
ListenableFuture<Boolean> playRequestedFuture = onPlayRequested();
972+
playRequestedFuture.addListener(
973+
() -> {
974+
boolean playRequested;
975+
try {
976+
playRequested = playRequestedFuture.get();
977+
} catch (ExecutionException | InterruptedException e) {
978+
sessionFuture.setException(new IllegalStateException(e));
979+
return;
980+
}
981+
if (!playRequested) {
982+
// Request denied, e.g. due to missing foreground service abilities.
983+
sessionFuture.set(new SessionResult(SessionResult.RESULT_ERROR_UNKNOWN));
984+
return;
985+
}
986+
boolean hasCurrentMediaItem =
987+
playerWrapper.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)
988+
&& playerWrapper.getCurrentMediaItem() != null;
989+
boolean canAddMediaItems =
990+
playerWrapper.isCommandAvailable(COMMAND_SET_MEDIA_ITEM)
991+
|| playerWrapper.isCommandAvailable(COMMAND_CHANGE_MEDIA_ITEMS);
992+
ControllerInfo controllerForRequest = resolveControllerInfoForCallback(controller);
993+
Player.Commands playCommand =
994+
new Player.Commands.Builder().add(Player.COMMAND_PLAY_PAUSE).build();
995+
if (hasCurrentMediaItem || !canAddMediaItems) {
996+
// No playback resumption needed or possible.
997+
if (!hasCurrentMediaItem) {
998+
Log.w(
999+
TAG,
1000+
"Play requested without current MediaItem, but playback resumption prevented by"
1001+
+ " missing available commands");
10141002
}
1015-
1016-
@Override
1017-
public void onFailure(Throwable t) {
1018-
if (t instanceof UnsupportedOperationException) {
1019-
Log.w(
1020-
TAG,
1021-
"UnsupportedOperationException: Make sure to implement"
1022-
+ " MediaSession.Callback.onPlaybackResumption() if you add a"
1023-
+ " media button receiver to your manifest or if you implement the recent"
1024-
+ " media item contract with your MediaLibraryService.",
1025-
t);
1026-
} else {
1027-
Log.e(
1028-
TAG,
1029-
"Failure calling MediaSession.Callback.onPlaybackResumption(): "
1030-
+ t.getMessage(),
1031-
t);
1032-
}
1033-
// Play as requested even if playback resumption fails.
1034-
Util.handlePlayButtonAction(playerWrapper);
1035-
if (callOnPlayerInteractionFinished) {
1036-
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1037-
}
1003+
Util.handlePlayButtonAction(playerWrapper);
1004+
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
1005+
if (callOnPlayerInteractionFinished) {
1006+
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
10381007
}
1039-
},
1040-
this::postOrRunOnApplicationHandler);
1041-
}
1008+
} else {
1009+
@Nullable
1010+
ListenableFuture<MediaItemsWithStartPosition> future =
1011+
checkNotNull(
1012+
callback.onPlaybackResumption(instance, controllerForRequest),
1013+
"Callback.onPlaybackResumption must return a non-null future");
1014+
Futures.addCallback(
1015+
future,
1016+
new FutureCallback<MediaItemsWithStartPosition>() {
1017+
@Override
1018+
public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) {
1019+
callWithControllerForCurrentRequestSet(
1020+
controllerForRequest,
1021+
() -> {
1022+
MediaUtils.setMediaItemsWithStartIndexAndPosition(
1023+
playerWrapper, mediaItemsWithStartPosition);
1024+
Util.handlePlayButtonAction(playerWrapper);
1025+
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
1026+
if (callOnPlayerInteractionFinished) {
1027+
onPlayerInteractionFinishedOnHandler(
1028+
controllerForRequest, playCommand);
1029+
}
1030+
})
1031+
.run();
1032+
}
1033+
1034+
@Override
1035+
public void onFailure(Throwable t) {
1036+
if (t instanceof UnsupportedOperationException) {
1037+
Log.w(
1038+
TAG,
1039+
"UnsupportedOperationException: Make sure to implement"
1040+
+ " MediaSession.Callback.onPlaybackResumption() if you add a media"
1041+
+ " button receiver to your manifest or if you implement the recent"
1042+
+ " media item contract with your MediaLibraryService.",
1043+
t);
1044+
} else {
1045+
Log.e(
1046+
TAG,
1047+
"Failure calling MediaSession.Callback.onPlaybackResumption(): "
1048+
+ t.getMessage(),
1049+
t);
1050+
}
1051+
// Play as requested even if playback resumption fails.
1052+
Util.handlePlayButtonAction(playerWrapper);
1053+
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
1054+
if (callOnPlayerInteractionFinished) {
1055+
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
1056+
}
1057+
}
1058+
},
1059+
this::postOrRunOnApplicationHandler);
1060+
}
1061+
},
1062+
this::postOrRunOnApplicationHandler);
1063+
return sessionFuture;
10421064
}
10431065

10441066
private void setLegacyMediaButtonPreferences(

Diff for: ‎libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java

+15-3
Original file line numberDiff line numberDiff line change
@@ -390,9 +390,21 @@ public void onPrepareFromUri(@Nullable Uri mediaUri, @Nullable Bundle extras) {
390390
public void onPlay() {
391391
dispatchSessionTaskWithPlayerCommand(
392392
COMMAND_PLAY_PAUSE,
393-
controller ->
394-
sessionImpl.handleMediaControllerPlayRequest(
395-
controller, /* callOnPlayerInteractionFinished= */ true),
393+
controller -> {
394+
try {
395+
// Call get() to make sure the call is blocking.
396+
SessionResult result =
397+
sessionImpl
398+
.handleMediaControllerPlayRequest(
399+
controller, /* callOnPlayerInteractionFinished= */ true)
400+
.get();
401+
if (result.resultCode != RESULT_SUCCESS) {
402+
Log.w(TAG, "onPlay() failed: " + result + " (from: " + controller + ")");
403+
}
404+
} catch (ExecutionException | InterruptedException e) {
405+
throw new IllegalStateException("Unexpected exception in onPlay() of " + controller, e);
406+
}
407+
},
396408
sessionCompat.getCurrentControllerInfo(),
397409
/* callOnPlayerInteractionFinished= */ false);
398410
}

Diff for: ‎libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java

+4-9
Original file line numberDiff line numberDiff line change
@@ -732,15 +732,10 @@ public void playForControllerInfo(ControllerInfo controller, int sequenceNumber)
732732
controller,
733733
sequenceNumber,
734734
COMMAND_PLAY_PAUSE,
735-
sendSessionResultSuccess(
736-
player -> {
737-
@Nullable MediaSessionImpl impl = sessionImpl.get();
738-
if (impl == null || impl.isReleased()) {
739-
return;
740-
}
741-
impl.handleMediaControllerPlayRequest(
742-
controller, /* callOnPlayerInteractionFinished= */ false);
743-
}));
735+
sendSessionResultWhenReady(
736+
(session, theController, sequenceId) ->
737+
session.handleMediaControllerPlayRequest(
738+
theController, /* callOnPlayerInteractionFinished= */ false)));
744739
}
745740

746741
@Override

0 commit comments

Comments
 (0)