提交 f360ce3e 编写于 作者: J Jesse Glick

Javadoc for Queue.Task claimed they were compared using .equals(), but in...

Javadoc for Queue.Task claimed they were compared using .equals(), but in critical cases this was not actually true.
Does not matter for the common case of Job, but when implementing custom tasks this could cause duplicate queue items (and executables).
上级 a7218821
......@@ -116,6 +116,7 @@ import org.kohsuke.stapler.export.ExportedBean;
import com.thoughtworks.xstream.XStream;
import com.thoughtworks.xstream.converters.basic.AbstractSingleValueConverter;
import javax.annotation.CheckForNull;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.interceptor.RequirePOST;
......@@ -448,12 +449,9 @@ public class Queue extends ResourceController implements Saveable {
/**
* Schedule a new build for this project.
*
* @return true if the project is actually added to the queue.
* false if the queue contained it and therefore the add()
* was noop
* @see #schedule(Task, int)
*/
public WaitingItem schedule(AbstractProject p) {
public @CheckForNull WaitingItem schedule(AbstractProject p) {
return schedule(p, p.getQuietPeriod());
}
......@@ -603,7 +601,7 @@ public class Queue extends ResourceController implements Saveable {
return schedule(p, quietPeriod)!=null;
}
public synchronized WaitingItem schedule(Task p, int quietPeriod) {
public synchronized @CheckForNull WaitingItem schedule(Task p, int quietPeriod) {
return schedule(p, quietPeriod, new Action[0]);
}
......@@ -618,7 +616,7 @@ public class Queue extends ResourceController implements Saveable {
/**
* Convenience wrapper method around {@link #schedule(Task, int, List)}
*/
public synchronized WaitingItem schedule(Task p, int quietPeriod, Action... actions) {
public synchronized @CheckForNull WaitingItem schedule(Task p, int quietPeriod, Action... actions) {
return schedule2(p, quietPeriod, actions).getCreateItem();
}
......@@ -853,8 +851,9 @@ public class Queue extends ResourceController implements Saveable {
return bi;
for (Item item : waitingList) {
if (item.task == t)
if (item.task.equals(t)) {
return item;
}
}
return null;
}
......@@ -870,8 +869,9 @@ public class Queue extends ResourceController implements Saveable {
result.addAll(buildables.getAll(t));
result.addAll(pendings.getAll(t));
for (Item item : waitingList) {
if (item.task == t)
if (item.task.equals(t)) {
result.add(item);
}
}
return result;
}
......@@ -892,8 +892,9 @@ public class Queue extends ResourceController implements Saveable {
if (blockedProjects.containsKey(t) || buildables.containsKey(t) || pendings.containsKey(t))
return true;
for (Item item : waitingList) {
if (item.task == t)
if (item.task.equals(t)) {
return true;
}
}
return false;
}
......@@ -2000,7 +2001,7 @@ public class Queue extends ResourceController implements Saveable {
private class ItemList<T extends Item> extends ArrayList<T> {
public T get(Task task) {
for (T item: this) {
if (item.task == task) {
if (item.task.equals(task)) {
return item;
}
}
......@@ -2010,7 +2011,7 @@ public class Queue extends ResourceController implements Saveable {
public List<T> getAll(Task task) {
List<T> result = new ArrayList<T>();
for (T item: this) {
if (item.task == task) {
if (item.task.equals(task)) {
result.add(item);
}
}
......@@ -2025,7 +2026,7 @@ public class Queue extends ResourceController implements Saveable {
Iterator<T> it = iterator();
while (it.hasNext()) {
T t = it.next();
if (t.task == task) {
if (t.task.equals(task)) {
it.remove();
return t;
}
......@@ -2034,7 +2035,7 @@ public class Queue extends ResourceController implements Saveable {
}
public void put(Task task, T item) {
assert item.task == task;
assert item.task.equals(task);
add(item);
}
......
......@@ -5,6 +5,7 @@ import hudson.model.Queue;
import hudson.model.Queue.Item;
import hudson.model.Queue.Task;
import hudson.model.Queue.WaitingItem;
import javax.annotation.CheckForNull;
/**
* Result of {@link Queue#schedule2}
......@@ -36,7 +37,7 @@ public abstract class ScheduleResult {
* the newly created item in the queue or the existing item that's already
* in the queue that matched the submitted task.
*/
public Item getItem() {
public @CheckForNull Item getItem() {
return null;
}
......@@ -44,7 +45,7 @@ public abstract class ScheduleResult {
* If {@link #isCreated()} returns true, then this method returns
* the newly created item, which is always of the type {@link WaitingItem}.
*/
public WaitingItem getCreateItem() {
public @CheckForNull WaitingItem getCreateItem() {
return null;
}
......
......@@ -26,27 +26,48 @@ package hudson.model;
import com.gargoylesoftware.htmlunit.html.HtmlFileInput;
import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import hudson.Launcher;
import hudson.matrix.AxisList;
import hudson.matrix.LabelAxis;
import hudson.matrix.MatrixBuild;
import hudson.matrix.MatrixProject;
import hudson.matrix.MatrixRun;
import hudson.matrix.TextAxis;
import hudson.model.Cause.*;
import hudson.model.Cause.RemoteCause;
import hudson.model.Cause.UserIdCause;
import hudson.model.Queue.*;
import hudson.model.Queue.BlockedItem;
import hudson.model.queue.AbstractQueueTask;
import hudson.model.queue.QueueTaskFuture;
import hudson.model.queue.ScheduleResult;
import hudson.model.queue.SubTask;
import hudson.security.ACL;
import hudson.security.GlobalMatrixAuthorizationStrategy;
import hudson.security.SparseACL;
import hudson.slaves.DumbSlave;
import hudson.slaves.DummyCloudImpl;
import hudson.slaves.NodeProvisioner;
import hudson.tasks.Shell;
import hudson.triggers.SCMTrigger.SCMTriggerCause;
import hudson.triggers.TimerTrigger.TimerTriggerCause;
import hudson.util.XStream2;
import hudson.util.OneShotEvent;
import hudson.Launcher;
import hudson.matrix.LabelAxis;
import hudson.matrix.MatrixRun;
import hudson.slaves.DummyCloudImpl;
import hudson.slaves.NodeProvisioner;
import hudson.util.XStream2;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicInteger;
import javax.inject.Inject;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import jenkins.model.Jenkins;
import jenkins.security.QueueItemAuthenticator;
import jenkins.security.QueueItemAuthenticatorConfiguration;
......@@ -68,22 +89,6 @@ import org.mortbay.jetty.bio.SocketConnector;
import org.mortbay.jetty.servlet.ServletHandler;
import org.mortbay.jetty.servlet.ServletHolder;
import javax.inject.Inject;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
/**
* @author Kohsuke Kawaguchi
*/
......@@ -348,6 +353,50 @@ public class QueueTest extends HudsonTestCase {
assertEquals("slave0", runs.get(0).getBuiltOnStr());
}
public void testTaskEquality() throws Exception {
AtomicInteger cnt = new AtomicInteger();
ScheduleResult result = jenkins.getQueue().schedule2(new TestTask(cnt), 0);
assertTrue(result.isCreated());
WaitingItem item = result.getCreateItem();
assertFalse(jenkins.getQueue().schedule2(new TestTask(cnt), 0).isCreated());
item.getFuture().get();
waitUntilNoActivity();
assertEquals(1, cnt.get());
}
private static final class TestTask extends AbstractQueueTask {
private final AtomicInteger cnt;
TestTask(AtomicInteger cnt) {
this.cnt = cnt;
}
@Override public boolean equals(Object o) {
return o instanceof TestTask && cnt == ((TestTask) o).cnt;
}
@Override public int hashCode() {
return cnt.hashCode();
}
@Override public boolean isBuildBlocked() {return false;}
@Override public String getWhyBlocked() {return null;}
@Override public String getName() {return "test";}
@Override public String getFullDisplayName() {return "Test";}
@Override public void checkAbortPermission() {}
@Override public boolean hasAbortPermission() {return true;}
@Override public String getUrl() {return "test/";}
@Override public String getDisplayName() {return "Test";}
@Override public Label getAssignedLabel() {return null;}
@Override public Node getLastBuiltOn() {return null;}
@Override public long getEstimatedDuration() {return -1;}
@Override public ResourceList getResourceList() {return new ResourceList();}
@Override public Executable createExecutable() throws IOException {
return new Executable() {
@Override public SubTask getParent() {return TestTask.this;}
@Override public long getEstimatedDuration() {return -1;}
@Override public void run() {
cnt.incrementAndGet();
}
};
}
}
public void testWaitForStart() throws Exception {
final OneShotEvent ev = new OneShotEvent();
FreeStyleProject p = createFreeStyleProject();
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册