From d5b163e8ffd1bf7f92f5f517e63765956f2cee56 Mon Sep 17 00:00:00 2001 From: shreyax Date: Fri, 17 May 2019 12:16:27 -0700 Subject: [PATCH] Address race condition in ParallelVisitor A task could potentially enqueue items and stop running by the time between checking if the processing queue is empty and the check for pending tasks. PiperOrigin-RevId: 248765394 --- .../devtools/build/lib/query2/ParallelVisitor.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/query2/ParallelVisitor.java b/src/main/java/com/google/devtools/build/lib/query2/ParallelVisitor.java index 2c4d4414415322..086f8961a04d92 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/ParallelVisitor.java +++ b/src/main/java/com/google/devtools/build/lib/query2/ParallelVisitor.java @@ -257,7 +257,7 @@ private void visitAndWaitForCompletion() throws QueryException, InterruptedExcep // 1. Errors (QueryException or InterruptedException) occurred and visitations should fail // fast. // 2. There is no pending visit in the queue and no pending task running. - while (!mustJobsBeStopped() && (!processingQueue.isEmpty() || getTaskCount() > 0)) { + while (!mustJobsBeStopped() && moreWorkToDo()) { // To achieve maximum efficiency, queue is drained in either of the following two // conditions: // @@ -292,6 +292,15 @@ private void visitAndWaitForCompletion() throws QueryException, InterruptedExcep awaitTerminationAndPropagateErrorsIfAny(); } + private boolean moreWorkToDo() { + // Note that we must check the task count first -- checking the processing queue first has the + // following race condition: + // (1) Check processing queue and observe that it is empty + // (2) A remaining task adds to the processing queue and shuts down + // (3) We check the task count and observe it is empty + return getTaskCount() > 0 || !processingQueue.isEmpty(); + } + private void awaitTerminationAndPropagateErrorsIfAny() throws QueryException, InterruptedException { try {