Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-26761] Recover gracefully from null WorkflowRun.checkouts, p…
…rinting some diagnostics.

Originally-Committed-As: 7d21f2d187a9f8973df403b47b5efd66e6b00bd2
  • Loading branch information
jglick committed Jul 28, 2015
1 parent 884e62d commit 34802c5
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 12 deletions.
Expand Up @@ -439,7 +439,7 @@ public void addTrigger(Trigger trigger) {
return Collections.emptySet();
}
List<SCM> scms = new LinkedList<SCM>();
for (WorkflowRun.SCMCheckout co : b.checkouts) {
for (WorkflowRun.SCMCheckout co : b.checkouts(null)) {
scms.add(co.scm);
}
return scms;
Expand All @@ -464,7 +464,7 @@ public void addTrigger(Trigger trigger) {
listener.getLogger().println("no previous build to compare to");
return PollingResult.NO_CHANGES;
}
for (WorkflowRun.SCMCheckout co : b.checkouts) {
for (WorkflowRun.SCMCheckout co : b.checkouts(listener)) {
if (!co.scm.supportsPolling()) {
listener.getLogger().println("polling not supported from " + co.workspace + " on " + co.node);
}
Expand Down
Expand Up @@ -118,7 +118,8 @@ public final class WorkflowRun extends Run<WorkflowJob,WorkflowRun> implements Q
/** map from node IDs to log positions from which we should copy text */
private Map<String,Long> logsToCopy;

List<SCMCheckout> checkouts;
/** JENKINS-26761: supposed to always be set but sometimes is not. Access only through {@link #checkouts(TaskListener)}. */
private @CheckForNull List<SCMCheckout> checkouts;
// TODO could use a WeakReference to reduce memory, but that complicates how we add to it incrementally; perhaps keep a List<WeakReference<ChangeLogSet<?>>>
private transient List<ChangeLogSet<? extends ChangeLogSet.Entry>> changeSets;

Expand All @@ -128,6 +129,7 @@ public final class WorkflowRun extends Run<WorkflowJob,WorkflowRun> implements Q
public WorkflowRun(WorkflowJob job) throws IOException {
super(job);
firstTime = true;
checkouts = new LinkedList<SCMCheckout>();
//System.err.printf("created %s @%h%n", this, this);
}

Expand Down Expand Up @@ -178,7 +180,6 @@ public WorkflowRun(WorkflowJob job, File dir) throws IOException {
listener.error("No flow definition, cannot run");
return;
}
checkouts = new LinkedList<SCMCheckout>();
Owner owner = new Owner(this);

FlowExecution newExecution = definition.create(owner, listener, getAllActions());
Expand Down Expand Up @@ -443,6 +444,7 @@ private String key() {
Queue.getInstance().schedule(new AfterRestartTask(this), 0);
}
}
checkouts(null); // only for diagnostics
synchronized (LOADING_RUNS) {
LOADING_RUNS.remove(key()); // or could just make the value type be WeakReference<WorkflowRun>
LOADING_RUNS.notifyAll();
Expand Down Expand Up @@ -522,15 +524,23 @@ public boolean hasntStartedYet() {
return isBuilding(); // there is no equivalent to a post-production state for flows
}

synchronized @Nonnull List<SCMCheckout> checkouts(@CheckForNull TaskListener listener) {
if (checkouts == null) {
LOGGER.log(Level.WARNING, "JENKINS-26761: no checkouts in {0}", this);
if (listener != null) {
listener.error("JENKINS-26761: list of SCM checkouts in " + this + " was lost; polling will be broken");
}
checkouts = new LinkedList<SCMCheckout>();
// Could this.save(), but might pollute diagnosis, and (worse) might clobber real data if there is >1 WorkflowRun with the same ID.
}
return checkouts;
}

@Exported
public synchronized List<ChangeLogSet<? extends ChangeLogSet.Entry>> getChangeSets() {
if (changeSets == null) {
changeSets = new ArrayList<ChangeLogSet<? extends ChangeLogSet.Entry>>();
if (checkouts == null) {
LOGGER.log(Level.WARNING, "no checkouts in {0}", this);
return changeSets;
}
for (SCMCheckout co : checkouts) {
for (SCMCheckout co : checkouts(null)) {
if (co.changelogFile != null && co.changelogFile.isFile()) {
try {
changeSets.add(co.scm.createChangeLogParser().parse(this, co.scm.getEffectiveBrowser(), co.changelogFile));
Expand All @@ -553,7 +563,7 @@ public synchronized HttpResponse doStop() {
}
}

private void onCheckout(SCM scm, FilePath workspace, @CheckForNull File changelogFile, @CheckForNull SCMRevisionState pollingBaseline) throws Exception {
private void onCheckout(SCM scm, FilePath workspace, TaskListener listener, @CheckForNull File changelogFile, @CheckForNull SCMRevisionState pollingBaseline) throws Exception {
if (changelogFile != null && changelogFile.isFile()) {
ChangeLogSet<?> cls = scm.createChangeLogParser().parse(this, scm.getEffectiveBrowser(), changelogFile);
getChangeSets().add(cls);
Expand All @@ -566,7 +576,7 @@ private void onCheckout(SCM scm, FilePath workspace, @CheckForNull File changelo
if (computer == null) {
throw new IllegalStateException();
}
checkouts.add(new SCMCheckout(scm, computer.getName(), workspace.getRemote(), changelogFile, pollingBaseline));
checkouts(listener).add(new SCMCheckout(scm, computer.getName(), workspace.getRemote(), changelogFile, pollingBaseline));
}

static final class SCMCheckout {
Expand Down Expand Up @@ -716,7 +726,7 @@ static void alias() {
@Extension public static final class SCMListenerImpl extends SCMListener {
@Override public void onCheckout(Run<?,?> build, SCM scm, FilePath workspace, TaskListener listener, File changelogFile, SCMRevisionState pollingBaseline) throws Exception {
if (build instanceof WorkflowRun) {
((WorkflowRun) build).onCheckout(scm, workspace, changelogFile, pollingBaseline);
((WorkflowRun) build).onCheckout(scm, workspace, listener, changelogFile, pollingBaseline);
}
}
}
Expand Down

0 comments on commit 34802c5

Please sign in to comment.