Commit 83b7f808 by tonihei Committed by christosts

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 2977bef8)
parent a68ab5f3
...@@ -242,16 +242,19 @@ public class FakeClock implements Clock { ...@@ -242,16 +242,19 @@ public class FakeClock implements Clock {
} }
handlerMessages.remove(messageIndex); handlerMessages.remove(messageIndex);
waitingForMessage = true; waitingForMessage = true;
boolean messageSent;
Handler realHandler = message.handler.handler;
if (message.runnable != null) { if (message.runnable != null) {
message.handler.handler.post(message.runnable); messageSent = realHandler.post(message.runnable);
} else { } else {
message messageSent =
.handler realHandler.sendMessage(
.handler realHandler.obtainMessage(message.what, message.arg1, message.arg2, message.obj));
.obtainMessage(message.what, message.arg1, message.arg2, message.obj) }
.sendToTarget(); messageSent &= message.handler.internalHandler.post(this::onMessageHandled);
if (!messageSent) {
onMessageHandled();
} }
message.handler.internalHandler.post(this::onMessageHandled);
} }
private synchronized void onMessageHandled() { private synchronized void onMessageHandled() {
......
...@@ -29,6 +29,7 @@ import com.google.common.base.Objects; ...@@ -29,6 +29,7 @@ import com.google.common.base.Objects;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.robolectric.shadows.ShadowLooper; import org.robolectric.shadows.ShadowLooper;
...@@ -40,6 +41,7 @@ public final class FakeClockTest { ...@@ -40,6 +41,7 @@ public final class FakeClockTest {
@Test @Test
public void currentTimeMillis_withoutBootTime() { public void currentTimeMillis_withoutBootTime() {
FakeClock fakeClock = new FakeClock(/* initialTimeMs= */ 10); FakeClock fakeClock = new FakeClock(/* initialTimeMs= */ 10);
assertThat(fakeClock.currentTimeMillis()).isEqualTo(10); assertThat(fakeClock.currentTimeMillis()).isEqualTo(10);
} }
...@@ -48,6 +50,7 @@ public final class FakeClockTest { ...@@ -48,6 +50,7 @@ public final class FakeClockTest {
FakeClock fakeClock = FakeClock fakeClock =
new FakeClock( new FakeClock(
/* bootTimeMs= */ 150, /* initialTimeMs= */ 200, /* isAutoAdvancing= */ false); /* bootTimeMs= */ 150, /* initialTimeMs= */ 200, /* isAutoAdvancing= */ false);
assertThat(fakeClock.currentTimeMillis()).isEqualTo(350); assertThat(fakeClock.currentTimeMillis()).isEqualTo(350);
} }
...@@ -55,17 +58,24 @@ public final class FakeClockTest { ...@@ -55,17 +58,24 @@ public final class FakeClockTest {
public void currentTimeMillis_afterAdvanceTime_currentTimeHasAdvanced() { public void currentTimeMillis_afterAdvanceTime_currentTimeHasAdvanced() {
FakeClock fakeClock = FakeClock fakeClock =
new FakeClock(/* bootTimeMs= */ 100, /* initialTimeMs= */ 50, /* isAutoAdvancing= */ false); new FakeClock(/* bootTimeMs= */ 100, /* initialTimeMs= */ 50, /* isAutoAdvancing= */ false);
fakeClock.advanceTime(/* timeDiffMs */ 250); fakeClock.advanceTime(/* timeDiffMs */ 250);
assertThat(fakeClock.currentTimeMillis()).isEqualTo(400); assertThat(fakeClock.currentTimeMillis()).isEqualTo(400);
} }
@Test @Test
public void elapsedRealtime_afterAdvanceTime_timeHasAdvanced() { public void elapsedRealtime_afterAdvanceTime_timeHasAdvanced() {
FakeClock fakeClock = new FakeClock(2000); FakeClock fakeClock = new FakeClock(2000);
assertThat(fakeClock.elapsedRealtime()).isEqualTo(2000); assertThat(fakeClock.elapsedRealtime()).isEqualTo(2000);
fakeClock.advanceTime(500); fakeClock.advanceTime(500);
assertThat(fakeClock.elapsedRealtime()).isEqualTo(2500); assertThat(fakeClock.elapsedRealtime()).isEqualTo(2500);
fakeClock.advanceTime(0); fakeClock.advanceTime(0);
assertThat(fakeClock.elapsedRealtime()).isEqualTo(2500); assertThat(fakeClock.elapsedRealtime()).isEqualTo(2500);
} }
...@@ -86,6 +96,7 @@ public final class FakeClockTest { ...@@ -86,6 +96,7 @@ public final class FakeClockTest {
.sendToTarget(); .sendToTarget();
ShadowLooper.idleMainLooper(); ShadowLooper.idleMainLooper();
shadowOf(handler.getLooper()).idle(); shadowOf(handler.getLooper()).idle();
handlerThread.quitSafely();
assertThat(callback.messages) assertThat(callback.messages)
.containsExactly( .containsExactly(
...@@ -126,6 +137,7 @@ public final class FakeClockTest { ...@@ -126,6 +137,7 @@ public final class FakeClockTest {
fakeClock.advanceTime(50); fakeClock.advanceTime(50);
shadowOf(handler.getLooper()).idle(); shadowOf(handler.getLooper()).idle();
handlerThread.quitSafely();
assertThat(callback.messages).hasSize(4); assertThat(callback.messages).hasSize(4);
assertThat(Iterables.getLast(callback.messages)) assertThat(Iterables.getLast(callback.messages))
...@@ -146,6 +158,7 @@ public final class FakeClockTest { ...@@ -146,6 +158,7 @@ public final class FakeClockTest {
handler.obtainMessage(/* what= */ 4).sendToTarget(); handler.obtainMessage(/* what= */ 4).sendToTarget();
ShadowLooper.idleMainLooper(); ShadowLooper.idleMainLooper();
shadowOf(handler.getLooper()).idle(); shadowOf(handler.getLooper()).idle();
handlerThread.quitSafely();
assertThat(callback.messages) assertThat(callback.messages)
.containsExactly( .containsExactly(
...@@ -192,6 +205,8 @@ public final class FakeClockTest { ...@@ -192,6 +205,8 @@ public final class FakeClockTest {
fakeClock.advanceTime(1000); fakeClock.advanceTime(1000);
shadowOf(handler.getLooper()).idle(); shadowOf(handler.getLooper()).idle();
assertTestRunnableStates(new boolean[] {true, true, true, true, true}, testRunnables); assertTestRunnableStates(new boolean[] {true, true, true, true, true}, testRunnables);
handlerThread.quitSafely();
} }
@Test @Test
...@@ -203,7 +218,6 @@ public final class FakeClockTest { ...@@ -203,7 +218,6 @@ public final class FakeClockTest {
HandlerWrapper handler = fakeClock.createHandler(handlerThread.getLooper(), callback); HandlerWrapper handler = fakeClock.createHandler(handlerThread.getLooper(), callback);
TestCallback otherCallback = new TestCallback(); TestCallback otherCallback = new TestCallback();
HandlerWrapper otherHandler = fakeClock.createHandler(handlerThread.getLooper(), otherCallback); HandlerWrapper otherHandler = fakeClock.createHandler(handlerThread.getLooper(), otherCallback);
TestRunnable testRunnable1 = new TestRunnable(); TestRunnable testRunnable1 = new TestRunnable();
TestRunnable testRunnable2 = new TestRunnable(); TestRunnable testRunnable2 = new TestRunnable();
Object messageToken = new Object(); Object messageToken = new Object();
...@@ -216,10 +230,10 @@ public final class FakeClockTest { ...@@ -216,10 +230,10 @@ public final class FakeClockTest {
handler.removeMessages(/* what= */ 2); handler.removeMessages(/* what= */ 2);
handler.removeCallbacksAndMessages(messageToken); handler.removeCallbacksAndMessages(messageToken);
fakeClock.advanceTime(50); fakeClock.advanceTime(50);
ShadowLooper.idleMainLooper(); ShadowLooper.idleMainLooper();
shadowOf(handlerThread.getLooper()).idle(); shadowOf(handlerThread.getLooper()).idle();
handlerThread.quitSafely();
assertThat(callback.messages) assertThat(callback.messages)
.containsExactly( .containsExactly(
...@@ -242,7 +256,6 @@ public final class FakeClockTest { ...@@ -242,7 +256,6 @@ public final class FakeClockTest {
HandlerWrapper handler = fakeClock.createHandler(handlerThread.getLooper(), callback); HandlerWrapper handler = fakeClock.createHandler(handlerThread.getLooper(), callback);
TestCallback otherCallback = new TestCallback(); TestCallback otherCallback = new TestCallback();
HandlerWrapper otherHandler = fakeClock.createHandler(handlerThread.getLooper(), otherCallback); HandlerWrapper otherHandler = fakeClock.createHandler(handlerThread.getLooper(), otherCallback);
TestRunnable testRunnable1 = new TestRunnable(); TestRunnable testRunnable1 = new TestRunnable();
TestRunnable testRunnable2 = new TestRunnable(); TestRunnable testRunnable2 = new TestRunnable();
Object messageToken = new Object(); Object messageToken = new Object();
...@@ -254,15 +267,14 @@ public final class FakeClockTest { ...@@ -254,15 +267,14 @@ public final class FakeClockTest {
otherHandler.sendEmptyMessage(/* what= */ 1); otherHandler.sendEmptyMessage(/* what= */ 1);
handler.removeCallbacksAndMessages(/* token= */ null); handler.removeCallbacksAndMessages(/* token= */ null);
fakeClock.advanceTime(50); fakeClock.advanceTime(50);
ShadowLooper.idleMainLooper(); ShadowLooper.idleMainLooper();
shadowOf(handlerThread.getLooper()).idle(); shadowOf(handlerThread.getLooper()).idle();
handlerThread.quitSafely();
assertThat(callback.messages).isEmpty(); assertThat(callback.messages).isEmpty();
assertThat(testRunnable1.hasRun).isFalse(); assertThat(testRunnable1.hasRun).isFalse();
assertThat(testRunnable2.hasRun).isFalse(); assertThat(testRunnable2.hasRun).isFalse();
// Assert that message on other handler wasn't removed. // Assert that message on other handler wasn't removed.
assertThat(otherCallback.messages) assertThat(otherCallback.messages)
.containsExactly( .containsExactly(
...@@ -295,6 +307,7 @@ public final class FakeClockTest { ...@@ -295,6 +307,7 @@ public final class FakeClockTest {
}); });
ShadowLooper.idleMainLooper(); ShadowLooper.idleMainLooper();
shadowOf(handler.getLooper()).idle(); shadowOf(handler.getLooper()).idle();
handlerThread.quitSafely();
assertThat(clockTimes).containsExactly(0L, 20L, 50L, 70L, 100L).inOrder(); assertThat(clockTimes).containsExactly(0L, 20L, 50L, 70L, 100L).inOrder();
} }
...@@ -333,6 +346,8 @@ public final class FakeClockTest { ...@@ -333,6 +346,8 @@ public final class FakeClockTest {
}); });
ShadowLooper.idleMainLooper(); ShadowLooper.idleMainLooper();
messagesFinished.block(); messagesFinished.block();
handlerThread1.quitSafely();
handlerThread2.quitSafely();
assertThat(executionOrder).containsExactly(1, 2, 3, 4, 5, 6, 7, 8).inOrder(); assertThat(executionOrder).containsExactly(1, 2, 3, 4, 5, 6, 7, 8).inOrder();
} }
...@@ -368,10 +383,39 @@ public final class FakeClockTest { ...@@ -368,10 +383,39 @@ public final class FakeClockTest {
ShadowLooper.idleMainLooper(); ShadowLooper.idleMainLooper();
shadowOf(handler1.getLooper()).idle(); shadowOf(handler1.getLooper()).idle();
shadowOf(handler2.getLooper()).idle(); shadowOf(handler2.getLooper()).idle();
handlerThread1.quitSafely();
handlerThread2.quitSafely();
assertThat(executionOrder).containsExactly(1, 2, 3, 4).inOrder(); 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) { private static void assertTestRunnableStates(boolean[] states, TestRunnable[] testRunnables) {
for (int i = 0; i < testRunnables.length; i++) { for (int i = 0; i < testRunnables.length; i++) {
assertThat(testRunnables[i].hasRun).isEqualTo(states[i]); assertThat(testRunnables[i].hasRun).isEqualTo(states[i]);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment