Merge pull request #65 from jglick/isReady-JENKINS-25890

jglick · jglick · commit 8740a01d48c3 · 2015-03-03T09:11:30.000-05:00
[JENKINS-25890] Deadlock
diff --git a/CHANGES.md b/CHANGES.md
@@ -5,6 +5,7 @@ Only noting significant user-visible or major API changes, not internal code cle
 ## 1.3 (upcoming)
 
 ### User changes
+* JENKINS-25890: deadlock during restart.
 * Fixed some file handle leaks caught by tests which may have affected Windows masters.
 * JENKINS-25779: snippet generator now omits default values of complex steps.
 * Ability to configure project display name.
diff --git a/aggregator/src/test/java/org/jenkinsci/plugins/workflow/WorkflowTest.java b/aggregator/src/test/java/org/jenkinsci/plugins/workflow/WorkflowTest.java
@@ -650,7 +650,6 @@ public static void finish(final boolean terminate) {
         });
     }
 
-    @RandomlyFails("TODO JENKINS-25890 sometimes triggers a deadlock after restart")
     @Issue("JENKINS-26513")
     @Test public void executorStepRestart() {
         story.addStep(new Statement() {
diff --git a/cps/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsBodySubContext.java b/cps/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsBodySubContext.java
@@ -74,7 +74,7 @@ public void setResult(Result r) {
     }
 
     @Override
-    public boolean isReady() throws IOException, InterruptedException {
+    public boolean isReady() {
         return base.isReady();
     }
 
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
@@ -59,6 +59,7 @@
 import java.util.concurrent.ExecutionException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import jenkins.util.Timer;
 import org.codehaus.groovy.runtime.InvokerInvocationException;
 
 import static org.jenkinsci.plugins.workflow.cps.persistence.PersistenceContext.*;
@@ -215,7 +216,7 @@ public String getDisplayName() {
      *
      * This can block for the entire duration of the PREPARING state.
      */
-    @CheckForNull CpsThread getThreadSynchronously() throws InterruptedException, IOException {
+    private @CheckForNull CpsThread getThreadSynchronously() throws InterruptedException, IOException {
         try {
             CpsThreadGroup g = getProgramPromise().get();
             return getThread(g);
@@ -224,13 +225,24 @@ public String getDisplayName() {
         }
     }
     
-    private @Nonnull ListenableFuture<CpsThreadGroup> getProgramPromise() throws IOException {
-        ListenableFuture<CpsThreadGroup> pp = getFlowExecution().programPromise;
-        assert pp != null;
-        return pp;
+    private @Nonnull ListenableFuture<CpsThreadGroup> getProgramPromise() {
+        final SettableFuture<CpsThreadGroup> f = SettableFuture.create();
+        // TODO is there some more convenient way of writing this using Futures.transform?
+        Timer.get().submit(new Runnable() {
+            @Override public void run() {
+                try {
+                    ListenableFuture<CpsThreadGroup> pp = getFlowExecution().programPromise;
+                    assert pp != null;
+                    f.set(pp.get());
+                } catch (Exception x) { // from getFlowExecution() or get()
+                    f.setException(x);
+                }
+            }
+        });
+        return f;
     }
 
-    @Override public boolean isReady() throws IOException, InterruptedException {
+    @Override public boolean isReady() {
         return getProgramPromise().isDone();
     }
 
diff --git a/step-api/src/main/java/org/jenkinsci/plugins/workflow/steps/StepContext.java b/step-api/src/main/java/org/jenkinsci/plugins/workflow/steps/StepContext.java
@@ -83,7 +83,7 @@ public abstract class StepContext implements FutureCallback<Object>, Serializabl
      * May be called to break deadlocks during reloading.
      * @return true normally, false if we are still reloading the context, for example during unpickling
      */
-    public abstract boolean isReady() throws IOException, InterruptedException;
+    public abstract boolean isReady();
 
     /**
      * Requests that any state held by the {@link StepExecution} be saved to disk.

Original file line number	Diff line number	Diff line change
`@@ -650,7 +650,6 @@ public static void finish(final boolean terminate) {`
`650`	`650`	`});`
`651`	`651`	`}`
`652`	`652`
`653`		`- @RandomlyFails("TODO JENKINS-25890 sometimes triggers a deadlock after restart")`
`654`	`653`	`@Issue("JENKINS-26513")`
`655`	`654`	`@Test public void executorStepRestart() {`
`656`	`655`	`story.addStep(new Statement() {`
Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ public void setResult(Result r) {`
`74`	`74`	`}`
`75`	`75`
`76`	`76`	`@Override`
`77`		`- public boolean isReady() throws IOException, InterruptedException {`
	`77`	`+ public boolean isReady() {`
`78`	`78`	`return base.isReady();`
`79`	`79`	`}`
`80`	`80`