Allow reverse dependency lookups on Skyframe graph to throw InteruptedException

--
MOS_MIGRATED_REVID=132999234

4 files changed, 43 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
index 6bf77ec4e0..47a9e616dd 100644
--- a/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/DelegatingNodeEntry.java
@@ -64,12 +64,14 @@ public abstract class DelegatingNodeEntry implements NodeEntry {
   }
 
   @Override
-  public DependencyState addReverseDepAndCheckIfDone(@Nullable SkyKey reverseDep) {
+  public DependencyState addReverseDepAndCheckIfDone(@Nullable SkyKey reverseDep)
+      throws InterruptedException {
     return getDelegate().addReverseDepAndCheckIfDone(reverseDep);
   }
 
   @Override
-  public DependencyState checkIfDoneForDirtyReverseDep(SkyKey reverseDep) {
+  public DependencyState checkIfDoneForDirtyReverseDep(SkyKey reverseDep)
+      throws InterruptedException {
     return getDelegate().checkIfDoneForDirtyReverseDep(reverseDep);
   }
 
@@ -159,7 +161,7 @@ public abstract class DelegatingNodeEntry implements NodeEntry {
   }
 
   @Override
-  public void removeReverseDep(SkyKey reverseDep) {
+  public void removeReverseDep(SkyKey reverseDep) throws InterruptedException {
     getDelegate().removeReverseDep(reverseDep);
   }
 
@@ -169,7 +171,7 @@ public abstract class DelegatingNodeEntry implements NodeEntry {
   }
 
   @Override
-  public Iterable<SkyKey> getReverseDeps() {
+  public Iterable<SkyKey> getReverseDeps() throws InterruptedException {
     return getDelegate().getReverseDeps();
   }
 
diff --git a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
index 096fc2d49b..8af633d2c4 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
@@ -282,7 +282,16 @@ public abstract class InvalidatingNodeVisitor<TGraph extends QueryableGraph> {
 
                 if (traverseGraph) {
                   // Propagate deletion upwards.
-                  visit(entry.getReverseDeps(), InvalidationType.DELETED);
+                  try {
+                    visit(entry.getReverseDeps(), InvalidationType.DELETED);
+                  } catch (InterruptedException e) {
+                    throw new IllegalStateException(
+                        "Deletion cannot happen on a graph that may have blocking operations: "
+                            + key
+                            + ", "
+                            + entry,
+                        e);
+                  }
 
                   // Unregister this node as an rdep from its direct deps, since reverse dep
                   // edges cannot point to non-existent nodes. To know whether the child has this
@@ -318,7 +327,17 @@ public abstract class InvalidatingNodeVisitor<TGraph extends QueryableGraph> {
                     NodeEntry dep = directDepEntry.getValue();
                     if (dep != null) {
                       if (dep.isDone() || !signalingDeps.contains(directDepEntry.getKey())) {
-                        dep.removeReverseDep(key);
+                        try {
+                          dep.removeReverseDep(key);
+                        } catch (InterruptedException e) {
+                          throw new IllegalStateException(
+                              "Deletion cannot happen on a graph that may have blocking "
+                                  + "operations: "
+                                  + key
+                                  + ", "
+                                  + entry,
+                              e);
+                        }
                       } else {
                         // This step is not strictly necessary, since all in-progress nodes are
                         // deleted during graph cleaning, which happens in a single
diff --git a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
index 82df6fa2ff..ee75ac3cf1 100644
--- a/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/NodeEntry.java
@@ -98,7 +98,7 @@ public interface NodeEntry extends ThinNodeEntry {
 
   /** Removes a reverse dependency. */
   @ThreadSafe
-  void removeReverseDep(SkyKey reverseDep);
+  void removeReverseDep(SkyKey reverseDep) throws InterruptedException;
 
   /**
    * Removes a reverse dependency.
@@ -114,7 +114,7 @@ public interface NodeEntry extends ThinNodeEntry {
    * check-then-act race; {@link #removeReverseDep} may fail for a key that is returned here.
    */
   @ThreadSafe
-  Iterable<SkyKey> getReverseDeps();
+  Iterable<SkyKey> getReverseDeps() throws InterruptedException;
 
   /**
    * Returns raw {@link SkyValue} stored in this entry, which may include metadata associated with
@@ -166,9 +166,9 @@ public interface NodeEntry extends ThinNodeEntry {
 
   /**
    * Queries if the node is done and adds the given key as a reverse dependency. The return code
-   * indicates whether a) the node is done, b) the reverse dependency is the first one, so the
-   * node needs to be scheduled, or c) the reverse dependency was added, and the node does not
-   * need to be scheduled.
+   * indicates whether a) the node is done, b) the reverse dependency is the first one, so the node
+   * needs to be scheduled, or c) the reverse dependency was added, and the node does not need to be
+   * scheduled.
    *
    * <p>This method <b>must</b> be called before any processing of the entry. This encourages
    * callers to check that the entry is ready to be processed.
@@ -179,14 +179,15 @@ public interface NodeEntry extends ThinNodeEntry {
    * However, a may complete first, in which case b has to re-schedule itself. Also see {@link
    * #setValue}.
    *
-   * <p>If the parameter is {@code null}, then no reverse dependency is added, but we still check
-   * if the node needs to be scheduled.
+   * <p>If the parameter is {@code null}, then no reverse dependency is added, but we still check if
+   * the node needs to be scheduled.
    *
    * <p>If {@code reverseDep} is a rebuilding dirty entry that was already a reverse dep of this
    * entry, then {@link #checkIfDoneForDirtyReverseDep} must be called instead.
    */
   @ThreadSafe
-  DependencyState addReverseDepAndCheckIfDone(@Nullable SkyKey reverseDep);
+  DependencyState addReverseDepAndCheckIfDone(@Nullable SkyKey reverseDep)
+      throws InterruptedException;
 
   /**
    * Similar to {@link #addReverseDepAndCheckIfDone}, except that {@param reverseDep} must already
@@ -196,7 +197,7 @@ public interface NodeEntry extends ThinNodeEntry {
    * node for evaluation if needed.
    */
   @ThreadSafe
-  DependencyState checkIfDoneForDirtyReverseDep(SkyKey reverseDep);
+  DependencyState checkIfDoneForDirtyReverseDep(SkyKey reverseDep) throws InterruptedException;
 
   /**
    * Tell this node that one of its dependencies is now done. Callers must check the return value,
diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
index aee81ead3b..8cfe1f6e59 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
@@ -207,7 +207,8 @@ public final class ParallelEvaluator implements Evaluator {
         NodeEntry entry,
         SkyKey child,
         NodeEntry childEntry,
-        boolean depAlreadyExists) {
+        boolean depAlreadyExists)
+        throws InterruptedException {
       Preconditions.checkState(!entry.isDone(), "%s %s", skyKey, entry);
       DependencyState dependencyState =
           depAlreadyExists
@@ -595,13 +596,16 @@ public final class ParallelEvaluator implements Evaluator {
    * its requested deps are done. However, that means we're assuming the SkyFunction would throw
    * that same error if all of its requested deps were done. Unfortunately, there is no way to
    * enforce that condition.
+   *
+   * @throws InterruptedException
    */
   private static void registerNewlyDiscoveredDepsForDoneEntry(
       SkyKey skyKey,
       NodeEntry entry,
       Map<SkyKey, ? extends NodeEntry> newlyRequestedDepMap,
       Set<SkyKey> oldDeps,
-      SkyFunctionEnvironment env) {
+      SkyFunctionEnvironment env)
+      throws InterruptedException {
     Set<SkyKey> unfinishedDeps = new HashSet<>();
     for (SkyKey dep : env.getNewlyRequestedDeps()) {
       if (!isDoneForBuild(newlyRequestedDepMap.get(dep))) {