Merge pull request jenkinsci#160 from bitwiseman/issue/JENKINS-65821

[JENKINS-65821] Fix race condition by using ConcurrentHashMap

src/main/java/org/jenkinsci/plugins/workflow/graph/StandardGraphLookupView.java

@@ -11,12 +11,12 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * Provides overall insight into the structure of a flow graph... but with limited visibility so we can change implementation.
- * Designed to work entirely on the basis of the {@link FlowNode#id} rather than the {@link FlowNode}s themselves.
+ * Designed to work entirely on the basis of the {@link FlowNode#getId()} rather than the {@link FlowNode}s themselves.
  */
 @SuppressFBWarnings(value = "ES_COMPARING_STRINGS_WITH_EQ", justification = "Can can use instance identity when comparing to a final constant")
 @Restricted(NoExternalUse.class)
@@ -25,10 +25,10 @@ public final class StandardGraphLookupView implements GraphLookupView, GraphList
     static final String INCOMPLETE = "";
 
     /** Map the blockStartNode to its endNode, to accellerate a range of operations */
-    HashMap<String, String> blockStartToEnd = new HashMap<>();
+    ConcurrentHashMap<String, String> blockStartToEnd = new ConcurrentHashMap<>();
 
     /** Map a node to its nearest enclosing block */
-    HashMap<String, String> nearestEnclosingBlock = new HashMap<>();
+    ConcurrentHashMap<String, String> nearestEnclosingBlock = new ConcurrentHashMap<>();
 
     public void clearCache() {
         blockStartToEnd.clear();
@@ -39,10 +39,11 @@ public void clearCache() {
     @Override
     public void onNewHead(@Nonnull FlowNode newHead) {
         if (newHead instanceof BlockEndNode) {
-            blockStartToEnd.put(((BlockEndNode)newHead).getStartNode().getId(), newHead.getId());
-            String overallEnclosing = nearestEnclosingBlock.get(((BlockEndNode) newHead).getStartNode().getId());
-            if (overallEnclosing != null) {
-                nearestEnclosingBlock.put(newHead.getId(), overallEnclosing);
+            String startNodeId = ((BlockEndNode)newHead).getStartNode().getId();
+            blockStartToEnd.put(startNodeId, newHead.getId());
+            String enclosingId = nearestEnclosingBlock.get(startNodeId);
+            if (enclosingId != null) {
+                nearestEnclosingBlock.put(newHead.getId(), enclosingId);
             }
         } else {
             if (newHead instanceof BlockStartNode) {
@@ -167,11 +168,9 @@ public BlockEndNode getEndNode(@Nonnull final BlockStartNode startNode) {
                 throw new RuntimeException(ioe);
             }
         } else {
-            BlockEndNode node = bruteForceScanForEnd(startNode);
-            if (node != null) {
-                blockStartToEnd.put(startNode.getId(), node.getId());
-            }
-            return node;
+            // returns the end node or null
+            // if this returns end node, it also adds start and end to blockStartToEnd
+            return bruteForceScanForEnd(startNode);
         }
     }
 
@@ -191,12 +190,8 @@ public BlockStartNode findEnclosingBlockStart(@Nonnull FlowNode node) {
             }
         }
 
-        BlockStartNode enclosing = bruteForceScanForEnclosingBlock(node);
-        if (enclosing != null) {
-            nearestEnclosingBlock.put(node.getId(), enclosing.getId());
-            return enclosing;
-        }
-        return null;
+        // when scan completes, enclosing is in the cache if it exists
+        return bruteForceScanForEnclosingBlock(node);
     }
 
     @Override