Merge pull request #343 from jglick/StageStepExecution-isReady-JENKIN…

…S-33005 [JENKINS-33005] Hang in StageStepExecution.println
jenkinsci · Feb 18, 2016 · 5b2dd6f · 5b2dd6f
2 parents 541faf6 + b9bcd4a
commit 5b2dd6f
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 13 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -5,6 +5,7 @@ Only noting significant user changes, not internal code cleanups and minor bug f
 ## 1.14 (upcoming)
 
 * Simple `git` step now checks out a branch, not a detached head, for ease of committing to the workspace.
+* [JENKINS-33005](https://issues.jenkins-ci.org/browse/JENKINS-33005): hang running `stage` step which tries to cancel an earlier build that could not be loaded.
 
 ## 1.14-beta-1 (Feb 10 2016)
 

diff --git a/cps/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/cps/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java
@@ -416,14 +416,21 @@ protected void initializeStorage() throws IOException {
     @Override
     public void onLoad(FlowExecutionOwner owner) throws IOException {
         this.owner = owner;
-        initializeStorage();
         try {
-            if (!isComplete())
-                loadProgramAsync(getProgramDataFile());
-        } catch (IOException e) {
-            SettableFuture<CpsThreadGroup> p = SettableFuture.create();
-            programPromise = p;
-            loadProgramFailed(e, p);
+            initializeStorage();
+            try {
+                if (!isComplete()) {
+                    loadProgramAsync(getProgramDataFile());
+                }
+            } catch (IOException e) {
+                SettableFuture<CpsThreadGroup> p = SettableFuture.create();
+                programPromise = p;
+                loadProgramFailed(e, p);
+            }
+        } finally {
+            if (programPromise == null) {
+                programPromise = Futures.immediateFailedFuture(new IllegalStateException("completed or broken execution"));
+            }
         }
     }
 

diff --git a/cps/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsStepContext.java b/cps/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsStepContext.java
@@ -238,7 +238,7 @@ public String getDisplayName() {
             ListenableFuture<CpsThreadGroup> pp;
             CpsFlowExecution flowExecution = getFlowExecution();
             while ((pp = flowExecution.programPromise) == null) {
-                Thread.sleep(100); // TODO why is this occasionally not set right away?
+                Thread.sleep(100); // TODO does JENKINS-33005 remove the need for this?
             }
             try {
                 threadGroup = pp.get();

diff --git a/support/src/main/java/org/jenkinsci/plugins/workflow/support/steps/StageStepExecution.java b/support/src/main/java/org/jenkinsci/plugins/workflow/support/steps/StageStepExecution.java
@@ -11,7 +11,6 @@
 import hudson.model.listeners.RunListener;
 import java.io.File;
 import java.io.IOException;
-import java.io.Serializable;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -27,6 +26,7 @@
 import jenkins.model.Jenkins;
 import org.jenkinsci.plugins.workflow.actions.LabelAction;
 import org.jenkinsci.plugins.workflow.actions.StageAction;
+import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner;
 import org.jenkinsci.plugins.workflow.graph.FlowNode;
 import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
 import org.jenkinsci.plugins.workflow.steps.FlowInterruptedException;
@@ -222,18 +222,26 @@ private static void cleanUp(Job<?,?> job, String jobName) {
     }
 
     private static void println(StepContext context, String message) {
+        if (!context.isReady()) {
+            LOGGER.log(Level.FINE, "cannot print message ‘{0}’ to dead {1}", new Object[] {message, context});
+            return;
+        }
         try {
             context.get(TaskListener.class).getLogger().println(message);
         } catch (Exception x) {
-            LOGGER.log(WARNING, null, x);
+            LOGGER.log(WARNING, "failed to print message to dead " + context, x);
         }
     }
 
     // TODO record the stage it got to and display that
     private static void cancel(StepContext context, StepContext newer) throws IOException, InterruptedException {
-        println(context, "Canceled since " + newer.get(Run.class).getDisplayName() + " got here");
-        println(newer, "Canceling older " + context.get(Run.class).getDisplayName());
-        context.onFailure(new FlowInterruptedException(Result.NOT_BUILT, new CanceledCause(newer.get(Run.class))));
+        if (context.isReady() && newer.isReady()) {
+            println(context, "Canceled since " + newer.get(Run.class).getDisplayName() + " got here");
+            println(newer, "Canceling older " + context.get(Run.class).getDisplayName());
+            context.onFailure(new FlowInterruptedException(Result.NOT_BUILT, new CanceledCause(newer.get(Run.class))));
+        } else {
+            LOGGER.log(WARNING, "cannot cancel dead {0} or {1}", new Object[] {context, newer});
+        }
     }
 
     /**
@@ -296,6 +304,9 @@ void unblock(String message) {
     @Extension
     public static final class Listener extends RunListener<Run<?,?>> {
         @Override public void onCompleted(Run<?,?> r, TaskListener listener) {
+            if (!(r instanceof FlowExecutionOwner.Executable) || ((FlowExecutionOwner.Executable) r).asFlowExecutionOwner() == null) {
+                return;
+            }
             exit(r);
         }
     }