[JENKINS-30269] Avoid infinite checking in the background task

jenkinsci · Mar 10, 2016 · d538492 · d538492
1 parent c716299
commit d538492
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 7 deletions.
diff --git a/src/main/java/org/jenkins/plugins/lockableresources/LockStepExecution.java b/src/main/java/org/jenkins/plugins/lockableresources/LockStepExecution.java
@@ -5,13 +5,17 @@
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
+import javax.annotation.Nonnull;
+
 import org.jenkins.plugins.lockableresources.queue.LockableResourcesStruct;
 import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
 import org.jenkinsci.plugins.workflow.steps.BodyExecution;
 import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
 import org.jenkinsci.plugins.workflow.steps.StepContext;
 import org.jenkinsci.plugins.workflow.steps.StepContextParameter;
+import org.jenkinsci.plugins.workflow.steps.StepExecution;
 
+import com.google.common.base.Function;
 import com.google.inject.Inject;
 
 import hudson.model.Run;
@@ -31,6 +35,7 @@ public class LockStepExecution extends AbstractStepExecutionImpl {
 
 	private transient volatile ScheduledFuture<?> task;
 	private BodyExecution body;
+	private final String id = UUID.randomUUID().toString();
 
 	@Override
 	public boolean start() throws Exception {
@@ -39,17 +44,18 @@ public boolean start() throws Exception {
 	}
 
 	private void tryLock(long delay) {
-		Timer.get().schedule(new Runnable() {
+		task = Timer.get().schedule(new Runnable() {
 			@Override
 			public void run() {
-				if (!proceed()) {
-					tryLock(1000); // try to lock every second
+				task = null;
+				if (!lockAndProceed()) {
+					retry(id, 1000); // try to lock every second
 				}
 			}
 		}, delay, TimeUnit.MILLISECONDS);
 	}
 
-	private boolean proceed() {
+	private boolean lockAndProceed() {
 		LockableResourcesStruct resourceHolder = new LockableResourcesStruct(step.resource);
 		if (LockableResourcesManager.get().lock(resourceHolder.required, run)) {
 			listener.getLogger().println("Lock aquired on [" + step.resource + "]");
@@ -63,6 +69,19 @@ private boolean proceed() {
 		}
 	}
 
+	private static void retry(final String id, final long delay) {
+		// retry only if the the execution of this step has not being requested to stop
+		StepExecution.applyAll(LockStepExecution.class, new Function<LockStepExecution, Void>() {
+			@Override
+			public Void apply(@Nonnull LockStepExecution execution) {
+				if (execution.id.equals(id)) {
+					execution.tryLock(delay);
+				}
+				return null;
+			}
+		});
+	}
+
 	private static final class Callback extends BodyExecutionCallback.TailCall {
 
 		private final LockableResourcesStruct resourceHolder;
@@ -87,6 +106,10 @@ public void stop(Throwable cause) throws Exception {
 		if (body != null) {
 			body.cancel(cause);
 		}
+		if (task != null) {
+			task.cancel(false);
+		}
+		getContext().onFailure(cause);
 	}
 
 	private static final long serialVersionUID = 1L;

diff --git a/src/main/java/org/jenkins/plugins/lockableresources/LockableResource.java b/src/main/java/org/jenkins/plugins/lockableresources/LockableResource.java
@@ -50,10 +50,12 @@ public class LockableResource extends AbstractDescribableImpl<LockableResource>
 	private String labels = "";
 	private String reservedBy = null;
 
-	private transient int queueItemId = NOT_QUEUED;
-	private transient String queueItemProject = null;
+	private int queueItemId = NOT_QUEUED;
+	private String queueItemProject = null;
 	private transient Run<?, ?> build = null;
-	private transient long queuingStarted = 0;
+	// Needed to make the state non-transient
+	private String buildExternalizableId = null;
+	private long queuingStarted = 0;
 
 	@Deprecated
 	public LockableResource(
@@ -184,6 +186,9 @@ public boolean isLocked() {
 
 	@WithBridgeMethods(value=AbstractBuild.class, adapterMethod="getAbstractBuild")
 	public Run<?, ?> getBuild() {
+		if (build == null && buildExternalizableId != null) {
+			build = Run.fromExternalizableId(buildExternalizableId);
+		}
 		return build;
 	}
 
@@ -205,6 +210,7 @@ public String getBuildName() {
 
 	public void setBuild(Run<?, ?> lockedBy) {
 		this.build = lockedBy;
+		this.buildExternalizableId = lockedBy.getExternalizableId();
 	}
 
 	public Task getTask() {