[FIXED JENKINS-33005] Make StageStepExecution defensively call isRead…

…y on foreign builds. Also make CpsFlowExecution.onLoad unconditionally set programPromise, in case CpsStepContext.getThreadGroupSynchronously is waiting for it.
jenkinsci · Feb 17, 2016 · 0b4c317 · 0b4c317
1 parent 2f0724b
commit 0b4c317
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 11 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;
@@ -222,18 +221,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()) {
+            context.onFailure(new FlowInterruptedException(Result.NOT_BUILT, new CanceledCause(newer.get(Run.class))));
+        } else {
+            LOGGER.log(WARNING, "cannot cancel dead {0}", context);
+        }
     }
 
     /**