[FIXED JENKINS-28926] Block while upstream/downstream building cycles never complete

- One could argue that without this change the system is functioning correctly and that previous behaviour
  was a bug. On the other hand, people have come to rely on the previous behaviour.
- The issue really centeres around state changes in the blocked tasks. Since blocking on upstream/downstream
  relies on checking the building projects and the queued (excluding blocked) tasks we need any change in
  the blocked task list to be visible immediately (i.e. update the snapshot)
- I was able to reliably reproduce this behaviour with a convoluted set of manually configured projects
  but turning this into a test case has not proved quite as easy. Manual testing confirms that the issue is
  fixed for my manual test case
- I have also added a sorting of the blocked list when probing for tasks to unblock. This should prioritise
  tasks as intended by the QueueSorter

core/src/main/java/hudson/model/Queue.java

@@ -1408,15 +1408,29 @@ public class Queue extends ResourceController implements Saveable {
                 }
             }
 
+            final QueueSorter s = sorter;
 
             {// blocked -> buildable
-                for (BlockedItem p : new ArrayList<BlockedItem>(blockedProjects.values())) {// copy as we'll mutate the list
+                // copy as we'll mutate the list and we want to process in a potentially different order
+                // TODO replace with <> operator after backporting to 1.609.3
+                List<BlockedItem> blockedItems = new ArrayList<BlockedItem>(blockedProjects.values());
+                // if facing a cycle of blocked tasks, ensure we process in the desired sort order
+                if (s != null) {
+                    s.sortBlockedItems(blockedItems);
+                } else {
+                    Collections.sort(blockedItems, QueueSorter.DEFAULT_BLOCKED_ITEM_COMPARATOR);
+                }
+                for (BlockedItem p : blockedItems) {
                     if (!isBuildBlocked(p) && allowNewBuildableTask(p.task)) {
                         // ready to be executed
                         Runnable r = makeBuildable(new BuildableItem(p));
                         if (r != null) {
                             p.leave(this);
                             r.run();
+                            // JENKINS-28926 we have removed a task from the blocked projects and added to building
+                            // thus we should update the snapshot so that subsequent blocked projects can correctly
+                            // determine if they are blocked by the lucky winner
+                            updateSnapshot();
                         }
                     }
                 }
@@ -1446,7 +1460,6 @@ public class Queue extends ResourceController implements Saveable {
                 }
             }
 
-            final QueueSorter s = sorter;
             if (s != null)
                 s.sortBuildableItems(buildables);
             
@@ -1461,6 +1474,9 @@ public class Queue extends ResourceController implements Saveable {
                     p.leave(this);
                     new BlockedItem(p).enter(this);
                     LOGGER.log(Level.FINE, "Catching that {0} is blocked in the last minute", p);
+                    // JENKINS-28926 we have moved an unblocked task into the blocked state, update snapshot
+                    // so that other buildables which might have been blocked by this can see the state change
+                    updateSnapshot();
                     continue;
                 }
 

core/src/main/java/hudson/model/queue/QueueSorter.java

@@ -8,6 +8,8 @@ import hudson.model.LoadBalancer;
 import hudson.model.Queue;
 import hudson.model.Queue.BuildableItem;
 
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 import java.util.logging.Logger;
 
@@ -19,6 +21,20 @@ import static hudson.init.InitMilestone.JOB_LOADED;
  * @since 1.343
  */
 public abstract class QueueSorter implements ExtensionPoint {
+    /**
+     * A comparator that sorts {@link Queue.BlockedItem} instances based on how long they have been in the queue.
+     * (We want the time since in queue by default as blocking on upstream/downstream considers waiting items
+     * also and thus the blocking starts once the task is in the queue not once the task is buildable)
+     *
+     * @since 1.FIXME
+     */
+    public static final Comparator<Queue.BlockedItem> DEFAULT_BLOCKED_ITEM_COMPARATOR = new Comparator<Queue.BlockedItem>() {
+        @Override
+        public int compare(Queue.BlockedItem o1, Queue.BlockedItem o2) {
+            return Long.compare(o1.getInQueueSince(), o2.getInQueueSince());
+        }
+    };
+
     /**
      * Sorts the buildable items list. The items at the beginning will be executed
      * before the items at the end of the list.
@@ -28,6 +44,18 @@ public abstract class QueueSorter implements ExtensionPoint {
      */
     public abstract void sortBuildableItems(List<BuildableItem> buildables);
 
+    /**
+     * Sorts the blocked items list. The items at the beginning will be considered for removal from the blocked state
+     * before the items at the end of the list.
+     *
+     * @param blockedItems
+     *      List of blocked items in the queue. Never null.
+     * @since 1.FIXME
+     */
+    public void sortBlockedItems(List<Queue.BlockedItem> blockedItems) {
+        Collections.sort(blockedItems, DEFAULT_BLOCKED_ITEM_COMPARATOR);
+    }
+
     /**
      * All registered {@link QueueSorter}s. Only the first one will be picked up,
      * unless explicitly overridden by {@link Queue#setSorter(QueueSorter)}.