Permit marking dirty/changed a node more than once

This functionality will be useful for node restarting. More than one
parent node may request the restart of a shared child node, and this
should not fail.

Instead, the returned MarkedDirtyResult indicates whether the dirtying
was redundant, and the calling code can assert on that.

RELNOTES: None.
PiperOrigin-RevId: 201005663

1 files changed, 13 insertions, 5 deletions

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 ceae771e88..9d2d811594 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InvalidatingNodeVisitor.java
@@ -207,7 +207,7 @@ public abstract class InvalidatingNodeVisitor<TGraph extends QueryableGraph> {
     }
   }
 
-  public static class DirtyingInvalidationState extends InvalidationState {
+  static class DirtyingInvalidationState extends InvalidationState {
     public DirtyingInvalidationState() {
       super(InvalidationType.CHANGED);
     }
@@ -477,7 +477,7 @@ public abstract class InvalidatingNodeVisitor<TGraph extends QueryableGraph> {
                 // method.
                 // Any exception thrown should be unrecoverable.
                 // This entry remains in the graph in this dirty state until it is re-evaluated.
-                MarkedDirtyResult markedDirtyResult = null;
+                MarkedDirtyResult markedDirtyResult;
                 try {
                   markedDirtyResult = entry.markDirty(isChanged);
                 } catch (InterruptedException e) {
@@ -487,8 +487,13 @@ public abstract class InvalidatingNodeVisitor<TGraph extends QueryableGraph> {
                   // visitation, so we can resume next time.
                   return;
                 }
-                if (markedDirtyResult == null) {
-                  // Another thread has already dirtied this node. Don't do anything in this thread.
+                Preconditions.checkState(
+                    !markedDirtyResult.wasCallRedundant(),
+                    "Node unexpectedly marked dirty or changed twice: %s",
+                    entry);
+                if (!markedDirtyResult.wasClean()) {
+                  // Another thread has already handled this node's rdeps. Don't do anything in this
+                  // thread.
                   if (supportInterruptions) {
                     pendingVisitations.remove(Pair.of(key, invalidationType));
                   }
@@ -496,7 +501,10 @@ public abstract class InvalidatingNodeVisitor<TGraph extends QueryableGraph> {
                 }
                 // Propagate dirtiness upwards and mark this node dirty/changed. Reverse deps should
                 // only be marked dirty (because only a dependency of theirs has changed).
-                visit(markedDirtyResult.getReverseDepsUnsafe(), InvalidationType.DIRTIED, key);
+                visit(
+                    markedDirtyResult.getReverseDepsUnsafeIfWasClean(),
+                    InvalidationType.DIRTIED,
+                    key);
 
                 progressReceiver.invalidated(key,
                     EvaluationProgressReceiver.InvalidationState.DIRTY);