[FIXED JENKINS-25314]

The original fix was made in 0c3d670. In this fix, I'm also moving the code that creates LeftItem. It makes no sense that there's Item.cancel(Queue) and Queue.cancel(Item) and they do different things!
jenkinsci · Oct 26, 2014 · 9b36e2a · 9b36e2a
1 parent b4a9671
commit 9b36e2a
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 7 deletions.
diff --git a/changelog.html b/changelog.html
@@ -55,6 +55,9 @@
 <!-- Record your changes in the trunk here. -->
 <div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
 <ul class=image>
+  <li class=bug>
+    Queue didn't always leave a trail for cancelled items properly
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-25314">issue 25314</a>)
   <li class=bug>
     JNA update for deprecated JNA-POSIX library.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-24527">issue 24527</a>)

diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java
@@ -658,12 +658,7 @@ public synchronized boolean cancel(Task p) {
 
     public synchronized boolean cancel(Item item) {
         LOGGER.log(Level.FINE, "Cancelling {0} item#{1}", new Object[] {item.task, item.id});
-        boolean r = item.cancel(this);
-
-        LeftItem li = new LeftItem(item);
-        li.enter(this);
-
-        return r;
+        return item.cancel(this);
     }
 
     /**
@@ -1588,10 +1583,17 @@ public String toString() {
         /**
          * Cancels this item, which updates {@link #future} to notify the listener, and
          * also leaves the queue.
+         *
+         * @return true
+         *      if the item was successfully cancelled.
          */
         /*package*/ boolean cancel(Queue q) {
             boolean r = leave(q);
-            if (r)  future.setAsCancelled();
+            if (r) {
+                future.setAsCancelled();
+                LeftItem li = new LeftItem(this);
+                li.enter(q);
+            }
             return r;
         }