Don't use computeIfAbsent in action graph dumping.

computeIfAbsent may throw (on best effort basis) a
ConcurrentModificationException if you change the underlying Map in the
computeIfAbsent call, for example when you call computeIfAbsent recursively in
the mappingFunction.
NestedSets are an inherent recursive structure, so we need to call
computeIfAbsent recursively in the action graph dump.

Since computeIfAbsent doesn't throw the exception all the time, it's hard to
come up with a reliable test case.

RELNOTES: None
PiperOrigin-RevId: 188151283

1 files changed, 9 insertions, 4 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/BaseCache.java b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/BaseCache.java
index f2eb7b4cd7..f2b3d8697a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/BaseCache.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/BaseCache.java
@@ -39,12 +39,17 @@ abstract class BaseCache<K, P> {
 
   String dataToId(K data) {
     K key = transformToKey(data);
-    return cache.computeIfAbsent(key, k -> {
-      String id = generateNextId();
+    String id = cache.get(key);
+    if (id == null) {
+      // Note that this cannot be replaced by computeIfAbsent since createProto is a recursive
+      // operation for the case of nested sets which will call dataToId on the same object and thus
+      // computeIfAbsent again.
+      id = generateNextId();
+      cache.put(key, id);
       P proto = createProto(data, id);
       addToActionGraphBuilder(proto);
-      return id;
-    });
+    }
+    return id;
   }
 
   abstract P createProto(K key, String id);