Fix JENKINS-39541 which is triggered by a very specific edge case wit…

…h finding the start node for a complex set of parallels
jenkinsci · Nov 23, 2016 · 21ada24 · 21ada24
1 parent 061f6df
commit 21ada24
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 8 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScanner.java b/src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScanner.java
@@ -327,7 +327,9 @@ ArrayDeque<ParallelBlockStart> leastCommonAncestor(@Nonnull final Set<FlowNode>
         }
 
         // Walk through, merging flownodes one-by-one until everything has merged to one ancestor
-        while (iterators.size() > 1) {
+        boolean mergedAll = false;
+		// Ends when we merged all branches together, or hit the start of the flow without it
+        while (!mergedAll && iterators.size() > 0) {
             ListIterator<Filterator<FlowNode>> itIterator = iterators.listIterator();
             ListIterator<FlowPiece> pieceIterator = livePieces.listIterator();
 
@@ -374,6 +376,9 @@ ArrayDeque<ParallelBlockStart> leastCommonAncestor(@Nonnull final Set<FlowNode>
                     // Merging removes the piece & its iterator from heads
                     itIterator.remove();
                     pieceIterator.remove();
+                    if (iterators.size() == 1) { // Merged in the final branch
+                        mergedAll = true;
+                    }
                 }
             }
         }
@@ -386,6 +391,7 @@ ArrayDeque<ParallelBlockStart> leastCommonAncestor(@Nonnull final Set<FlowNode>
     protected void setHeads(@Nonnull Collection<FlowNode> heads) {
         if (heads.size() > 1) {
             parallelBlockStartStack = leastCommonAncestor(new LinkedHashSet<FlowNode>(heads));
+            assert parallelBlockStartStack.size() > 0;
             currentParallelStart = parallelBlockStartStack.pop();
             currentParallelStartNode = currentParallelStart.forkStart;
             myCurrent = currentParallelStart.unvisited.pop();

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScannerTest.java b/src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScannerTest.java
@@ -457,20 +457,16 @@ public void testVariousParallelCombos() throws Exception {
         FlowExecution exec = b.getExecution();
         ForkScanner scan = new ForkScanner();
 
-        // Test different start points in branch A & B
+        // Test different start points in branch A & B, 20 and 19 were one error case.
         for (int i=0; i < 4; i++) {
             for (int j=0; j<5; j++) {
                 int branchANodeId = i+20;
                 int branchBNodeId = j+15;
+                System.out.println("Starting test with nodes "+branchANodeId+","+branchBNodeId);
                 ArrayList<FlowNode> starts = new ArrayList<FlowNode>();
                 FlowTestUtils.addNodesById(starts, exec, branchANodeId, branchBNodeId);
                 List<FlowNode> all = scan.filteredNodes(starts, Predicates.<FlowNode>alwaysTrue());
-                try {
-                    Assert.assertEquals(new HashSet<FlowNode>(all).size(), all.size());
-                } catch (Throwable t) {
-                    System.out.println("Error starting with nodes: "+branchANodeId+" "+branchBNodeId);
-                    throw t;
-                }
+                Assert.assertEquals(new HashSet<FlowNode>(all).size(), all.size());
                 scan.reset();
             }
         }