Allow reverse dependency lookups on Skyframe graph to throw InteruptedException

--
MOS_MIGRATED_REVID=132999234

5 files changed, 30 insertions, 31 deletions

diff --git a/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java b/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java
index b27d2ef6e7..49db9cdc0b 100644
--- a/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java
+++ b/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java
@@ -135,7 +135,7 @@ public class DeterministicHelper extends NotifyingHelper {
     }
 
     @Override
-    public synchronized Collection<SkyKey> getReverseDeps() {
+    public synchronized Collection<SkyKey> getReverseDeps() throws InterruptedException {
       TreeSet<SkyKey> result = new TreeSet<>(ALPHABETICAL_SKYKEY_COMPARATOR);
       Iterables.addAll(result, super.getReverseDeps());
       return result;
diff --git a/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java b/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java
index fa5a7dfaf7..649083581b 100644
--- a/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java
@@ -193,14 +193,14 @@ public abstract class GraphConcurrencyTest {
                     .isNotEqualTo(DependencyState.DONE);
                 waitForAddedRdep.countDown();
                 waitForSetValue.await(TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+                for (int k = chunkSize; k <= numIterations; k++) {
+                  entry.removeReverseDep(key("rdep" + j));
+                  entry.addReverseDepAndCheckIfDone(key("rdep" + j));
+                  entry.getReverseDeps();
+                }
               } catch (InterruptedException e) {
                 fail("Test failed: " + e.toString());
               }
-              for (int k = chunkSize; k <= numIterations; k++) {
-                entry.removeReverseDep(key("rdep" + j));
-                entry.addReverseDepAndCheckIfDone(key("rdep" + j));
-                entry.getReverseDeps();
-              }
             }
           };
       pool.execute(wrapper.wrap(r));
@@ -245,6 +245,7 @@ public abstract class GraphConcurrencyTest {
           final Iterable<SkyKey> keys = ImmutableList.of(key1, key2);
           Runnable r =
               new Runnable() {
+                @Override
                 public void run() {
                   for (SkyKey key : keys) {
                     NodeEntry entry = null;
@@ -268,15 +269,15 @@ public abstract class GraphConcurrencyTest {
                     NodeEntry entry = entries.get(key);
                     // {@code entry.addReverseDepAndCheckIfDone(null)} should return
                     // NEEDS_SCHEDULING at most once.
-                    if (startEvaluation(entry).equals(DependencyState.NEEDS_SCHEDULING)) {
-                      assertTrue(valuesSet.add(key));
-                      // Set to done.
-                      try {
+                    try {
+                      if (startEvaluation(entry).equals(DependencyState.NEEDS_SCHEDULING)) {
+                        assertTrue(valuesSet.add(key));
+                        // Set to done.
                         entry.setValue(new StringValue("bar" + keyNum), startingVersion);
-                      } catch (InterruptedException e) {
-                        throw new IllegalStateException(key + ", " + entry, e);
+                        assertThat(entry.isDone()).isTrue();
                       }
-                      assertThat(entry.isDone()).isTrue();
+                    } catch (InterruptedException e) {
+                      throw new IllegalStateException(key + ", " + entry, e);
                     }
                   }
                   // This shouldn't cause any problems from the other threads.
@@ -360,16 +361,13 @@ public abstract class GraphConcurrencyTest {
               }
               try {
                 entry.markDirty(true);
-              } catch (InterruptedException e) {
-                throw new IllegalStateException(keyNum + ", " + entry, e);
-              }
-              // Make some changes, like adding a dep and rdep.
-              entry.addReverseDepAndCheckIfDone(key("rdep"));
-              entry.markRebuilding();
-              addTemporaryDirectDep(entry, key("dep"));
-              entry.signalDep();
-              // Move node from dirty back to done.
-              try {
+
+                // Make some changes, like adding a dep and rdep.
+                entry.addReverseDepAndCheckIfDone(key("rdep"));
+                entry.markRebuilding();
+                addTemporaryDirectDep(entry, key("dep"));
+                entry.signalDep();
+
                 entry.setValue(new StringValue("bar" + keyNum), getNextVersion(startingVersion));
               } catch (InterruptedException e) {
                 throw new IllegalStateException(keyNum + ", " + entry, e);
@@ -461,7 +459,7 @@ public abstract class GraphConcurrencyTest {
     }
   }
 
-  private static DependencyState startEvaluation(NodeEntry entry) {
+  private static DependencyState startEvaluation(NodeEntry entry) throws InterruptedException {
     return entry.addReverseDepAndCheckIfDone(null);
   }
 
diff --git a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
index 00681c8d3b..9c19360762 100644
--- a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
@@ -56,7 +56,7 @@ public class InMemoryNodeEntryTest {
 
   @Test
   public void createEntry() {
-    NodeEntry entry = new InMemoryNodeEntry();
+    InMemoryNodeEntry entry = new InMemoryNodeEntry();
     entry.addReverseDepAndCheckIfDone(null); // Start evaluation.
     assertFalse(entry.isDone());
     assertTrue(entry.isReady());
@@ -148,7 +148,7 @@ public class InMemoryNodeEntryTest {
 
   @Test
   public void crashOnTooManySignals() {
-    NodeEntry entry = new InMemoryNodeEntry();
+    InMemoryNodeEntry entry = new InMemoryNodeEntry();
     entry.addReverseDepAndCheckIfDone(null); // Start evaluation.
     try {
       entry.signalDep();
diff --git a/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java b/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java
index 1b5911f166..744cb0672a 100644
--- a/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java
+++ b/src/test/java/com/google/devtools/build/skyframe/NodeEntrySubject.java
@@ -21,7 +21,6 @@ import com.google.common.truth.DefaultSubject;
 import com.google.common.truth.FailureStrategy;
 import com.google.common.truth.IterableSubject;
 import com.google.common.truth.Subject;
-
 import javax.annotation.Nullable;
 
 /**
@@ -43,7 +42,7 @@ class NodeEntrySubject extends Subject<NodeEntrySubject, NodeEntry> {
   }
 
   ComparableSubject<?, NodeEntry.DependencyState> addReverseDepAndCheckIfDone(
-      @Nullable SkyKey reverseDep) {
+      @Nullable SkyKey reverseDep) throws InterruptedException {
     return assertThat(getSubject().addReverseDepAndCheckIfDone(reverseDep))
         .named(detail("AddReverseDepAndCheckIfDone"));
   }
diff --git a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
index 5adbe45eb6..6cde57318a 100644
--- a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
+++ b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java
@@ -216,7 +216,8 @@ public class NotifyingHelper {
     }
 
     @Override
-    public DependencyState addReverseDepAndCheckIfDone(SkyKey reverseDep) {
+    public DependencyState addReverseDepAndCheckIfDone(SkyKey reverseDep)
+        throws InterruptedException {
       graphListener.accept(myKey, EventType.ADD_REVERSE_DEP, Order.BEFORE, reverseDep);
       DependencyState result = super.addReverseDepAndCheckIfDone(reverseDep);
       graphListener.accept(myKey, EventType.ADD_REVERSE_DEP, Order.AFTER, reverseDep);
@@ -224,7 +225,7 @@ public class NotifyingHelper {
     }
 
     @Override
-    public void removeReverseDep(SkyKey reverseDep) {
+    public void removeReverseDep(SkyKey reverseDep) throws InterruptedException {
       graphListener.accept(myKey, EventType.REMOVE_REVERSE_DEP, Order.BEFORE, reverseDep);
       super.removeReverseDep(reverseDep);
       graphListener.accept(myKey, EventType.REMOVE_REVERSE_DEP, Order.AFTER, reverseDep);
@@ -293,7 +294,8 @@ public class NotifyingHelper {
     }
 
     @Override
-    public DependencyState checkIfDoneForDirtyReverseDep(SkyKey reverseDep) {
+    public DependencyState checkIfDoneForDirtyReverseDep(SkyKey reverseDep)
+        throws InterruptedException {
       graphListener.accept(myKey, EventType.CHECK_IF_DONE, Order.BEFORE, reverseDep);
       return super.checkIfDoneForDirtyReverseDep(reverseDep);
     }