From f9c6fb4e90b6190c0653a491dd7aacc766246bc6 Mon Sep 17 00:00:00 2001 From: tonihei Date: Mon, 28 Nov 2022 09:25:18 +0000 Subject: [PATCH] Ensure messages sent on a dead thread don't block FakeClock execution FakeClock keeps an internal list of messages to be executed to ensure deterministic serialization. The next message from the list is triggered by a separate helper message sent to the real Handler. However, if the target HandlerThread is no longer alive (e.g. when it quit itself during the message execution), this helper message is never executed and the entire message execution chain is stuck forever. This can be solved by checking the return values of Hander.post or Handler.sendMessage, which are false if the message won't be delivered. If the messages are not delivered, we can unblock the chain by marking the message as complete and triggering the next one. PiperOrigin-RevId: 491275031 (cherry picked from commit 8fcc06309323847b47ed8ab225cd861335448d36) --- .../androidx/media3/test/utils/FakeClock.java | 17 +++--- .../media3/test/utils/FakeClockTest.java | 54 +++++++++++++++++-- 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/FakeClock.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/FakeClock.java index bbd558208a..db37f66405 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/FakeClock.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/FakeClock.java @@ -244,16 +244,19 @@ public class FakeClock implements Clock { } handlerMessages.remove(messageIndex); waitingForMessage = true; + boolean messageSent; + Handler realHandler = message.handler.handler; if (message.runnable != null) { - message.handler.handler.post(message.runnable); + messageSent = realHandler.post(message.runnable); } else { - message - .handler - .handler - .obtainMessage(message.what, message.arg1, message.arg2, message.obj) - .sendToTarget(); + messageSent = + realHandler.sendMessage( + realHandler.obtainMessage(message.what, message.arg1, message.arg2, message.obj)); + } + messageSent &= message.handler.internalHandler.post(this::onMessageHandled); + if (!messageSent) { + onMessageHandled(); } - message.handler.internalHandler.post(this::onMessageHandled); } private synchronized void onMessageHandled() { diff --git a/libraries/test_utils/src/test/java/androidx/media3/test/utils/FakeClockTest.java b/libraries/test_utils/src/test/java/androidx/media3/test/utils/FakeClockTest.java index 393ce98408..51dd19bab5 100644 --- a/libraries/test_utils/src/test/java/androidx/media3/test/utils/FakeClockTest.java +++ b/libraries/test_utils/src/test/java/androidx/media3/test/utils/FakeClockTest.java @@ -29,6 +29,7 @@ import com.google.common.base.Objects; import com.google.common.collect.Iterables; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.shadows.ShadowLooper; @@ -40,6 +41,7 @@ public final class FakeClockTest { @Test public void currentTimeMillis_withoutBootTime() { FakeClock fakeClock = new FakeClock(/* initialTimeMs= */ 10); + assertThat(fakeClock.currentTimeMillis()).isEqualTo(10); } @@ -48,6 +50,7 @@ public final class FakeClockTest { FakeClock fakeClock = new FakeClock( /* bootTimeMs= */ 150, /* initialTimeMs= */ 200, /* isAutoAdvancing= */ false); + assertThat(fakeClock.currentTimeMillis()).isEqualTo(350); } @@ -55,17 +58,24 @@ public final class FakeClockTest { public void currentTimeMillis_afterAdvanceTime_currentTimeHasAdvanced() { FakeClock fakeClock = new FakeClock(/* bootTimeMs= */ 100, /* initialTimeMs= */ 50, /* isAutoAdvancing= */ false); + fakeClock.advanceTime(/* timeDiffMs */ 250); + assertThat(fakeClock.currentTimeMillis()).isEqualTo(400); } @Test public void elapsedRealtime_afterAdvanceTime_timeHasAdvanced() { FakeClock fakeClock = new FakeClock(2000); + assertThat(fakeClock.elapsedRealtime()).isEqualTo(2000); + fakeClock.advanceTime(500); + assertThat(fakeClock.elapsedRealtime()).isEqualTo(2500); + fakeClock.advanceTime(0); + assertThat(fakeClock.elapsedRealtime()).isEqualTo(2500); } @@ -86,6 +96,7 @@ public final class FakeClockTest { .sendToTarget(); ShadowLooper.idleMainLooper(); shadowOf(handler.getLooper()).idle(); + handlerThread.quitSafely(); assertThat(callback.messages) .containsExactly( @@ -126,6 +137,7 @@ public final class FakeClockTest { fakeClock.advanceTime(50); shadowOf(handler.getLooper()).idle(); + handlerThread.quitSafely(); assertThat(callback.messages).hasSize(4); assertThat(Iterables.getLast(callback.messages)) @@ -146,6 +158,7 @@ public final class FakeClockTest { handler.obtainMessage(/* what= */ 4).sendToTarget(); ShadowLooper.idleMainLooper(); shadowOf(handler.getLooper()).idle(); + handlerThread.quitSafely(); assertThat(callback.messages) .containsExactly( @@ -192,6 +205,8 @@ public final class FakeClockTest { fakeClock.advanceTime(1000); shadowOf(handler.getLooper()).idle(); assertTestRunnableStates(new boolean[] {true, true, true, true, true}, testRunnables); + + handlerThread.quitSafely(); } @Test @@ -203,7 +218,6 @@ public final class FakeClockTest { HandlerWrapper handler = fakeClock.createHandler(handlerThread.getLooper(), callback); TestCallback otherCallback = new TestCallback(); HandlerWrapper otherHandler = fakeClock.createHandler(handlerThread.getLooper(), otherCallback); - TestRunnable testRunnable1 = new TestRunnable(); TestRunnable testRunnable2 = new TestRunnable(); Object messageToken = new Object(); @@ -216,10 +230,10 @@ public final class FakeClockTest { handler.removeMessages(/* what= */ 2); handler.removeCallbacksAndMessages(messageToken); - fakeClock.advanceTime(50); ShadowLooper.idleMainLooper(); shadowOf(handlerThread.getLooper()).idle(); + handlerThread.quitSafely(); assertThat(callback.messages) .containsExactly( @@ -242,7 +256,6 @@ public final class FakeClockTest { HandlerWrapper handler = fakeClock.createHandler(handlerThread.getLooper(), callback); TestCallback otherCallback = new TestCallback(); HandlerWrapper otherHandler = fakeClock.createHandler(handlerThread.getLooper(), otherCallback); - TestRunnable testRunnable1 = new TestRunnable(); TestRunnable testRunnable2 = new TestRunnable(); Object messageToken = new Object(); @@ -254,15 +267,14 @@ public final class FakeClockTest { otherHandler.sendEmptyMessage(/* what= */ 1); handler.removeCallbacksAndMessages(/* token= */ null); - fakeClock.advanceTime(50); ShadowLooper.idleMainLooper(); shadowOf(handlerThread.getLooper()).idle(); + handlerThread.quitSafely(); assertThat(callback.messages).isEmpty(); assertThat(testRunnable1.hasRun).isFalse(); assertThat(testRunnable2.hasRun).isFalse(); - // Assert that message on other handler wasn't removed. assertThat(otherCallback.messages) .containsExactly( @@ -295,6 +307,7 @@ public final class FakeClockTest { }); ShadowLooper.idleMainLooper(); shadowOf(handler.getLooper()).idle(); + handlerThread.quitSafely(); assertThat(clockTimes).containsExactly(0L, 20L, 50L, 70L, 100L).inOrder(); } @@ -333,6 +346,8 @@ public final class FakeClockTest { }); ShadowLooper.idleMainLooper(); messagesFinished.block(); + handlerThread1.quitSafely(); + handlerThread2.quitSafely(); assertThat(executionOrder).containsExactly(1, 2, 3, 4, 5, 6, 7, 8).inOrder(); } @@ -368,10 +383,39 @@ public final class FakeClockTest { ShadowLooper.idleMainLooper(); shadowOf(handler1.getLooper()).idle(); shadowOf(handler2.getLooper()).idle(); + handlerThread1.quitSafely(); + handlerThread2.quitSafely(); assertThat(executionOrder).containsExactly(1, 2, 3, 4).inOrder(); } + @Test + public void createHandler_messageOnDeadThread_doesNotBlockExecution() { + HandlerThread handlerThread1 = new HandlerThread("FakeClockTest"); + handlerThread1.start(); + HandlerThread handlerThread2 = new HandlerThread("FakeClockTest"); + handlerThread2.start(); + FakeClock fakeClock = new FakeClock(/* initialTimeMs= */ 0); + HandlerWrapper handler1 = + fakeClock.createHandler(handlerThread1.getLooper(), /* callback= */ null); + HandlerWrapper handler2 = + fakeClock.createHandler(handlerThread2.getLooper(), /* callback= */ null); + + ConditionVariable messagesFinished = new ConditionVariable(); + AtomicBoolean messageOnDeadThreadExecuted = new AtomicBoolean(); + handler1.post( + () -> { + handlerThread1.quitSafely(); + handler1.post(() -> messageOnDeadThreadExecuted.set(true)); + handler2.post(messagesFinished::open); + }); + ShadowLooper.idleMainLooper(); + messagesFinished.block(); + handlerThread2.quitSafely(); + + assertThat(messageOnDeadThreadExecuted.get()).isFalse(); + } + private static void assertTestRunnableStates(boolean[] states, TestRunnable[] testRunnables) { for (int i = 0; i < testRunnables.length; i++) { assertThat(testRunnables[i].hasRun).isEqualTo(states[i]);