diff options
author | Googler <noreply@google.com> | 2017-06-01 23:23:16 +0200 |
---|---|---|
committer | László Csomor <laszlocsomor@google.com> | 2017-06-02 15:26:58 +0200 |
commit | 97fc108aeb0cf7e5ad5ce6ba67273bb49faa6333 (patch) | |
tree | f734a9ef4912b6e7d4be3a74581ee72582d7e939 /src/main/java/com/google/devtools/build/lib/concurrent | |
parent | 484e9750f4223a5750d62d9fa1844019147497d2 (diff) |
Fix a bug in ParallelVisitor which prevents visitation task from being interrupted eagerly
The original code swallows the InterruptedException, sets the interrupt bit and
stops new tasks from being submitted. However it does not actively send an
interrupt signal to all running and pending tasks already in the pool. It is
partly due to the misleading syntax of awaitTermination, which actually quitely
waits for all tasks to exit even if interruptWorkers is set to true. Both
errors are fixed in this change.
RELNOTES: None
PiperOrigin-RevId: 157762029
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/concurrent')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitor.java | 30 |
1 files changed, 15 insertions, 15 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitor.java b/src/main/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitor.java index f82ead9512..7c0809d431 100644 --- a/src/main/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitor.java +++ b/src/main/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitor.java @@ -201,6 +201,20 @@ public class AbstractQueueVisitor implements QuiescingExecutor { @Override public final void awaitQuiescence(boolean interruptWorkers) throws InterruptedException { + Throwables.propagateIfPossible(catastrophe); + try { + synchronized (zeroRemainingTasks) { + while (remainingTasks.get() != 0 && !jobsMustBeStopped) { + zeroRemainingTasks.wait(); + } + } + } catch (InterruptedException e) { + // Mark the visitor, so that it's known to be interrupted, and + // then break out of here, stop the worker threads and return ASAP, + // sending the interruption to the parent thread. + setInterrupted(); + } + awaitTermination(interruptWorkers); } @@ -431,26 +445,12 @@ public class AbstractQueueVisitor implements QuiescingExecutor { * any worker thread failed unexpectedly. */ protected final void awaitTermination(boolean interruptWorkers) throws InterruptedException { - Throwables.propagateIfPossible(catastrophe); - try { - synchronized (zeroRemainingTasks) { - while (remainingTasks.get() != 0 && !jobsMustBeStopped) { - zeroRemainingTasks.wait(); - } - } - } catch (InterruptedException e) { - // Mark the visitor, so that it's known to be interrupted, and - // then break out of here, stop the worker threads and return ASAP, - // sending the interruption to the parent thread. - setInterrupted(); - } - reallyAwaitTermination(interruptWorkers); if (isInterrupted()) { // Set interrupted bit on current thread so that callers can see that it was interrupted. Note // that if the thread was interrupted while awaiting termination, we might not hit this - // codepath, but then the current thread's interrupt bit is already set, so we are fine. + // code path, but then the current thread's interrupt bit is already set, so we are fine. Thread.currentThread().interrupt(); } // Throw the first unhandled (worker thread) exception in the main thread. We throw an unchecked |