Merge pull request #304 from Vlatombe/computeNext-remove-side-effect

Do not unregister completed executions when iterating over `FlowExecutionList` to avoid unnecessary log warnings
jenkinsci · Aug 28, 2023 · d9baddd · d9baddd
2 parents a2ff520 + ca179a2
commit d9baddd
Showing 1 changed file with 25 additions and 9 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionList.java b/src/main/java/org/jenkinsci/plugins/workflow/flow/FlowExecutionList.java
@@ -39,7 +39,6 @@
 import java.util.logging.Logger;
 import org.jenkinsci.plugins.workflow.graph.FlowNode;
 import org.jenkinsci.plugins.workflow.graphanalysis.LinearBlockHoppingScanner;
-import org.jvnet.hudson.reactor.Milestone;
 
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.Beta;
@@ -80,7 +79,7 @@ public FlowExecutionList() {
      */
     @Override
     public Iterator<FlowExecution> iterator() {
-        return new AbstractIterator<FlowExecution>() {
+        return new AbstractIterator<>() {
             final Iterator<FlowExecutionOwner> base = runningTasks.iterator();
 
             @Override
@@ -89,9 +88,7 @@ protected FlowExecution computeNext() {
                     FlowExecutionOwner o = base.next();
                     try {
                         FlowExecution e = o.get();
-                        if (e.isComplete()) {
-                            unregister(o);
-                        } else {
+                        if (!e.isComplete()) {
                             return e;
                         }
                     } catch (Throwable e) {
@@ -208,13 +205,32 @@ public boolean isResumptionComplete() {
     public static class ItemListenerImpl extends ItemListener {
         @Override
         public void onLoaded() {
-            FlowExecutionList list = FlowExecutionList.get();
-            for (final FlowExecution e : list) {
-                // The call to FlowExecutionOwner.get in the implementation of iterator() is sufficent to load the Pipeline.
+            FlowExecutionList.get().resume();
+        }
+    }
+
+    private void resume() {
+        boolean needSave = false;
+        for (var it = runningTasks.iterator(); it.hasNext(); ) {
+            var o = it.next();
+            try {
+                FlowExecution e = o.get();
                 LOGGER.log(Level.FINE, "Eagerly loaded {0}", e);
+                if (e.isComplete()) {
+                    LOGGER.log(Level.FINE, "Unregistering completed " + o, e);
+                    it.remove();
+                    needSave = true;
+                }
+            } catch (IOException ex) {
+                LOGGER.log(Level.FINE, "Failed to load " + o + ". Unregistering", ex);
+                it.remove();
+                needSave = true;
             }
-            list.resumptionComplete = true;
         }
+        if (needSave) {
+            saveLater();
+        }
+        resumptionComplete = true;
     }
 
     /**