Explicitly prevent callbacks after player is released

We currently implicitly rely on the internal playback thread to not send
new updates after the player got released. This may not always be ensured
since we let the release call timeout. For the timeout case, there may
still be a pending operation returning much later when it unstuck itself.

Fix this and potential other edge cases by explicitly removing all listeners
and preventing new listeners from being added after the release.

PiperOrigin-RevId: 338217220
This commit is contained in:
tonihei 2020-10-21 09:02:47 +01:00 committed by Oliver Woodman
parent 6abee66504
commit 54506b506b
3 changed files with 57 additions and 0 deletions

View file

@ -721,6 +721,7 @@ import java.util.concurrent.TimeoutException;
new TimeoutException("Player release timed out."),
ExoPlaybackException.TIMEOUT_OPERATION_RELEASE)));
}
listeners.release();
playbackInfoUpdateHandler.removeCallbacksAndMessages(null);
if (analyticsCollector != null) {
bandwidthMeter.removeEventListener(analyticsCollector);

View file

@ -48,6 +48,8 @@ public final class ListenerSet<T> {
private final ArrayDeque<Runnable> flushingEvents;
private final ArrayDeque<Runnable> queuedEvents;
private boolean released;
/** Creates the listener set. */
public ListenerSet() {
listeners = new CopyOnWriteArraySet<>();
@ -63,6 +65,9 @@ public final class ListenerSet<T> {
* @param listener The listener to be added.
*/
public void add(T listener) {
if (released) {
return;
}
Assertions.checkNotNull(listener);
listeners.add(new ListenerHolder<T>(listener));
}
@ -124,6 +129,19 @@ public final class ListenerSet<T> {
flushEvents();
}
/**
* Releases the set of listeners.
*
* <p>This will ensure no events are sent to any listener after this method has been called.
*/
public void release() {
for (ListenerHolder<T> listenerHolder : listeners) {
listenerHolder.release();
}
listeners.clear();
released = true;
}
private static final class ListenerHolder<T> {
@Nonnull public final T listener;

View file

@ -184,6 +184,44 @@ public class ListenerSetTest {
verify(listener2, times(2)).callback1();
}
@Test
public void release_stopsForwardingEventsImmediately() {
ListenerSet<TestListener> listenerSet = new ListenerSet<>();
TestListener listener2 = mock(TestListener.class);
// Listener1 releases the set from within the callback.
TestListener listener1 =
spy(
new TestListener() {
@Override
public void callback1() {
listenerSet.release();
}
});
listenerSet.add(listener1);
listenerSet.add(listener2);
// Listener2 shouldn't even get this event as it's released before the event can be invoked.
listenerSet.sendEvent(TestListener::callback1);
listenerSet.sendEvent(TestListener::callback2);
verify(listener1).callback1();
verify(listener2, never()).callback1();
verify(listener1, never()).callback2();
verify(listener2, never()).callback2();
}
@Test
public void release_preventsRegisteringNewListeners() {
ListenerSet<TestListener> listenerSet = new ListenerSet<>();
TestListener listener = mock(TestListener.class);
listenerSet.release();
listenerSet.add(listener);
listenerSet.sendEvent(TestListener::callback1);
verify(listener, never()).callback1();
}
private interface TestListener {
default void callback1() {}