Skip to content

Commit c760c50

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

File tree

3 files changed

+128
-95
lines changed

3 files changed

+128
-95
lines changed

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

Lines changed: 109 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@
8888
import com.google.common.util.concurrent.FutureCallback;
8989
import com.google.common.util.concurrent.Futures;
9090
import com.google.common.util.concurrent.ListenableFuture;
91+
import com.google.common.util.concurrent.MoreExecutors;
9192
import com.google.common.util.concurrent.SettableFuture;
9293
import java.lang.ref.WeakReference;
9394
import java.util.ArrayList;
@@ -938,20 +939,23 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
938939
});
939940
}
940941

941-
/* package */ boolean onPlayRequested() {
942+
/* package */ ListenableFuture<Boolean> onPlayRequested() {
942943
if (Looper.myLooper() != Looper.getMainLooper()) {
943944
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-
}
945+
mainHandler.post(
946+
() -> {
947+
try {
948+
playRequested.set(onPlayRequested().get());
949+
} catch (ExecutionException | InterruptedException e) {
950+
playRequested.setException(new IllegalStateException(e));
951+
}
952+
});
953+
return playRequested;
950954
}
951955
if (this.mediaSessionListener != null) {
952-
return this.mediaSessionListener.onPlayRequested(instance);
956+
return Futures.immediateFuture(this.mediaSessionListener.onPlayRequested(instance));
953957
}
954-
return true;
958+
return Futures.immediateFuture(true);
955959
}
956960

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

10441070
private void setLegacyMediaButtonPreferences(

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

Lines changed: 15 additions & 3 deletions
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
}

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

Lines changed: 4 additions & 9 deletions
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)