Cleanup MarkedDirtyResult docs and constructor

--
MOS_MIGRATED_REVID=108069029

1 files changed, 10 insertions, 3 deletions

diff --git a/src/main/java/com/google/devtools/build/skyframe/ThinNodeEntry.java b/src/main/java/com/google/devtools/build/skyframe/ThinNodeEntry.java
index 55ec6cb493..0d91c03d1a 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ThinNodeEntry.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ThinNodeEntry.java
@@ -13,13 +13,14 @@
 // limitations under the License.
 package com.google.devtools.build.skyframe;
 
+import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 
 import javax.annotation.Nullable;
 
 /**
  * A node in the graph without the means to access its value. All operations on this class are
- * thread-safe.
+ * thread-safe. Note, however, the warning on the return value of {@link #markDirty}.
  *
  * <p>This interface is public only for the benefit of alternative graph implementations outside of
  * the package.
@@ -91,7 +92,8 @@ public interface ThinNodeEntry {
    *
    * @return A {@link ThinNodeEntry.MarkedDirtyResult} if the node was previously clean, and
    * {@code null} if it was already dirty. If it was already dirty, the caller should abort its
-   * handling of this node, since another thread is already dealing with it.
+   * handling of this node, since another thread is already dealing with it. Note the warning on
+   * {@link ThinNodeEntry.MarkedDirtyResult} regarding the collection it provides.
    */
   @Nullable
   @ThreadSafe
@@ -102,12 +104,17 @@ public interface ThinNodeEntry {
    * iterable of the node's reverse deps for efficiency, because the single use case for {@link
    * #markDirty} is during invalidation, and if such an invalidation call wins, the invalidator
    * must immediately afterwards schedule the invalidation of the node's reverse deps.
+   *
+   * <p>Warning: {@link #getReverseDepsUnsafe()} may return a live view of the reverse deps
+   * collection of the marked-dirty node. The consumer of this data must be careful only to
+   * iterate over and consume its values while that collection is guaranteed not to change. This
+   * is true during invalidation, because reverse deps don't change during invalidation.
    */
   class MarkedDirtyResult {
     private final Iterable<SkyKey> reverseDepsUnsafe;
 
     public MarkedDirtyResult(Iterable<SkyKey> reverseDepsUnsafe) {
-      this.reverseDepsUnsafe = reverseDepsUnsafe;
+      this.reverseDepsUnsafe = Preconditions.checkNotNull(reverseDepsUnsafe);
     }
 
     public Iterable<SkyKey> getReverseDepsUnsafe() {