ConditionVariable: Add uninterruptible block

Sometimes it's useful to be able to block until something on some other thread
"really has finished". This will be needed for moving caching operations onto
the executor in Downloader implementations, since we need to guarantee that
Downloader.download doesn't return until it's no longer modifying the
underlying cache.

One solution to this is of course just to not interrupt the thread that's
blocking on the condition variable, but there are cases where you do need to do
this in case the thread is at some other point in its execution. This is true
for Downloader implementations, where the Download.download thread will also
be blocking on PriorityTaskManager.proceed. Arranging to conditionally
interrupt the thread based on where it's got to is probably possible, but seems
complicated and error prone.

Issue: #5978
PiperOrigin-RevId: 313152413
This commit is contained in:
olly 2020-05-26 10:04:03 +01:00 committed by Oliver Woodman
parent ee11d9d6fb
commit 03ea39b175
2 changed files with 80 additions and 29 deletions

View file

@ -111,6 +111,26 @@ public class ConditionVariable {
return isOpen;
}
/**
* Blocks until the condition is open. Unlike {@link #block}, this method will continue to block
* if the calling thread is interrupted. If the calling thread was interrupted then its {@link
* Thread#isInterrupted() interrupted status} will be set when the method returns.
*/
public synchronized void blockUninterruptible() {
boolean wasInterrupted = false;
while (!isOpen) {
try {
wait();
} catch (InterruptedException e) {
wasInterrupted = true;
}
}
if (wasInterrupted) {
// Restore the interrupted status.
Thread.currentThread().interrupt();
}
}
/** Returns whether the condition is opened. */
public synchronized boolean isOpen() {
return isOpen;

View file

@ -49,34 +49,7 @@ public class ConditionVariableTest {
}
@Test
public void blockWithoutTimeout_blocks() throws InterruptedException {
ConditionVariable conditionVariable = buildTestConditionVariable();
AtomicBoolean blockReturned = new AtomicBoolean();
AtomicBoolean blockWasInterrupted = new AtomicBoolean();
Thread blockingThread =
new Thread(
() -> {
try {
conditionVariable.block();
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 blockWithMaxTimeout_blocks() throws InterruptedException {
public void blockWithMaxTimeout_blocks_thenThrowsWhenInterrupted() throws InterruptedException {
ConditionVariable conditionVariable = buildTestConditionVariable();
AtomicBoolean blockReturned = new AtomicBoolean();
@ -103,7 +76,34 @@ public class ConditionVariableTest {
}
@Test
public void open_unblocksBlock() throws InterruptedException {
public void block_blocks_thenThrowsWhenInterrupted() throws InterruptedException {
ConditionVariable conditionVariable = buildTestConditionVariable();
AtomicBoolean blockReturned = new AtomicBoolean();
AtomicBoolean blockWasInterrupted = new AtomicBoolean();
Thread blockingThread =
new Thread(
() -> {
try {
conditionVariable.block();
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 block_blocks_thenReturnsWhenOpened() throws InterruptedException {
ConditionVariable conditionVariable = buildTestConditionVariable();
AtomicBoolean blockReturned = new AtomicBoolean();
@ -129,6 +129,37 @@ public class ConditionVariableTest {
assertThat(conditionVariable.isOpen()).isTrue();
}
@Test
public void blockUnterruptible_blocksIfInterrupted_thenUnblocksWhenOpened()
throws InterruptedException {
ConditionVariable conditionVariable = buildTestConditionVariable();
AtomicBoolean blockReturned = new AtomicBoolean();
AtomicBoolean interruptedStatusSet = new AtomicBoolean();
Thread blockingThread =
new Thread(
() -> {
conditionVariable.blockUninterruptible();
blockReturned.set(true);
interruptedStatusSet.set(Thread.currentThread().isInterrupted());
});
blockingThread.start();
Thread.sleep(500);
assertThat(blockReturned.get()).isFalse();
blockingThread.interrupt();
Thread.sleep(500);
// blockUninterruptible should still be blocked.
assertThat(blockReturned.get()).isFalse();
conditionVariable.open();
blockingThread.join();
// blockUninterruptible should have set the thread's interrupted status on exit.
assertThat(interruptedStatusSet.get()).isTrue();
assertThat(conditionVariable.isOpen()).isTrue();
}
private static ConditionVariable buildTestConditionVariable() {
return new ConditionVariable(
new SystemClock() {