ConditionVariable: Fix block(long) to correctly handle large timeouts

PiperOrigin-RevId: 308815613
This commit is contained in:
olly 2020-04-28 14:59:23 +01:00 committed by Oliver Woodman
parent be07b3cad5
commit 1c34029e87
3 changed files with 47 additions and 11 deletions

View File

@ -86,18 +86,27 @@ public class ConditionVariable {
}
/**
* Blocks until the condition is opened or until {@code timeout} milliseconds have passed.
* Blocks until the condition is opened or until {@code timeoutMs} have passed.
*
* @param timeout The maximum time to wait in milliseconds.
* @param timeoutMs The maximum time to wait in milliseconds. If {@code timeoutMs <= 0} then the
* call will return immediately without blocking.
* @return True if the condition was opened, false if the call returns because of the timeout.
* @throws InterruptedException If the thread is interrupted.
*/
public synchronized boolean block(long timeout) throws InterruptedException {
long now = clock.elapsedRealtime();
long end = now + timeout;
while (!isOpen && now < end) {
wait(end - now);
now = clock.elapsedRealtime();
public synchronized boolean block(long timeoutMs) throws InterruptedException {
if (timeoutMs <= 0) {
return isOpen;
}
long nowMs = clock.elapsedRealtime();
long endMs = nowMs + timeoutMs;
if (endMs < nowMs) {
// timeoutMs is large enough for (nowMs + timeoutMs) to rollover. Block indefinitely.
block();
} else {
while (!isOpen && nowMs < endMs) {
wait(endMs - nowMs);
nowMs = clock.elapsedRealtime();
}
}
return isOpen;
}

View File

@ -43,7 +43,7 @@ public class ConditionVariableTest {
public void blockWithTimeout_blocksForAtLeastTimeout() throws InterruptedException {
ConditionVariable conditionVariable = buildTestConditionVariable();
long startTimeMs = System.currentTimeMillis();
assertThat(conditionVariable.block(/* timeout= */ 500)).isFalse();
assertThat(conditionVariable.block(/* timeoutMs= */ 500)).isFalse();
long endTimeMs = System.currentTimeMillis();
assertThat(endTimeMs - startTimeMs).isAtLeast(500);
}
@ -75,6 +75,33 @@ public class ConditionVariableTest {
assertThat(conditionVariable.isOpen()).isFalse();
}
@Test
public void blockWithMaxTimeout_blocks() throws InterruptedException {
ConditionVariable conditionVariable = buildTestConditionVariable();
AtomicBoolean blockReturned = new AtomicBoolean();
AtomicBoolean blockWasInterrupted = new AtomicBoolean();
Thread blockingThread =
new Thread(
() -> {
try {
conditionVariable.block(/* timeoutMs= */ Long.MAX_VALUE);
blockReturned.set(true);
} catch (InterruptedException e) {
blockWasInterrupted.set(true);
}
});
blockingThread.start();
Thread.sleep(500);
assertThat(blockReturned.get()).isFalse();
blockingThread.interrupt();
blockingThread.join();
assertThat(blockWasInterrupted.get()).isTrue();
assertThat(conditionVariable.isOpen()).isFalse();
}
@Test
public void open_unblocksBlock() throws InterruptedException {
ConditionVariable conditionVariable = buildTestConditionVariable();

View File

@ -30,7 +30,7 @@ public class TestUtilTest {
public void createRobolectricConditionVariable_blockWithTimeout_timesOut()
throws InterruptedException {
ConditionVariable conditionVariable = TestUtil.createRobolectricConditionVariable();
assertThat(conditionVariable.block(/* timeout= */ 1)).isFalse();
assertThat(conditionVariable.block(/* timeoutMs= */ 1)).isFalse();
assertThat(conditionVariable.isOpen()).isFalse();
}
@ -39,7 +39,7 @@ public class TestUtilTest {
throws InterruptedException {
ConditionVariable conditionVariable = TestUtil.createRobolectricConditionVariable();
long startTimeMs = System.currentTimeMillis();
assertThat(conditionVariable.block(/* timeout= */ 500)).isFalse();
assertThat(conditionVariable.block(/* timeoutMs= */ 500)).isFalse();
long endTimeMs = System.currentTimeMillis();
assertThat(endTimeMs - startTimeMs).isAtLeast(500);
}