Skip to content
Permalink
Browse files

[FIXED JENKINS-31614] Avoid acquiring the Queue lock while holding ot…

…her locks.
  • Loading branch information
jglick committed Jan 5, 2016
1 parent be247cb commit c89a054375f4e58947e9926d0be74abc5174a478
@@ -5,6 +5,7 @@ Only noting significant user changes, not internal code cleanups and minor bug f
## 1.13 (upcoming)

* [JENKINS-26126](https://issues.jenkins-ci.org/browse/JENKINS-26126) (partial): introspect Workflow steps to generate static reference documentation (link from _Snippet Generator_). Planned to be used for IDE auto-completion as well.
* [JENKINS-31614](https://issues.jenkins-ci.org/browse/JENKINS-31614): avoiding various deadlocks involving `Queue`.
* [JENKINS-31897](https://issues.jenkins-ci.org/browse/JENKINS-31897): parameters with default values may now be omitted from the `parameters` option to the `build` step.
* [JENKINS-31909](https://issues.jenkins-ci.org/browse/JENKINS-31909): form validation warning about Groovy syntax errors was broken in 1.11.
* [JENKINS-31391](https://issues.jenkins-ci.org/browse/JENKINS-31391): pass `EXECUTOR_NUMBER` into `node {}`.
@@ -39,6 +39,7 @@
/**
* Start preparing rehydration of this value, and when it's ready or fail, report that to the
* given call.
* An implementation should return quickly and avoid acquiring locks in this method itself (as opposed to the future).
*/
public abstract ListenableFuture<?> rehydrate();

@@ -474,7 +474,11 @@ private String key() {
listener = new StreamBuildListener(new NullStream());
}
completed = new AtomicBoolean();
Queue.getInstance().schedule(new AfterRestartTask(this), 0);
Timer.get().submit(new Runnable() { // JENKINS-31614
@Override public void run() {
Queue.getInstance().schedule(new AfterRestartTask(WorkflowRun.this), 0);
}
});
}
}
checkouts(null); // only for diagnostics
@@ -25,7 +25,6 @@
package org.jenkinsci.plugins.workflow.support.pickles;

import org.jenkinsci.plugins.workflow.pickles.Pickle;
import org.jenkinsci.plugins.workflow.support.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import hudson.Extension;
import hudson.model.Executor;
@@ -62,17 +61,19 @@ private ExecutorPickle(Executor executor) {
}

@Override public ListenableFuture<Executor> rehydrate() {
Queue.Item item = Queue.getInstance().schedule2(task, 0).getItem();
if (item == null) {
// TODO should also report when !ScheduleResult.created, since that is arguably an error
return Futures.immediateFailedFuture(new IllegalStateException("queue refused " + task));
}

final Future<Queue.Executable> future = item.getFuture().getStartCondition();

return new TryRepeatedly<Executor>(1) {
return new TryRepeatedly<Executor>(1, 0) {
Future<Queue.Executable> future;
@Override
protected Executor tryResolve() throws Exception {
if (future == null) {
Queue.Item item = Queue.getInstance().schedule2(task, 0).getItem();
if (item == null) {
// TODO should also report when !ScheduleResult.created, since that is arguably an error
throw new IllegalStateException("queue refused " + task);
}
future = item.getFuture().getStartCondition();
}

if (!future.isDone()) {
return null;
}
@@ -38,15 +38,19 @@
* @author Kohsuke Kawaguchi
*/
public abstract class TryRepeatedly<V> extends AbstractFuture<V> {
private final int seconds;
private final int delay;
private ScheduledFuture<?> next;

protected TryRepeatedly(int seconds) {
this.seconds = seconds;
tryLater();
protected TryRepeatedly(int delay) {
this(delay, delay);
}

private void tryLater() {
protected TryRepeatedly(int delay, int initialDelay) {
this.delay = delay;
tryLater(initialDelay);
}

private void tryLater(int currentDelay) {
// TODO log a warning if trying for too long; probably Pickle.rehydrate should be given a TaskListener to note progress

if (isCancelled()) return;
@@ -57,14 +61,14 @@ public void run() {
try {
V v = tryResolve();
if (v == null)
tryLater();
tryLater(delay);
else
set(v);
} catch (Throwable t) {
setException(t);
}
}
}, seconds, TimeUnit.SECONDS);
}, currentDelay, TimeUnit.SECONDS);
}

@Override
@@ -348,8 +348,13 @@ private static void finish(@CheckForNull String cookie) {
LOGGER.log(FINE, "no running task corresponds to {0}", cookie);
return;
}
assert runningTask.execution != null && runningTask.launcher != null;
runningTask.execution.completed(null);
final AsynchronousExecution execution = runningTask.execution;
assert execution != null && runningTask.launcher != null;
Timer.get().submit(new Runnable() { // JENKINS-31614
@Override public void run() {
execution.completed(null);
}
});
try {
runningTask.launcher.kill(Collections.singletonMap(COOKIE_VAR, cookie));
} catch (ChannelClosedException x) {

0 comments on commit c89a054

Please sign in to comment.
You can’t perform that action at this time.