Use TrackingAwaiter properly to track lost exceptions. Using the static method wasn't guaranteed to catch all bugs. Also convert to a singleton since there's no reason to have multiple instances.

--
MOS_MIGRATED_REVID=102158719

5 files changed, 332 insertions, 294 deletions

diff --git a/src/test/java/com/google/devtools/build/skyframe/ChainedFunction.java b/src/test/java/com/google/devtools/build/skyframe/ChainedFunction.java
index 4f87bb38ca..48325b0765 100644
--- a/src/test/java/com/google/devtools/build/skyframe/ChainedFunction.java
+++ b/src/test/java/com/google/devtools/build/skyframe/ChainedFunction.java
@@ -54,12 +54,12 @@ final class ChainedFunction implements SkyFunction {
         notifyStart.countDown();
       }
       if (waitToFinish != null) {
-        TrackingAwaiter.waitAndMaybeThrowInterrupt(waitToFinish,
-            key + " timed out waiting to finish");
+        TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+            waitToFinish, key + " timed out waiting to finish");
         if (waitForException) {
           SkyFunctionEnvironment skyEnv = (SkyFunctionEnvironment) env;
-          TrackingAwaiter.waitAndMaybeThrowInterrupt(skyEnv.getExceptionLatchForTesting(),
-              key + " timed out waiting for exception");
+          TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+              skyEnv.getExceptionLatchForTesting(), key + " timed out waiting for exception");
         }
       }
       for (SkyKey dep : deps) {
diff --git a/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java b/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java
index c0d349d025..aa9b036154 100644
--- a/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/EagerInvalidatorTest.java
@@ -40,6 +40,7 @@ import com.google.devtools.build.skyframe.InvalidatingNodeVisitor.DirtyingNodeVi
 import com.google.devtools.build.skyframe.InvalidatingNodeVisitor.InvalidationState;
 import com.google.devtools.build.skyframe.InvalidatingNodeVisitor.InvalidationType;
 
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.runners.Enclosed;
@@ -69,6 +70,11 @@ public class EagerInvalidatorTest {
 
   private IntVersion graphVersion = new IntVersion(0);
 
+  @After
+  public void assertNoTrackedErrors() {
+    TrackingAwaiter.INSTANCE.assertNoErrors();
+  }
+
   // The following three methods should be abstract, but junit4 does not allow us to run inner
   // classes in an abstract outer class. Thus, we provide implementations. These methods will never
   // be run because only the inner classes, annotated with @RunWith, will actually be executed.
@@ -466,37 +472,34 @@ public class EagerInvalidatorTest {
       }
       int countDownStart = validValuesToDo > 0 ? random.nextInt(validValuesToDo) : 0;
       final CountDownLatch countDownToInterrupt = new CountDownLatch(countDownStart);
-      final EvaluationProgressReceiver receiver = new EvaluationProgressReceiver() {
-        @Override
-        public void invalidated(SkyKey skyKey, InvalidationState state) {
-          countDownToInterrupt.countDown();
-          if (countDownToInterrupt.getCount() == 0) {
-            mainThread.interrupt();
-            try {
-              // Wait for the main thread to be interrupted uninterruptibly, because the main thread
-              // is going to interrupt us, and we don't want to get into an interrupt fight. Only
-              // if we get interrupted without the main thread also being interrupted will this
-              // throw an InterruptedException.
-              TrackingAwaiter.waitAndMaybeThrowInterrupt(
-                  visitor.get().getInterruptionLatchForTestingOnly(),
-                  "Main thread was not interrupted");
-            } catch (InterruptedException e) {
-              throw new IllegalStateException(e);
+      final EvaluationProgressReceiver receiver =
+          new EvaluationProgressReceiver() {
+            @Override
+            public void invalidated(SkyKey skyKey, InvalidationState state) {
+              countDownToInterrupt.countDown();
+              if (countDownToInterrupt.getCount() == 0) {
+                mainThread.interrupt();
+                // Wait for the main thread to be interrupted uninterruptibly, because the main
+                // thread is going to interrupt us, and we don't want to get into an interrupt
+                // fight. Only if we get interrupted without the main thread also being interrupted
+                // will this throw an InterruptedException.
+                TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                    visitor.get().getInterruptionLatchForTestingOnly(),
+                    "Main thread was not interrupted");
+              }
             }
-          }
-        }
 
-        @Override
-        public void enqueueing(SkyKey skyKey) {
-          throw new UnsupportedOperationException();
-        }
+            @Override
+            public void enqueueing(SkyKey skyKey) {
+              throw new UnsupportedOperationException();
+            }
 
-        @Override
-        public void evaluated(SkyKey skyKey, Supplier<SkyValue> skyValueSupplier,
-            EvaluationState state) {
-          throw new UnsupportedOperationException();
-        }
-      };
+            @Override
+            public void evaluated(
+                SkyKey skyKey, Supplier<SkyValue> skyValueSupplier, EvaluationState state) {
+              throw new UnsupportedOperationException();
+            }
+          };
       try {
         invalidate(graph, receiver,
             Sets.newHashSet(
diff --git a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
index 9199204349..c75af925a6 100644
--- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
@@ -55,6 +55,7 @@ import com.google.devtools.build.skyframe.NotifyingInMemoryGraph.Listener;
 import com.google.devtools.build.skyframe.NotifyingInMemoryGraph.Order;
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -96,6 +97,11 @@ public class MemoizingEvaluatorTest {
     initializeTester(null);
   }
 
+  @After
+  public void assertNoTrackedErrors() {
+    TrackingAwaiter.INSTANCE.assertNoErrors();
+  }
+
   public void initializeTester(@Nullable TrackingInvalidationReceiver customInvalidationReceiver) {
     emittedEventState = new MemoizingEvaluator.EmittedEventState();
     tester = new MemoizingEvaluatorTester();
@@ -544,34 +550,35 @@ public class MemoizingEvaluatorTest {
   public void alreadyAnalyzedBadTarget() throws Exception {
     final SkyKey mid = GraphTester.toSkyKey("mid");
     final CountDownLatch valueSet = new CountDownLatch(1);
-    final TrackingAwaiter trackingAwaiter = new TrackingAwaiter();
-    setGraphForTesting(new NotifyingInMemoryGraph(new Listener() {
-      @Override
-      public void accept(SkyKey key, EventType type, Order order, Object context) {
-        if (!key.equals(mid)) {
-          return;
-        }
-        switch (type) {
-          case ADD_REVERSE_DEP:
-            if (context == null) {
-              // Context is null when we are enqueuing this value as a top-level job.
-              trackingAwaiter.awaitLatchAndTrackExceptions(valueSet, "value not set");
-            }
-            break;
-          case SET_VALUE:
-            valueSet.countDown();
-            break;
-          default:
-            break;
-        }
-      }
-    }));
+    setGraphForTesting(
+        new NotifyingInMemoryGraph(
+            new Listener() {
+              @Override
+              public void accept(SkyKey key, EventType type, Order order, Object context) {
+                if (!key.equals(mid)) {
+                  return;
+                }
+                switch (type) {
+                  case ADD_REVERSE_DEP:
+                    if (context == null) {
+                      // Context is null when we are enqueuing this value as a top-level job.
+                      TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                          valueSet, "value not set");
+                    }
+                    break;
+                  case SET_VALUE:
+                    valueSet.countDown();
+                    break;
+                  default:
+                    break;
+                }
+              }
+            }));
     SkyKey top = GraphTester.skyKey("top");
     tester.getOrCreate(top).addDependency(mid).setComputedValue(CONCATENATE);
     tester.getOrCreate(mid).setHasError(true);
     tester.eval(/*keepGoing=*/false, top, mid);
     assertEquals(0L, valueSet.getCount());
-    trackingAwaiter.assertNoErrors();
     assertThat(tester.invalidationReceiver.evaluated).containsExactly(mid);
   }
 
@@ -892,22 +899,25 @@ public class MemoizingEvaluatorTest {
     final CountDownLatch errorThrown = new CountDownLatch(1);
     // We don't do anything on the first build.
     final AtomicBoolean secondBuild = new AtomicBoolean(false);
-    final TrackingAwaiter trackingAwaiter = new TrackingAwaiter();
-    setGraphForTesting(new DeterministicInMemoryGraph(new Listener() {
-      @Override
-      public void accept(SkyKey key, EventType type, Order order, Object context) {
-        if (!secondBuild.get()) {
-          return;
-        }
-        if (key.equals(otherTop) && type == EventType.SIGNAL) {
-          // otherTop is being signaled that dep1 is done. Tell the error value that it is ready,
-          // then wait until the error is thrown, so that otherTop's builder is not re-entered.
-          valuesReady.countDown();
-          trackingAwaiter.awaitLatchAndTrackExceptions(errorThrown, "error not thrown");
-          return;
-        }
-      }
-    }));
+    setGraphForTesting(
+        new DeterministicInMemoryGraph(
+            new Listener() {
+              @Override
+              public void accept(SkyKey key, EventType type, Order order, Object context) {
+                if (!secondBuild.get()) {
+                  return;
+                }
+                if (key.equals(otherTop) && type == EventType.SIGNAL) {
+                  // otherTop is being signaled that dep1 is done. Tell the error value that it is
+                  // ready, then wait until the error is thrown, so that otherTop's builder is not
+                  // re-entered.
+                  valuesReady.countDown();
+                  TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                      errorThrown, "error not thrown");
+                  return;
+                }
+              }
+            }));
     final SkyKey dep1 = GraphTester.toSkyKey("dep1");
     tester.set(dep1, new StringValue("dep1"));
     final SkyKey dep2 = GraphTester.toSkyKey("dep2");
@@ -957,7 +967,6 @@ public class MemoizingEvaluatorTest {
     // they appear here.
     EvaluationResult<StringValue> result =
         tester.eval(/*keepGoing=*/false, otherTop, topKey, exceptionMarker);
-    trackingAwaiter.assertNoErrors();
     assertThat(result.errorMap().keySet()).containsExactly(topKey);
     Iterable<CycleInfo> cycleInfos = result.getError(topKey).getCycleInfo();
     assertWithMessage(result.toString()).that(cycleInfos).isNotEmpty();
@@ -1453,29 +1462,32 @@ public class MemoizingEvaluatorTest {
     final AtomicBoolean delayTopSignaling = new AtomicBoolean(false);
     final CountDownLatch topSignaled = new CountDownLatch(1);
     final CountDownLatch topRestartedBuild = new CountDownLatch(1);
-    final TrackingAwaiter trackingAwaiter = new TrackingAwaiter();
-    setGraphForTesting(new DeterministicInMemoryGraph(new Listener() {
-      @Override
-      public void accept(SkyKey key, EventType type, Order order, Object context) {
-        if (!delayTopSignaling.get()) {
-          return;
-        }
-        if (key.equals(top) && type == EventType.SIGNAL && order == Order.AFTER) {
-          // top is signaled by firstKey (since slowAddingDep is blocking), so slowAddingDep is now
-          // free to acknowledge top as a parent.
-          topSignaled.countDown();
-          return;
-        }
-        if (key.equals(slowAddingDep) && type == EventType.ADD_REVERSE_DEP
-            && context.equals(top) && order == Order.BEFORE) {
-          // If top is trying to declare a dep on slowAddingDep, wait until firstKey has signaled
-          // top. Then this add dep will return DONE and top will be signaled, making it ready, so
-          // it will be enqueued.
-          trackingAwaiter.awaitLatchAndTrackExceptions(topSignaled,
-              "first key didn't signal top in time");
-        }
-      }
-    }));
+    setGraphForTesting(
+        new DeterministicInMemoryGraph(
+            new Listener() {
+              @Override
+              public void accept(SkyKey key, EventType type, Order order, Object context) {
+                if (!delayTopSignaling.get()) {
+                  return;
+                }
+                if (key.equals(top) && type == EventType.SIGNAL && order == Order.AFTER) {
+                  // top is signaled by firstKey (since slowAddingDep is blocking), so slowAddingDep
+                  // is now free to acknowledge top as a parent.
+                  topSignaled.countDown();
+                  return;
+                }
+                if (key.equals(slowAddingDep)
+                    && type == EventType.ADD_REVERSE_DEP
+                    && context.equals(top)
+                    && order == Order.BEFORE) {
+                  // If top is trying to declare a dep on slowAddingDep, wait until firstKey has
+                  // signaled top. Then this add dep will return DONE and top will be signaled,
+                  // making it ready, so it will be enqueued.
+                  TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                      topSignaled, "first key didn't signal top in time");
+                }
+              }
+            }));
     // Value that is modified on the second build. Its thread won't finish until it signals top,
     // which will wait for the signal before it enqueues its next dep. We prevent the thread from
     // finishing by having the listener to which it reports its warning block until top's builder
@@ -1498,18 +1510,19 @@ public class MemoizingEvaluatorTest {
         return env.valuesMissing() ? null : new StringValue("top");
       }
     });
-    reporter = new DelegatingEventHandler(reporter) {
-      @Override
-      public void handle(Event e) {
-        super.handle(e);
-        if (e.getKind() == EventKind.WARNING) {
-          if (!throwError) {
-            trackingAwaiter.awaitLatchAndTrackExceptions(topRestartedBuild,
-                "top's builder did not start in time");
+    reporter =
+        new DelegatingEventHandler(reporter) {
+          @Override
+          public void handle(Event e) {
+            super.handle(e);
+            if (e.getKind() == EventKind.WARNING) {
+              if (!throwError) {
+                TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                    topRestartedBuild, "top's builder did not start in time");
+              }
+            }
           }
-        }
-      }
-    };
+        };
     // First build : just prime the graph.
     EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/false, top);
     assertFalse(result.hasError());
@@ -1522,7 +1535,6 @@ public class MemoizingEvaluatorTest {
     tester.invalidate();
     delayTopSignaling.set(true);
     result = tester.eval(/*keepGoing=*/false, top);
-    trackingAwaiter.assertNoErrors();
     if (throwError) {
       assertTrue(result.hasError());
       assertThat(result.keyNames()).isEmpty(); // No successfully evaluated values.
@@ -1663,38 +1675,39 @@ public class MemoizingEvaluatorTest {
     // changed thread checks value entry once (to see if it is changed). dirty thread checks twice,
     // to see if it is changed, and if it is dirty.
     final CountDownLatch threadsStarted = new CountDownLatch(3);
-    final TrackingAwaiter trackingAwaiter = new TrackingAwaiter();
-    setGraphForTesting(new NotifyingInMemoryGraph(new Listener() {
-      @Override
-      public void accept(SkyKey key, EventType type, Order order, Object context) {
-        if (!blockingEnabled.get()) {
-          return;
-        }
-        if (!key.equals(parent)) {
-          return;
-        }
-        if (type == EventType.IS_CHANGED && order == Order.BEFORE) {
-          threadsStarted.countDown();
-        }
-        // Dirtiness only checked by dirty thread.
-        if (type == EventType.IS_DIRTY && order == Order.BEFORE) {
-          threadsStarted.countDown();
-        }
-        if (type == EventType.MARK_DIRTY) {
-          trackingAwaiter.awaitLatchAndTrackExceptions(threadsStarted,
-              "Both threads did not query if value isChanged in time");
-          boolean isChanged = (Boolean) context;
-          if (order == Order.BEFORE && !isChanged) {
-            trackingAwaiter.awaitLatchAndTrackExceptions(waitForChanged,
-                "'changed' thread did not mark value changed in time");
-            return;
-          }
-          if (order == Order.AFTER && isChanged) {
-            waitForChanged.countDown();
-          }
-        }
-      }
-    }));
+    setGraphForTesting(
+        new NotifyingInMemoryGraph(
+            new Listener() {
+              @Override
+              public void accept(SkyKey key, EventType type, Order order, Object context) {
+                if (!blockingEnabled.get()) {
+                  return;
+                }
+                if (!key.equals(parent)) {
+                  return;
+                }
+                if (type == EventType.IS_CHANGED && order == Order.BEFORE) {
+                  threadsStarted.countDown();
+                }
+                // Dirtiness only checked by dirty thread.
+                if (type == EventType.IS_DIRTY && order == Order.BEFORE) {
+                  threadsStarted.countDown();
+                }
+                if (type == EventType.MARK_DIRTY) {
+                  TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                      threadsStarted, "Both threads did not query if value isChanged in time");
+                  boolean isChanged = (Boolean) context;
+                  if (order == Order.BEFORE && !isChanged) {
+                    TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                        waitForChanged, "'changed' thread did not mark value changed in time");
+                    return;
+                  }
+                  if (order == Order.AFTER && isChanged) {
+                    waitForChanged.countDown();
+                  }
+                }
+              }
+            }));
     SkyKey leaf = GraphTester.toSkyKey("leaf");
     tester.set(leaf, new StringValue("leaf"));
     tester.getOrCreate(parent).addDependency(leaf).setComputedValue(CONCATENATE);
@@ -1712,7 +1725,6 @@ public class MemoizingEvaluatorTest {
     blockingEnabled.set(true);
     result = tester.eval(/*keepGoing=*/false, parent);
     assertEquals("leafother2", result.get(parent).getValue());
-    trackingAwaiter.assertNoErrors();
     assertEquals(0, waitForChanged.getCount());
     assertEquals(0, threadsStarted.getCount());
   }
@@ -2977,7 +2989,6 @@ public class MemoizingEvaluatorTest {
     // Keep track of any exceptions thrown during evaluation.
     final AtomicReference<Pair<SkyKey, ? extends Exception>> unexpectedException =
         new AtomicReference<>();
-    final TrackingAwaiter trackingAwaiter = new TrackingAwaiter();
     setGraphForTesting(
         new DeterministicInMemoryGraph(
             new Listener() {
@@ -2990,7 +3001,7 @@ public class MemoizingEvaluatorTest {
                     || type != EventType.SIGNAL) {
                   return;
                 }
-                trackingAwaiter.awaitLatchAndTrackExceptions(
+                TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
                     shutdownAwaiterStarted, "shutdown awaiter not started");
                 if (key.equals(uncachedParentKey)) {
                   // When the uncached parent is first signaled by its changed dep, make sure that
@@ -3062,15 +3073,10 @@ public class MemoizingEvaluatorTest {
             new SkyFunction() {
               @Override
               public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
-                try {
-                  shutdownAwaiterStarted.countDown();
-                  TrackingAwaiter.waitAndMaybeThrowInterrupt(
-                      ((ParallelEvaluator.SkyFunctionEnvironment) env)
-                          .getExceptionLatchForTesting(),
-                      "");
-                } catch (InterruptedException e) {
-                  unexpectedException.set(Pair.of(skyKey, e));
-                }
+                shutdownAwaiterStarted.countDown();
+                TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                    ((ParallelEvaluator.SkyFunctionEnvironment) env).getExceptionLatchForTesting(),
+                    "exception not thrown");
                 // Threadpool is shutting down. Don't try to synchronize anything in the future
                 // during error bubbling.
                 synchronizeThreads.set(false);
@@ -3089,7 +3095,6 @@ public class MemoizingEvaluatorTest {
       throw new AssertionError(unexpected.first + ", " + unexpected.second + ", "
           + Arrays.toString(unexpected.second.getStackTrace()));
     }
-    trackingAwaiter.assertNoErrors();
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
index 5d90a50e61..4ca65506f5 100644
--- a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
@@ -53,6 +53,7 @@ import com.google.devtools.build.skyframe.NotifyingInMemoryGraph.Listener;
 import com.google.devtools.build.skyframe.NotifyingInMemoryGraph.Order;
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -93,6 +94,11 @@ public class ParallelEvaluatorTest {
     reporter = new Reporter(eventCollector);
   }
 
+  @After
+  public void assertNoTrackedErrors() {
+    TrackingAwaiter.INSTANCE.assertNoErrors();
+  }
+
   private ParallelEvaluator makeEvaluator(ProcessableGraph graph,
       ImmutableMap<SkyFunctionName, ? extends SkyFunction> builders, boolean keepGoing,
       Predicate<Event> storedEventFilter) {
@@ -716,35 +722,38 @@ public class ParallelEvaluatorTest {
     final SkyKey midKey = GraphTester.toSkyKey("mid");
     SkyKey badKey = GraphTester.toSkyKey("bad");
     final AtomicBoolean waitForSecondCall = new AtomicBoolean(false);
-    final TrackingAwaiter trackingAwaiter = new TrackingAwaiter();
     final CountDownLatch otherThreadWinning = new CountDownLatch(1);
     final AtomicReference<Thread> firstThread = new AtomicReference<>();
-    graph = new NotifyingInMemoryGraph(new Listener() {
-      @Override
-      public void accept(SkyKey key, EventType type, Order order, Object context) {
-        if (!waitForSecondCall.get()) {
-          return;
-        }
-        if (key.equals(midKey)) {
-          if (type == EventType.CREATE_IF_ABSENT) {
-            // The first thread to create midKey will not be the first thread to add a reverse dep
-            // to it.
-            firstThread.compareAndSet(null, Thread.currentThread());
-            return;
-          }
-          if (type == EventType.ADD_REVERSE_DEP) {
-            if (order == Order.BEFORE && Thread.currentThread().equals(firstThread.get())) {
-              // If this thread created midKey, block until the other thread adds a dep on it.
-              trackingAwaiter.awaitLatchAndTrackExceptions(otherThreadWinning,
-                  "other thread didn't pass this one");
-            } else if (order == Order.AFTER && !Thread.currentThread().equals(firstThread.get())) {
-              // This thread has added a dep. Allow the other thread to proceed.
-              otherThreadWinning.countDown();
-            }
-          }
-        }
-      }
-    });
+    graph =
+        new NotifyingInMemoryGraph(
+            new Listener() {
+              @Override
+              public void accept(SkyKey key, EventType type, Order order, Object context) {
+                if (!waitForSecondCall.get()) {
+                  return;
+                }
+                if (key.equals(midKey)) {
+                  if (type == EventType.CREATE_IF_ABSENT) {
+                    // The first thread to create midKey will not be the first thread to add a
+                    // reverse dep to it.
+                    firstThread.compareAndSet(null, Thread.currentThread());
+                    return;
+                  }
+                  if (type == EventType.ADD_REVERSE_DEP) {
+                    if (order == Order.BEFORE && Thread.currentThread().equals(firstThread.get())) {
+                      // If this thread created midKey, block until the other thread adds a dep on
+                      // it.
+                      TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                          otherThreadWinning, "other thread didn't pass this one");
+                    } else if (order == Order.AFTER
+                        && !Thread.currentThread().equals(firstThread.get())) {
+                      // This thread has added a dep. Allow the other thread to proceed.
+                      otherThreadWinning.countDown();
+                    }
+                  }
+                }
+              }
+            });
     tester.getOrCreate(topKey).addDependency(midKey).setComputedValue(CONCATENATE);
     tester.getOrCreate(midKey).addDependency(badKey).setComputedValue(CONCATENATE);
     tester.getOrCreate(badKey).setHasError(true);
@@ -752,7 +761,6 @@ public class ParallelEvaluatorTest {
     assertThat(result.getError(midKey).getRootCauses()).containsExactly(badKey);
     waitForSecondCall.set(true);
     result = eval(/*keepGoing=*/true, topKey, midKey);
-    trackingAwaiter.assertNoErrors();
     assertNotNull(firstThread.get());
     assertEquals(0, otherThreadWinning.getCount());
     assertThat(result.getError(midKey).getRootCauses()).containsExactly(badKey);
@@ -2042,76 +2050,82 @@ public class ParallelEvaluatorTest {
     final SkyKey otherErrorKey = GraphTester.toSkyKey("otherErrorKey");
 
     final CountDownLatch errorCommitted = new CountDownLatch(1);
-    final TrackingAwaiter trackingAwaiterForErrorCommitted = new TrackingAwaiter();
 
     final CountDownLatch otherStarted = new CountDownLatch(1);
-    final TrackingAwaiter trackingAwaiterForOtherStarted = new TrackingAwaiter();
 
     final CountDownLatch otherDone = new CountDownLatch(1);
-    final TrackingAwaiter trackingAwaiterForOtherDone = new TrackingAwaiter();
 
     final AtomicInteger numOtherInvocations = new AtomicInteger(0);
     final AtomicReference<String> bogusInvocationMessage = new AtomicReference<>(null);
     final AtomicReference<String> nonNullValueMessage = new AtomicReference<>(null);
 
-    tester.getOrCreate(errorKey).setBuilder(new SkyFunction() {
-      @Override
-      public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException {
-        // Given that errorKey waits for otherErrorKey to begin evaluation before completing its
-        // evaluation,
-        trackingAwaiterForOtherStarted.awaitLatchAndTrackExceptions(otherStarted,
-            "otherErrorKey's SkyFunction didn't start in time.");
-        // And given that errorKey throws an error,
-        throw new GenericFunctionException(new SomeErrorException("error"), Transience.PERSISTENT);
-      }
-
-      @Override
-      public String extractTag(SkyKey skyKey) {
-        return null;
-      }
-    });
-    tester.getOrCreate(otherErrorKey).setBuilder(new SkyFunction() {
-      @Override
-      public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException {
-        otherStarted.countDown();
-        int invocations = numOtherInvocations.incrementAndGet();
-        // And given that otherErrorKey waits for errorKey's error to be committed before trying
-        // to get errorKey's value,
-        trackingAwaiterForErrorCommitted.awaitLatchAndTrackExceptions(errorCommitted,
-            "errorKey's error didn't get committed to the graph in time");
-        try {
-          SkyValue value = env.getValueOrThrow(errorKey, SomeErrorException.class);
-          if (value != null) {
-            nonNullValueMessage.set("bogus non-null value " + value);
-          }
-          if (invocations != 1) {
-            bogusInvocationMessage.set("bogus invocation count: " + invocations);
-          }
-          otherDone.countDown();
-          // And given that otherErrorKey throws an error,
-          throw new GenericFunctionException(new SomeErrorException("other"),
-              Transience.PERSISTENT);
-        } catch (SomeErrorException e) {
-          fail();
-          return null;
-        }
-      }
-
-      @Override
-      public String extractTag(SkyKey skyKey) {
-        return null;
-      }
-    });
-    graph = new NotifyingInMemoryGraph(new Listener() {
-      @Override
-      public void accept(SkyKey key, EventType type, Order order, Object context) {
-        if (key.equals(errorKey) && type == EventType.SET_VALUE && order == Order.AFTER) {
-          errorCommitted.countDown();
-          trackingAwaiterForOtherDone.awaitLatchAndTrackExceptions(otherDone,
-              "otherErrorKey's SkyFunction didn't finish in time.");
-        }
-      }
-    });
+    tester
+        .getOrCreate(errorKey)
+        .setBuilder(
+            new SkyFunction() {
+              @Override
+              public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException {
+                // Given that errorKey waits for otherErrorKey to begin evaluation before completing
+                // its evaluation,
+                TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                    otherStarted, "otherErrorKey's SkyFunction didn't start in time.");
+                // And given that errorKey throws an error,
+                throw new GenericFunctionException(
+                    new SomeErrorException("error"), Transience.PERSISTENT);
+              }
+
+              @Override
+              public String extractTag(SkyKey skyKey) {
+                return null;
+              }
+            });
+    tester
+        .getOrCreate(otherErrorKey)
+        .setBuilder(
+            new SkyFunction() {
+              @Override
+              public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException {
+                otherStarted.countDown();
+                int invocations = numOtherInvocations.incrementAndGet();
+                // And given that otherErrorKey waits for errorKey's error to be committed before
+                // trying to get errorKey's value,
+                TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                    errorCommitted, "errorKey's error didn't get committed to the graph in time");
+                try {
+                  SkyValue value = env.getValueOrThrow(errorKey, SomeErrorException.class);
+                  if (value != null) {
+                    nonNullValueMessage.set("bogus non-null value " + value);
+                  }
+                  if (invocations != 1) {
+                    bogusInvocationMessage.set("bogus invocation count: " + invocations);
+                  }
+                  otherDone.countDown();
+                  // And given that otherErrorKey throws an error,
+                  throw new GenericFunctionException(
+                      new SomeErrorException("other"), Transience.PERSISTENT);
+                } catch (SomeErrorException e) {
+                  fail();
+                  return null;
+                }
+              }
+
+              @Override
+              public String extractTag(SkyKey skyKey) {
+                return null;
+              }
+            });
+    graph =
+        new NotifyingInMemoryGraph(
+            new Listener() {
+              @Override
+              public void accept(SkyKey key, EventType type, Order order, Object context) {
+                if (key.equals(errorKey) && type == EventType.SET_VALUE && order == Order.AFTER) {
+                  errorCommitted.countDown();
+                  TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                      otherDone, "otherErrorKey's SkyFunction didn't finish in time.");
+                }
+              }
+            });
 
     // When the graph is evaluated in noKeepGoing mode,
     EvaluationResult<StringValue> result = eval(/*keepGoing=*/false,
@@ -2142,11 +2156,8 @@ public class ParallelEvaluatorTest {
   @Test
   public void raceConditionWithNoKeepGoingErrors_FutureError() throws Exception {
     final CountDownLatch errorCommitted = new CountDownLatch(1);
-    final TrackingAwaiter trackingAwaiterForError = new TrackingAwaiter();
     final CountDownLatch otherStarted = new CountDownLatch(1);
-    final TrackingAwaiter trackingAwaiterForOther = new TrackingAwaiter();
     final CountDownLatch otherParentSignaled = new CountDownLatch(1);
-    final TrackingAwaiter trackingAwaiterForOtherParent = new TrackingAwaiter();
     final SkyKey errorParentKey = GraphTester.toSkyKey("errorParentKey");
     final SkyKey errorKey = GraphTester.toSkyKey("errorKey");
     final SkyKey otherParentKey = GraphTester.toSkyKey("otherParentKey");
@@ -2166,34 +2177,40 @@ public class ParallelEvaluatorTest {
         return null;
       }
     });
-    tester.getOrCreate(otherKey).setBuilder(new SkyFunction() {
-      @Override
-      public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException {
-        otherStarted.countDown();
-        trackingAwaiterForError.awaitLatchAndTrackExceptions(errorCommitted,
-            "error didn't get committed to the graph in time");
-        return new StringValue("other");
-      }
-
-      @Override
-      public String extractTag(SkyKey skyKey) {
-        return null;
-      }
-    });
-    tester.getOrCreate(errorKey).setBuilder(new SkyFunction() {
-      @Override
-      public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException {
-        trackingAwaiterForOther.awaitLatchAndTrackExceptions(otherStarted,
-            "other didn't start in time");
-        throw new GenericFunctionException(new SomeErrorException("error"),
-            Transience.PERSISTENT);
-      }
-
-      @Override
-      public String extractTag(SkyKey skyKey) {
-        return null;
-      }
-    });
+    tester
+        .getOrCreate(otherKey)
+        .setBuilder(
+            new SkyFunction() {
+              @Override
+              public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException {
+                otherStarted.countDown();
+                TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                    errorCommitted, "error didn't get committed to the graph in time");
+                return new StringValue("other");
+              }
+
+              @Override
+              public String extractTag(SkyKey skyKey) {
+                return null;
+              }
+            });
+    tester
+        .getOrCreate(errorKey)
+        .setBuilder(
+            new SkyFunction() {
+              @Override
+              public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException {
+                TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                    otherStarted, "other didn't start in time");
+                throw new GenericFunctionException(
+                    new SomeErrorException("error"), Transience.PERSISTENT);
+              }
+
+              @Override
+              public String extractTag(SkyKey skyKey) {
+                return null;
+              }
+            });
     tester.getOrCreate(errorParentKey).setBuilder(new SkyFunction() {
       @Override
       public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException {
@@ -2221,26 +2238,31 @@ public class ParallelEvaluatorTest {
         return null;
       }
     });
-    graph = new NotifyingInMemoryGraph(new Listener() {
-      @Override
-      public void accept(SkyKey key, EventType type, Order order, Object context) {
-        if (key.equals(errorKey) && type == EventType.SET_VALUE && order == Order.AFTER) {
-          errorCommitted.countDown();
-          trackingAwaiterForOtherParent.awaitLatchAndTrackExceptions(otherParentSignaled,
-              "otherParent didn't get signaled in time");
-          // We try to give some time for ParallelEvaluator to incorrectly re-evaluate
-          // 'otherParentKey'. This test case is testing for a real race condition and the 10ms time
-          // was chosen experimentally to give a true positive rate of 99.8% (without a sleep it
-          // has a 1% true positive rate). There's no good way to do this without sleeping. We
-          // *could* introspect ParallelEvaulator's AbstractQueueVisitor to see if the re-evaluation
-          // has been enqueued, but that's relying on pretty low-level implementation details.
-          Uninterruptibles.sleepUninterruptibly(10, TimeUnit.MILLISECONDS);
-        }
-        if (key.equals(otherParentKey) && type == EventType.SIGNAL && order == Order.AFTER) {
-          otherParentSignaled.countDown();
-        }
-      }
-    });
+    graph =
+        new NotifyingInMemoryGraph(
+            new Listener() {
+              @Override
+              public void accept(SkyKey key, EventType type, Order order, Object context) {
+                if (key.equals(errorKey) && type == EventType.SET_VALUE && order == Order.AFTER) {
+                  errorCommitted.countDown();
+                  TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions(
+                      otherParentSignaled, "otherParent didn't get signaled in time");
+                  // We try to give some time for ParallelEvaluator to incorrectly re-evaluate
+                  // 'otherParentKey'. This test case is testing for a real race condition and the
+                  // 10ms time was chosen experimentally to give a true positive rate of 99.8%
+                  // (without a sleep it has a 1% true positive rate). There's no good way to do
+                  // this without sleeping. We *could* introspect ParallelEvaulator's
+                  // AbstractQueueVisitor to see if the re-evaluation has been enqueued, but that's
+                  // relying on pretty low-level implementation details.
+                  Uninterruptibles.sleepUninterruptibly(10, TimeUnit.MILLISECONDS);
+                }
+                if (key.equals(otherParentKey)
+                    && type == EventType.SIGNAL
+                    && order == Order.AFTER) {
+                  otherParentSignaled.countDown();
+                }
+              }
+            });
     EvaluationResult<StringValue> result = eval(/*keepGoing=*/false,
         ImmutableList.of(otherParentKey, errorParentKey));
     assertTrue(result.hasError());
diff --git a/src/test/java/com/google/devtools/build/skyframe/TrackingAwaiter.java b/src/test/java/com/google/devtools/build/skyframe/TrackingAwaiter.java
index 3757583ccb..687ed9a1d9 100644
--- a/src/test/java/com/google/devtools/build/skyframe/TrackingAwaiter.java
+++ b/src/test/java/com/google/devtools/build/skyframe/TrackingAwaiter.java
@@ -24,8 +24,16 @@ import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
-/** Safely await {@link CountDownLatch}es in tests, storing any exceptions that happen. */
+/**
+ * Safely await {@link CountDownLatch}es in tests, storing any exceptions that happen. Callers
+ * should call {@link #assertNoErrors} at the end of each test method, either manually or using an
+ * {@code @After} hook.
+ */
 public class TrackingAwaiter {
+  public static final TrackingAwaiter INSTANCE = new TrackingAwaiter();
+
+  private TrackingAwaiter() {}
+
   private final ConcurrentLinkedQueue<Pair<String, Throwable>> exceptionsThrown =
       new ConcurrentLinkedQueue<>();
 
@@ -42,7 +50,7 @@ public class TrackingAwaiter {
    * this was not a race condition, but an honest-to-goodness interrupt, and we propagate the
    * exception onward.
    */
-  public static void waitAndMaybeThrowInterrupt(CountDownLatch latch, String errorMessage)
+  private static void waitAndMaybeThrowInterrupt(CountDownLatch latch, String errorMessage)
       throws InterruptedException {
     if (Uninterruptibles.awaitUninterruptibly(latch, TestUtils.WAIT_TIMEOUT_SECONDS,
         TimeUnit.SECONDS)) {