Attempt to detect loops when iterating through siblings in FlowGraphT…

…able
jenkinsci · May 27, 2020 · cea1744 · cea1744
1 parent 7a0255b
commit cea1744
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 9 deletions.
diff --git a/Jenkinsfile b/Jenkinsfile
@@ -1 +1,5 @@
-buildPlugin(configurations: buildPlugin.recommendedConfigurations())
+buildPlugin(configurations: configurations: [
+  [ platform: "linux", jdk: "8", jenkins: null ],
+  [ platform: "windows", jdk: "8", jenkins: null ],
+  [ platform: "linux", jdk: "11", jenkins: "2.222.3" ]
+])
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/visualization/table/FlowGraphTable.java b/src/main/java/org/jenkinsci/plugins/workflow/support/visualization/table/FlowGraphTable.java
@@ -14,10 +14,12 @@
 import javax.annotation.Nullable;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.IdentityHashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Stack;
+import java.util.function.Function;
 
 /**
  * Data model behind the tree list view of a flow graph
@@ -294,6 +296,14 @@ public long getDurationMillis() {
             return this.durationMillis;
         }
 
+        private Row getNextGraphSibling() {
+            return nextGraphSibling;
+        }
+
+        private Row getNextTreeSibling() {
+            return nextTreeSibling;
+        }
+
         public String getDurationString() {
             if (!this.hasTiming) {
                 return "no timing";
@@ -327,10 +337,7 @@ void addGraphChild(Row r) {
         }
 
         void addGraphSibling(Row r) {
-            Row s = this;
-            while (s.nextGraphSibling !=null) {
-                s = s.nextGraphSibling;
-            }
+            Row s = findLastSibling(this, Row::getNextGraphSibling);
             s.nextGraphSibling = r;
 
             if (s.hasStartTime && r.hasStartTime) {
@@ -352,16 +359,26 @@ void addTreeChild(Row r) {
         void addTreeSibling(Row r) {
             if (r.isEnd())  return;
 
-            Row s = this;
-            while (s.nextTreeSibling !=null) {
-                s = s.nextTreeSibling;
-            }
+            Row s = findLastSibling(this, Row::getNextTreeSibling);
             s.nextTreeSibling = r;
 
             if (s.hasStartTime && r.hasStartTime) { // Store timing
                 s.durationMillis = (r.startTimeMillis-s.startTimeMillis);
                 s.hasTiming = true;
             }
         }
+
+        private Row findLastSibling(Row r, Function<Row, Row> siblingGetter) {
+            Row s = r;
+            IdentityHashMap<Row, Boolean> visited = new IdentityHashMap<>(Collections.singletonMap(s, true));
+            while (siblingGetter.apply(s) != null) {
+                Row nextS = siblingGetter.apply(s);
+                if (visited.put(nextS, true) != null) {
+                    throw new IllegalStateException("Saw " + nextS.node + " twice when finding siblings of " + r.node);
+                }
+                s = nextS;
+            }
+            return s;
+        }
     }
 }