Skip to content

session: fix a deadlock due to onPlayRequested() #2256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+124 −95
Open

session: fix a deadlock due to onPlayRequested() #2256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 105 additions & 83 deletions libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -938,20 +938,23 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
});
}

/* package */ boolean onPlayRequested() {
/* package */ ListenableFuture<Boolean> onPlayRequested() {
if (Looper.myLooper() != Looper.getMainLooper()) {
SettableFuture<Boolean> playRequested = SettableFuture.create();
mainHandler.post(() -> playRequested.set(onPlayRequested()));
try {
return playRequested.get();
} catch (InterruptedException | ExecutionException e) {
throw new IllegalStateException(e);
}
mainHandler.post(
() -> {
try {
playRequested.set(onPlayRequested().get());
} catch (ExecutionException | InterruptedException e) {
playRequested.setException(new IllegalStateException(e));
}
});
return playRequested;
}
if (this.mediaSessionListener != null) {
return this.mediaSessionListener.onPlayRequested(instance);
return Futures.immediateFuture(this.mediaSessionListener.onPlayRequested(instance));
}
return true;
return Futures.immediateFuture(true);
}

/**
Expand All @@ -962,83 +965,102 @@ protected MediaSessionServiceLegacyStub getLegacyBrowserService() {
*
* @param controller The controller requesting to play.
*/
/* package */ void handleMediaControllerPlayRequest(
/* package */ ListenableFuture<SessionResult> handleMediaControllerPlayRequest(
ControllerInfo controller, boolean callOnPlayerInteractionFinished) {
if (!onPlayRequested()) {
// Request denied, e.g. due to missing foreground service abilities.
return;
}
boolean hasCurrentMediaItem =
playerWrapper.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)
&& playerWrapper.getCurrentMediaItem() != null;
boolean canAddMediaItems =
playerWrapper.isCommandAvailable(COMMAND_SET_MEDIA_ITEM)
|| playerWrapper.isCommandAvailable(COMMAND_CHANGE_MEDIA_ITEMS);
ControllerInfo controllerForRequest = resolveControllerInfoForCallback(controller);
Player.Commands playCommand =
new Player.Commands.Builder().add(Player.COMMAND_PLAY_PAUSE).build();
if (hasCurrentMediaItem || !canAddMediaItems) {
// No playback resumption needed or possible.
if (!hasCurrentMediaItem) {
Log.w(
TAG,
"Play requested without current MediaItem, but playback resumption prevented by"
+ " missing available commands");
}
Util.handlePlayButtonAction(playerWrapper);
if (callOnPlayerInteractionFinished) {
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
}
} else {
@Nullable
ListenableFuture<MediaItemsWithStartPosition> future =
checkNotNull(
callback.onPlaybackResumption(instance, controllerForRequest),
"Callback.onPlaybackResumption must return a non-null future");
Futures.addCallback(
future,
new FutureCallback<MediaItemsWithStartPosition>() {
@Override
public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) {
callWithControllerForCurrentRequestSet(
controllerForRequest,
() -> {
MediaUtils.setMediaItemsWithStartIndexAndPosition(
playerWrapper, mediaItemsWithStartPosition);
Util.handlePlayButtonAction(playerWrapper);
if (callOnPlayerInteractionFinished) {
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
}
})
.run();
SettableFuture<SessionResult> sessionFuture = SettableFuture.create();
ListenableFuture<Boolean> playRequestedFuture = onPlayRequested();
playRequestedFuture.addListener(
() -> {
boolean playRequested;
try {
playRequested = playRequestedFuture.get();
} catch (ExecutionException | InterruptedException e) {
sessionFuture.setException(new IllegalStateException(e));
return;
}
if (!playRequested) {
// Request denied, e.g. due to missing foreground service abilities.
sessionFuture.set(new SessionResult(SessionResult.RESULT_ERROR_UNKNOWN));
return;
}
boolean hasCurrentMediaItem =
playerWrapper.isCommandAvailable(Player.COMMAND_GET_CURRENT_MEDIA_ITEM)
&& playerWrapper.getCurrentMediaItem() != null;
boolean canAddMediaItems =
playerWrapper.isCommandAvailable(COMMAND_SET_MEDIA_ITEM)
|| playerWrapper.isCommandAvailable(COMMAND_CHANGE_MEDIA_ITEMS);
ControllerInfo controllerForRequest = resolveControllerInfoForCallback(controller);
Player.Commands playCommand =
new Player.Commands.Builder().add(Player.COMMAND_PLAY_PAUSE).build();
if (hasCurrentMediaItem || !canAddMediaItems) {
// No playback resumption needed or possible.
if (!hasCurrentMediaItem) {
Log.w(
TAG,
"Play requested without current MediaItem, but playback resumption prevented by"
+ " missing available commands");
}

@Override
public void onFailure(Throwable t) {
if (t instanceof UnsupportedOperationException) {
Log.w(
TAG,
"UnsupportedOperationException: Make sure to implement"
+ " MediaSession.Callback.onPlaybackResumption() if you add a"
+ " media button receiver to your manifest or if you implement the recent"
+ " media item contract with your MediaLibraryService.",
t);
} else {
Log.e(
TAG,
"Failure calling MediaSession.Callback.onPlaybackResumption(): "
+ t.getMessage(),
t);
}
// Play as requested even if playback resumption fails.
Util.handlePlayButtonAction(playerWrapper);
if (callOnPlayerInteractionFinished) {
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
}
Util.handlePlayButtonAction(playerWrapper);
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
if (callOnPlayerInteractionFinished) {
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
}
},
this::postOrRunOnApplicationHandler);
}
} else {
@Nullable
ListenableFuture<MediaItemsWithStartPosition> future =
checkNotNull(
callback.onPlaybackResumption(instance, controllerForRequest),
"Callback.onPlaybackResumption must return a non-null future");
Futures.addCallback(
future,
new FutureCallback<MediaItemsWithStartPosition>() {
@Override
public void onSuccess(MediaItemsWithStartPosition mediaItemsWithStartPosition) {
callWithControllerForCurrentRequestSet(
controllerForRequest,
() -> {
MediaUtils.setMediaItemsWithStartIndexAndPosition(
playerWrapper, mediaItemsWithStartPosition);
Util.handlePlayButtonAction(playerWrapper);
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
if (callOnPlayerInteractionFinished) {
onPlayerInteractionFinishedOnHandler(
controllerForRequest, playCommand);
}
})
.run();
}

@Override
public void onFailure(Throwable t) {
if (t instanceof UnsupportedOperationException) {
Log.w(
TAG,
"UnsupportedOperationException: Make sure to implement"
+ " MediaSession.Callback.onPlaybackResumption() if you add a media"
+ " button receiver to your manifest or if you implement the recent"
+ " media item contract with your MediaLibraryService.",
t);
} else {
Log.e(
TAG,
"Failure calling MediaSession.Callback.onPlaybackResumption(): "
+ t.getMessage(),
t);
}
// Play as requested even if playback resumption fails.
Util.handlePlayButtonAction(playerWrapper);
sessionFuture.set(new SessionResult(SessionResult.RESULT_SUCCESS));
if (callOnPlayerInteractionFinished) {
onPlayerInteractionFinishedOnHandler(controllerForRequest, playCommand);
}
}
},
this::postOrRunOnApplicationHandler);
}
},
this::postOrRunOnApplicationHandler);
return sessionFuture;
}

private void setLegacyMediaButtonPreferences(
Expand Down
18 changes: 15 additions & 3 deletions libraries/session/src/main/java/androidx/media3/session/MediaSessionLegacyStub.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,21 @@ public void onPrepareFromUri(@Nullable Uri mediaUri, @Nullable Bundle extras) {
public void onPlay() {
dispatchSessionTaskWithPlayerCommand(
COMMAND_PLAY_PAUSE,
controller ->
sessionImpl.handleMediaControllerPlayRequest(
controller, /* callOnPlayerInteractionFinished= */ true),
controller -> {
try {
// Call get() to make sure the call is blocking.
SessionResult result =
sessionImpl
.handleMediaControllerPlayRequest(
controller, /* callOnPlayerInteractionFinished= */ true)
.get();
if (result.resultCode != RESULT_SUCCESS) {
Log.w(TAG, "onPlay() failed: " + result + " (from: " + controller + ")");
}
} catch (ExecutionException | InterruptedException e) {
throw new IllegalStateException("Unexpected exception in onPlay() of " + controller, e);
}
},
sessionCompat.getCurrentControllerInfo(),
/* callOnPlayerInteractionFinished= */ false);
}
Expand Down
13 changes: 4 additions & 9 deletions libraries/session/src/main/java/androidx/media3/session/MediaSessionStub.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -732,15 +732,10 @@ public void playForControllerInfo(ControllerInfo controller, int sequenceNumber)
controller,
sequenceNumber,
COMMAND_PLAY_PAUSE,
sendSessionResultSuccess(
player -> {
@Nullable MediaSessionImpl impl = sessionImpl.get();
if (impl == null || impl.isReleased()) {
return;
}
impl.handleMediaControllerPlayRequest(
controller, /* callOnPlayerInteractionFinished= */ false);
}));
sendSessionResultWhenReady(
(session, theController, sequenceId) ->
session.handleMediaControllerPlayRequest(
theController, /* callOnPlayerInteractionFinished= */ false)));
}

@Override
Expand Down