Skip to content

Commit

Permalink
[JENKINS-67351] Avoid deadlock when resuming Pipelines in some cases (j…
Browse files Browse the repository at this point in the history
…enkinsci#188)

* [JENKINS-67531] Avoid deadlock when resuming Pipelines in some cases

* [JENKINS-67531] Switch to an independent ExecutorService as a precaution against other types of deadlock as in JENKINS-25890

* [JENKINS-67351] Add new FlowExecutionList.isResumptionComplete method to help consumers avoid resuming Pipelines inadvertently

* [JENKINS-67351] Mark FlowExecutionList.isResumptionComplete as a beta API

* Revert "[JENKINS-67531] Switch to an independent ExecutorService as a precaution against other types of deadlock as in JENKINS-25890"

This reverts commit c178493.
  • Loading branch information
dwnusbaum authored Dec 17, 2021
1 parent 6a04bac commit 6d6de20
Showing 1 changed file with 20 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import hudson.Extension;
import hudson.ExtensionList;
import hudson.XmlFile;
import hudson.init.InitMilestone;
import hudson.init.Terminator;
import hudson.model.listeners.ItemListener;
import hudson.remoting.SingleLaneExecutorService;
Expand All @@ -31,6 +32,7 @@
import java.util.logging.Logger;

import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.Beta;
import org.kohsuke.accmod.restrictions.DoNotUse;

/**
Expand All @@ -44,6 +46,8 @@ public class FlowExecutionList implements Iterable<FlowExecution> {
private final SingleLaneExecutorService executor = new SingleLaneExecutorService(Timer.get());
private XmlFile configFile;

private transient volatile boolean resumptionComplete;

public FlowExecutionList() {
load();
}
Expand Down Expand Up @@ -163,6 +167,18 @@ public static FlowExecutionList get() {
return l;
}

/**
* Returns true if all executions that were present in this {@link FlowExecutionList} have been loaded and resumed.
*
* This takes place slightly after {@link InitMilestone#COMPLETED} is reached during Jenkins startup.
*
* Useful to avoid resuming Pipelines in contexts that may lead to deadlock.
*/
@Restricted(Beta.class)
public boolean isResumptionComplete() {
return resumptionComplete;
}

/**
* When Jenkins starts up and everything is loaded, be sure to proactively resurrect
* all the ongoing {@link FlowExecution}s so that they start running again.
Expand All @@ -171,10 +187,12 @@ public static FlowExecutionList get() {
public static class ItemListenerImpl extends ItemListener {
@Override
public void onLoaded() {
for (final FlowExecution e : FlowExecutionList.get()) {
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.
LOGGER.log(Level.FINE, "Eagerly loaded {0}", e);
}
list.resumptionComplete = true;
}
}

Expand Down Expand Up @@ -279,7 +297,7 @@ public void onFailure(Throwable t) {
}
}

}, MoreExecutors.directExecutor()); // TODO: Unclear if we need to run this asynchronously or if StepExecution.onResume has any particular thread requirements.
}, Timer.get()); // We always hold RunMap and WorkflowRun locks here, so we resume steps on a different thread to avoid potential deadlocks. See JENKINS-67351.
}
}
}

0 comments on commit 6d6de20

Please sign in to comment.