Merge pull request jenkinsci#278 from dwnusbaum/pickleresolver-timeout

Add a timeout for pickle resolution to prevent indefinitely stuck builds

Author: jglick

src/main/java/org/jenkinsci/plugins/workflow/support/pickles/serialization/PickleResolver.java

@@ -24,23 +24,38 @@
 
 package org.jenkinsci.plugins.workflow.support.pickles.serialization;
 
-import org.jenkinsci.plugins.workflow.pickles.Pickle;
-import org.jenkinsci.plugins.workflow.support.concurrent.Futures;
 import com.google.common.base.Function;
+import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
-import org.jboss.marshalling.ObjectResolver;
-
+import com.google.common.util.concurrent.MoreExecutors;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.concurrent.TimeUnit;
+import jenkins.util.SystemProperties;
+import jenkins.util.Timer;
+import org.jboss.marshalling.ObjectResolver;
 import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner;
+import org.jenkinsci.plugins.workflow.pickles.Pickle;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * {@link ObjectResolver} that resolves {@link DryCapsule} to unpickled objects.
  *
  * @author Kohsuke Kawaguchi
  */
 public class PickleResolver implements ObjectResolver {
+
+    /**
+     * Pickle resolution will fail automatically after this many seconds.
+     * <p>This is intended to prevent Pipeline builds from hanging forever in unusual cases.
+     */
+    @Restricted(NoExternalUse.class)
+    @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Non-final for modification from script console")
+    public static long RESOLUTION_TIMEOUT_SECONDS = SystemProperties.getLong(PickleResolver.class + ".RESOLUTION_TIMEOUT_SECONDS", TimeUnit.HOURS.toSeconds(1));
+
     /**
      * Persisted forms of the stateful objects.
      */
@@ -83,7 +98,7 @@ public ListenableFuture<PickleResolver> rehydrate(Collection<ListenableFuture<?>
             // TODO log("rehydrating " + r);
             ListenableFuture<?> future;
             try {
-                future = r.rehydrate(owner);
+                future = Futures.withTimeout(r.rehydrate(owner), RESOLUTION_TIMEOUT_SECONDS, TimeUnit.SECONDS, Timer.get());
             } catch (RuntimeException x) {
                 future = Futures.immediateFailedFuture(x);
             }
@@ -93,7 +108,7 @@ public ListenableFuture<PickleResolver> rehydrate(Collection<ListenableFuture<?>
                     // TODO log("rehydrated to " + input);
                     return input;
                 }
-            }));
+            }, MoreExecutors.directExecutor()));
         }
 
         ListenableFuture<List<Object>> all = Futures.allAsList(members);
@@ -104,7 +119,7 @@ public PickleResolver apply(List<Object> input) {
                 values = input;
                 return PickleResolver.this;
             }
-        });
+        }, MoreExecutors.directExecutor());
     }
 
     @Override

src/test/java/org/jenkinsci/plugins/workflow/support/pickles/serialization/PickleResolverTest.java

@@ -0,0 +1,76 @@
+package org.jenkinsci.plugins.workflow.support.pickles.serialization;
+
+import com.google.common.util.concurrent.ListenableFuture;
+import hudson.model.Result;
+import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
+import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner;
+import org.jenkinsci.plugins.workflow.job.WorkflowJob;
+import org.jenkinsci.plugins.workflow.pickles.Pickle;
+import org.jenkinsci.plugins.workflow.support.pickles.SingleTypedPickleFactory;
+import org.jenkinsci.plugins.workflow.support.pickles.TryRepeatedly;
+import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.BuildWatcher;
+import org.jvnet.hudson.test.FlagRule;
+import org.jvnet.hudson.test.JenkinsSessionRule;
+import org.jvnet.hudson.test.TestExtension;
+
+public class PickleResolverTest {
+    @ClassRule
+    public static BuildWatcher watcher = new BuildWatcher();
+
+    @Rule
+    public JenkinsSessionRule sessions = new JenkinsSessionRule();
+
+    @Rule
+    public FlagRule<Long> resetPickleResolutionTimeout = new FlagRule<>(() -> PickleResolver.RESOLUTION_TIMEOUT_SECONDS, x -> PickleResolver.RESOLUTION_TIMEOUT_SECONDS = x);
+
+    @Test
+    public void timeout() throws Throwable {
+        sessions.then(r -> {
+            var p = r.jenkins.createProject(WorkflowJob.class, "stuckPickle");
+            p.setDefinition(new CpsFlowDefinition(
+                    "def x = new org.jenkinsci.plugins.workflow.support.pickles.serialization.PickleResolverTest.StuckPickle.Marker()\n" +
+                    "semaphore 'wait'\n" +
+                    "echo x.getClass().getName()", false));
+            var b = p.scheduleBuild2(0).waitForStart();
+            SemaphoreStep.waitForStart("wait/1", b);
+        });
+        PickleResolver.RESOLUTION_TIMEOUT_SECONDS = 3;
+        sessions.then(r -> {
+            var p = r.jenkins.getItemByFullName("stuckPickle", WorkflowJob.class);
+            var b = p.getBuildByNumber(1);
+            r.assertBuildStatus(Result.FAILURE, r.waitForCompletion(b));
+            r.assertLogContains("Timed out: StuckPickle", b);
+        });
+    }
+
+    public static class StuckPickle extends Pickle {
+        @Override
+        public ListenableFuture<Marker> rehydrate(FlowExecutionOwner owner) {
+            return new TryRepeatedly<Marker>(1) {
+                @Override
+                protected Marker tryResolve() {
+                    return null;
+                }
+                @Override protected FlowExecutionOwner getOwner() {
+                    return owner;
+                }
+                @Override public String toString() {
+                    return "StuckPickle for " + owner;
+                }
+            };
+        }
+
+        public static class Marker {}
+
+        @TestExtension("timeout")
+        public static final class Factory extends SingleTypedPickleFactory<Marker> {
+            @Override protected Pickle pickle(Marker object) {
+                return new StuckPickle();
+            }
+        }
+    }
+}