Merge pull request #1596 from stephenc/threadsafe-node-queue

[JENKINS-27565] Fix threading issues with Nodes and Queue

core/src/main/java/hudson/Functions.java

@@ -28,6 +28,7 @@ package hudson;
 import hudson.cli.CLICommand;
 import hudson.console.ConsoleAnnotationDescriptor;
 import hudson.console.ConsoleAnnotatorFactory;
+import hudson.init.InitMilestone;
 import hudson.model.AbstractProject;
 import hudson.model.Action;
 import hudson.model.Describable;
@@ -201,7 +202,25 @@ public class Functions {
     public static String rfc822Date(Calendar cal) {
         return Util.RFC822_DATETIME_FORMATTER.format(cal.getTime());
     }
-    
+
+    /**
+     * During Jenkins start-up, before {@link InitMilestone#PLUGINS_STARTED} the extensions lists will be empty
+     * and they are not guaranteed to be fully populated until after {@link InitMilestone#EXTENSIONS_AUGMENTED}.
+     * If you attempt to access the extensions list from a UI thread while the extensions are being loaded you will
+     * hit a big honking great monitor lock that will block until the effective extension list has been determined
+     * (as if a plugin fails to start, all of the failed plugin's extensions and any dependent plugins' extensions
+     * will have to be evicted from the list of extensions. In practical terms this only affects the
+     * "Jenkins is loading" screen, but as that screen uses the generic layouts we provide this utility method
+     * so that the generic layouts can avoid iterating extension lists while Jenkins is starting up.
+     *
+     * @return {@code true} if the extensions lists have been populated.
+     * @since 1.FIXME
+     */
+    public static boolean isExtensionsAvailable() {
+        final Jenkins jenkins = Jenkins.getInstance();
+        return jenkins != null && jenkins.getInitLevel().compareTo(InitMilestone.EXTENSIONS_AUGMENTED) >= 0;
+    }
+
     public static void initPageVariables(JellyContext context) {
         StaplerRequest currentRequest = Stapler.getCurrentRequest();
         String rootURL = currentRequest.getContextPath();

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

@@ -34,7 +34,6 @@ import jenkins.model.Jenkins;
 import org.kohsuke.stapler.StaplerFallback;
 import org.kohsuke.stapler.StaplerProxy;
 
-import java.io.IOException;
 import java.util.*;
 import java.util.concurrent.CopyOnWriteArraySet;
 import java.util.logging.Logger;
@@ -48,8 +47,6 @@ public abstract class AbstractCIBase extends Node implements ItemGroup<TopLevelI
 
     private static final Logger LOGGER = Logger.getLogger(AbstractCIBase.class.getName());
 
-    private final transient Object updateComputerLock = new Object();
-
     /**
      * If you are calling this on Hudson something is wrong.
      *
@@ -137,15 +134,20 @@ public abstract class AbstractCIBase extends Node implements ItemGroup<TopLevelI
         used.add(c);
     }
 
-    /*package*/ void removeComputer(Computer computer) {
-        Map<Node,Computer> computers = getComputerMap();
-        for (Map.Entry<Node, Computer> e : computers.entrySet()) {
-            if (e.getValue() == computer) {
-                computers.remove(e.getKey());
-                computer.onRemoved();
-                return;
+    /*package*/ void removeComputer(final Computer computer) {
+        Queue.withLock(new Runnable() {
+            @Override
+            public void run() {
+                Map<Node,Computer> computers = getComputerMap();
+                for (Map.Entry<Node, Computer> e : computers.entrySet()) {
+                    if (e.getValue() == computer) {
+                        computers.remove(e.getKey());
+                        computer.onRemoved();
+                        return;
+                    }
+                }
             }
-        }
+        });
     }
 
     /*package*/ @CheckForNull Computer getComputer(Node n) {
@@ -160,36 +162,46 @@ public abstract class AbstractCIBase extends Node implements ItemGroup<TopLevelI
      * This method tries to reuse existing {@link Computer} objects
      * so that we won't upset {@link Executor}s running in it.
      */
-    protected void updateComputerList(boolean automaticSlaveLaunch) throws IOException {
-        Map<Node,Computer> computers = getComputerMap();
-        synchronized(updateComputerLock) {// just so that we don't have two code updating computer list at the same time
-            Map<String,Computer> byName = new HashMap<String,Computer>();
-            for (Computer c : computers.values()) {
-                Node node = c.getNode();
-                if (node == null)
-                    continue;   // this computer is gone
-                byName.put(node.getNodeName(),c);
-            }
+    protected void updateComputerList(final boolean automaticSlaveLaunch) {
+        final Map<Node,Computer> computers = getComputerMap();
+        final Set<Computer> old = new HashSet<Computer>(computers.size());
+        Queue.withLock(new Runnable() {
+            @Override
+            public void run() {
+                Map<String,Computer> byName = new HashMap<String,Computer>();
+                for (Computer c : computers.values()) {
+                    old.add(c);
+                    Node node = c.getNode();
+                    if (node == null)
+                        continue;   // this computer is gone
+                    byName.put(node.getNodeName(),c);
+                }
 
-            final Set<Computer> old = new HashSet<Computer>(computers.values());
-            Set<Computer> used = new HashSet<Computer>();
+                Set<Computer> used = new HashSet<Computer>(old.size());
 
-            updateComputer(this, byName, used, automaticSlaveLaunch);
-            for (Node s : getNodes()) {
-                long start = System.currentTimeMillis();
-                updateComputer(s, byName, used, automaticSlaveLaunch);
-                if(LOG_STARTUP_PERFORMANCE)
-                    LOGGER.info(String.format("Took %dms to update node %s",
-                            System.currentTimeMillis()-start, s.getNodeName()));
-            }
+                updateComputer(AbstractCIBase.this, byName, used, automaticSlaveLaunch);
+                for (Node s : getNodes()) {
+                    long start = System.currentTimeMillis();
+                    updateComputer(s, byName, used, automaticSlaveLaunch);
+                    if(LOG_STARTUP_PERFORMANCE)
+                        LOGGER.info(String.format("Took %dms to update node %s",
+                                System.currentTimeMillis()-start, s.getNodeName()));
+                }
 
-            // find out what computers are removed, and kill off all executors.
-            // when all executors exit, it will be removed from the computers map.
-            // so don't remove too quickly
-            old.removeAll(used);
-            for (Computer c : old) {
-                killComputer(c);
+                // find out what computers are removed, and kill off all executors.
+                // when all executors exit, it will be removed from the computers map.
+                // so don't remove too quickly
+                old.removeAll(used);
+                // we need to start the process of reducing the executors on all computers as distinct
+                // from the killing action which should not excessively use the Queue lock.
+                for (Computer c : old) {
+                    c.inflictMortalWound();
+                }
             }
+        });
+        for (Computer c : old) {
+            // when we get to here, the number of executors should be zero so this call should not need the Queue.lock
+            killComputer(c);
         }
         getQueue().scheduleMaintenance();
         for (ComputerListener cl : ComputerListener.all())

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

@@ -45,6 +45,7 @@ import hudson.security.AccessControlled;
 import hudson.security.Permission;
 import hudson.security.PermissionGroup;
 import hudson.security.PermissionScope;
+import hudson.slaves.AbstractCloudSlave;
 import hudson.slaves.ComputerLauncher;
 import hudson.slaves.ComputerListener;
 import hudson.slaves.NodeProperty;
@@ -63,8 +64,11 @@ import hudson.util.NamingThreadFactory;
 import jenkins.model.Jenkins;
 import jenkins.util.ContextResettingExecutorService;
 import jenkins.security.MasterToSlaveCallable;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.args4j.Argument;
 import org.kohsuke.args4j.CmdLineException;
+import org.kohsuke.stapler.Stapler;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.QueryParameter;
@@ -77,6 +81,7 @@ import org.kohsuke.stapler.export.ExportedBean;
 import org.kohsuke.args4j.Option;
 import org.kohsuke.stapler.interceptor.RequirePOST;
 
+import javax.annotation.concurrent.GuardedBy;
 import javax.servlet.ServletException;
 import java.io.File;
 import java.io.FilenameFilter;
@@ -176,6 +181,58 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces
 
     protected final Object statusChangeLock = new Object();
 
+    /**
+     * Keeps track of stack traces to track the tremination requests for this computer.
+     *
+     * @since 1.FIXME
+     * @see Executor#resetWorkUnit(String)
+     */
+    private transient final List<TerminationRequest> terminatedBy = Collections.synchronizedList(new ArrayList
+            <TerminationRequest>());
+
+    /**
+     * This method captures the information of a request to terminate a computer instance. Method is public as
+     * it needs to be called from {@link AbstractCloudSlave} and {@link jenkins.model.Nodes}. In general you should
+     * not need to call this method directly, however if implementing a custom node type or a different path
+     * for removing nodes, it may make sense to call this method in order to capture the originating request.
+     *
+     * @since 1.FIXME
+     */
+    public void recordTermination() {
+        StaplerRequest request = Stapler.getCurrentRequest();
+        if (request != null) {
+            terminatedBy.add(new TerminationRequest(
+                    String.format("Termination requested at %s by %s [id=%d] from HTTP request for %s",
+                            new Date(),
+                            Thread.currentThread(),
+                            Thread.currentThread().getId(),
+                            request.getRequestURL()
+                    )
+            ));
+        } else {
+            terminatedBy.add(new TerminationRequest(
+                    String.format("Termination requested at %s by %s [id=%d]",
+                            new Date(),
+                            Thread.currentThread(),
+                            Thread.currentThread().getId()
+                    )
+            ));
+        }
+    }
+
+    /**
+     * Returns the list of captured termination requests for this Computer. This method is used by {@link Executor}
+     * to provide details on why a Computer was removed in-between work being scheduled against the {@link Executor}
+     * and the {@link Executor} starting to execute the task.
+     *
+     * @return the (possibly empty) list of termination requests.
+     * @see Executor#resetWorkUnit(String)
+     * @since 1.FIXME
+     */
+    public List<TerminationRequest> getTerminatedBy() {
+        return new ArrayList<TerminationRequest>(terminatedBy);
+    }
+
     public Computer(Node node) {
         setNode(node);
     }
@@ -404,6 +461,7 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces
      * @since 1.320
      */
     public Future<?> disconnect(OfflineCause cause) {
+        recordTermination();
         offlineCause = cause;
         if (Util.isOverridden(Computer.class,getClass(),"disconnect"))
             return disconnect();    // legacy subtypes that extend disconnect().
@@ -419,6 +477,7 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces
      *      Use {@link #disconnect(OfflineCause)} and specify the cause.
      */
     public Future<?> disconnect() {
+        recordTermination();
         if (Util.isOverridden(Computer.class,getClass(),"disconnect",OfflineCause.class))
             // if the subtype already derives disconnect(OfflineCause), delegate to it
             return disconnect(null);
@@ -442,7 +501,7 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces
     @CLIMethod(name="offline-node")
     public void cliOffline(@Option(name="-m",usage="Record the note about why you are disconnecting this node") String cause) throws ExecutionException, InterruptedException {
         checkPermission(DISCONNECT);
-        setTemporarilyOffline(true,new ByCLI(cause));
+        setTemporarilyOffline(true, new ByCLI(cause));
     }
 
     @CLIMethod(name="online-node")
@@ -698,6 +757,26 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces
      * @see #onRemoved()
      */
     protected void kill() {
+        // On most code paths, this should already be zero, and thus this next call becomes a no-op... and more
+        // importantly it will not acquire a lock on the Queue... not that the lock is bad, more that the lock
+        // may delay unnecessarily
+        setNumExecutors(0);
+    }
+
+    /**
+     * Called by {@link Jenkins#updateComputerList()} to notify {@link Computer} that it will be discarded.
+     *
+     * <p>
+     * Note that at this point {@link #getNode()} returns null.
+     *
+     * <p>
+     * Note that the Queue lock is already held when this method is called.
+     *
+     * @see #onRemoved()
+     */
+    @Restricted(NoExternalUse.class)
+    @GuardedBy("hudson.model.Queue.lock")
+    /*package*/ void inflictMortalWound() {
         setNumExecutors(0);
     }
 
@@ -808,6 +887,28 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces
         return new ArrayList<OneOffExecutor>(oneOffExecutors);
     }
 
+    /**
+     * Used to render the list of executors.
+     * @return a snapshot of the executor display information
+     * @since 1.FIXME
+     */
+    @Restricted(NoExternalUse.class)
+    public List<DisplayExecutor> getDisplayExecutors() {
+        // The size may change while we are populating, but let's start with a reasonable guess to minimize resizing
+        List<DisplayExecutor> result = new ArrayList<DisplayExecutor>(executors.size()+oneOffExecutors.size());
+        int index = 0;
+        for (Executor e: executors) {
+            result.add(new DisplayExecutor(Integer.toString(index+1), String.format("executors/%d", index), e));
+            index++;
+        }
+        index = 0;
+        for (OneOffExecutor e: oneOffExecutors) {
+            result.add(new DisplayExecutor("", String.format("oneOffExecutors/%d", index), e));
+            index++;
+        }
+        return result;
+    }
+
     /**
      * Returns true if all the executors of this computer are idle.
      */
@@ -867,14 +968,21 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces
     /**
      * Called by {@link Executor} to kill excessive executors from this computer.
      */
-    /*package*/ synchronized void removeExecutor(Executor e) {
-        executors.remove(e);
-        addNewExecutorIfNecessary();
-        if(!isAlive()) // TODO except from interrupt/doYank this is called while the executor still isActive(), so how could !this.isAlive()?
-        {
-            AbstractCIBase ciBase = Jenkins.getInstance();
-            ciBase.removeComputer(this);
-        }
+    /*package*/ void removeExecutor(final Executor e) {
+        Queue.withLock(new Runnable() {
+            @Override
+            public void run() {
+                synchronized (Computer.this) {
+                    executors.remove(e);
+                    addNewExecutorIfNecessary();
+                    if (!isAlive()) // TODO except from interrupt/doYank this is called while the executor still isActive(), so how could !this.isAlive()?
+                    {
+                        AbstractCIBase ciBase = Jenkins.getInstance();
+                        ciBase.removeComputer(Computer.this);
+                    }
+                }
+            }
+        });
     }
 
     /**
@@ -1122,12 +1230,13 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces
     public void doRssAll( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
         rss(req, rsp, " all builds", getBuilds());
     }
-    public void doRssFailed( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
+
+    public void doRssFailed(StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
         rss(req, rsp, " failed builds", getBuilds().failureOnly());
     }
     private void rss(StaplerRequest req, StaplerResponse rsp, String suffix, RunList runs) throws IOException, ServletException {
-        RSS.forwardToRss(getDisplayName()+ suffix, getUrl(),
-            runs.newBuilds(), Run.FEED_ADAPTER, req, rsp );
+        RSS.forwardToRss(getDisplayName() + suffix, getUrl(),
+                runs.newBuilds(), Run.FEED_ADAPTER, req, rsp);
     }
 
     @RequirePOST
@@ -1205,7 +1314,7 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces
         _doScript(req, rsp, "_scriptText.jelly");
     }
 
-    protected void _doScript( StaplerRequest req, StaplerResponse rsp, String view) throws IOException, ServletException {
+    protected void _doScript(StaplerRequest req, StaplerResponse rsp, String view) throws IOException, ServletException {
         Jenkins._doScript(req, rsp, req.getView(this, view), getChannel(), getACL());
     }
 
@@ -1329,7 +1438,7 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces
      * Handles incremental log.
      */
     public void doProgressiveLog( StaplerRequest req, StaplerResponse rsp) throws IOException {
-        getLogText().doProgressText(req,rsp);
+        getLogText().doProgressText(req, rsp);
     }
 
     /**
@@ -1414,6 +1523,101 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces
         }
     }
 
+    /**
+     * A value class to provide a consistent snapshot view of the state of an executor to avoid race conditions
+     * during rendering of the executors list.
+     *
+     * @since 1.FIXME
+     */
+    @Restricted(NoExternalUse.class)
+    public static class DisplayExecutor implements ModelObject {
+
+        @Nonnull
+        private final String displayName;
+        @Nonnull
+        private final String url;
+        @Nonnull
+        private final Executor executor;
+
+        public DisplayExecutor(@Nonnull String displayName, @Nonnull String url, @Nonnull Executor executor) {
+            this.displayName = displayName;
+            this.url = url;
+            this.executor = executor;
+        }
+
+        @Override
+        @Nonnull
+        public String getDisplayName() {
+            return displayName;
+        }
+
+        @Nonnull
+        public String getUrl() {
+            return url;
+        }
+
+        @Nonnull
+        public Executor getExecutor() {
+            return executor;
+        }
+
+        @Override
+        public String toString() {
+            final StringBuilder sb = new StringBuilder("DisplayExecutor{");
+            sb.append("displayName='").append(displayName).append('\'');
+            sb.append(", url='").append(url).append('\'');
+            sb.append(", executor=").append(executor);
+            sb.append('}');
+            return sb.toString();
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) {
+                return true;
+            }
+            if (o == null || getClass() != o.getClass()) {
+                return false;
+            }
+
+            DisplayExecutor that = (DisplayExecutor) o;
+
+            if (!executor.equals(that.executor)) {
+                return false;
+            }
+
+            return true;
+        }
+
+        @Override
+        public int hashCode() {
+            return executor.hashCode();
+        }
+    }
+
+    /**
+     * Used to trace requests to terminate a computer.
+     *
+     * @since 1.FIXME
+     */
+    public static class TerminationRequest extends RuntimeException {
+        private final long when;
+        public TerminationRequest(String message) {
+            super(message);
+            this.when = System.currentTimeMillis();
+        }
+
+        /**
+         * Returns the when the termination request was created.
+         *
+         * @return the difference, measured in milliseconds, between
+         * the time of the termination request and midnight, January 1, 1970 UTC.
+         */
+        public long getWhen() {
+            return when;
+        }
+    }
+
     public static final PermissionGroup PERMISSIONS = new PermissionGroup(Computer.class,Messages._Computer_Permissions_Title());
     public static final Permission CONFIGURE = new Permission(PERMISSIONS,"Configure", Messages._Computer_ConfigurePermission_Description(), Permission.CONFIGURE, PermissionScope.COMPUTER);
     /**

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

@@ -121,7 +121,7 @@ public class Hudson extends Jenkins {
      *      Use {@link #getNodes()}. Since 1.252.
      */
     public List<Slave> getSlaves() {
-        return (List)slaves;
+        return (List)getNodes();
     }
 
     /**

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

@@ -373,6 +373,10 @@ public abstract class Node extends AbstractModelObject implements Reconfigurable
             if (c!=null)    return c;
         }
 
+        if (!isAcceptingTasks()) {
+            return CauseOfBlockage.fromMessage(Messages._Node_BecauseNodeIsNotAcceptingTasks(getNodeName()));
+        }
+
         // Looks like we can take the task
         return null;
     }

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

 /*
  * The MIT License
- * 
+ *
  * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi
- * 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
  * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
  * copies of the Software, and to permit persons to whom the Software is
  * furnished to do so, subject to the following conditions:
- * 
+ *
  * The above copyright notice and this permission notice shall be included in
  * all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -29,6 +29,7 @@ import java.util.Set;
 import java.util.Collection;
 import java.util.AbstractCollection;
 import java.util.Iterator;
+import java.util.concurrent.Callable;
 import java.util.concurrent.CopyOnWriteArraySet;
 import javax.annotation.Nonnull;
 
@@ -61,7 +62,7 @@ public class ResourceController {
 
     /**
      * Union of all {@link Resource}s that are currently in use.
-     * Updated as a task starts/completes executing. 
+     * Updated as a task starts/completes executing.
      */
     private ResourceList inUse = ResourceList.EMPTY;
 
@@ -74,26 +75,37 @@ public class ResourceController {
      * @throws InterruptedException
      *      the thread can be interrupted while waiting for the available resources.
      */
-    public void execute(@Nonnull Runnable task, ResourceActivity activity ) throws InterruptedException {
-        ResourceList resources = activity.getResourceList();
-        synchronized(this) {
-            while(inUse.isCollidingWith(resources))
-                wait();
-
-            // we have a go
-            inProgress.add(activity);
-            inUse = ResourceList.union(inUse,resources);
-        }
+    public void execute(@Nonnull Runnable task, final ResourceActivity activity ) throws InterruptedException {
+        final ResourceList resources = activity.getResourceList();
+        _withLock(new Runnable() {
+            @Override
+            public void run() {
+                while(inUse.isCollidingWith(resources))
+                    try {
+                        // TODO revalidate the resource list after re-acquiring lock, for now we just let the build fail
+                        _await();
+                    } catch (InterruptedException e) {
+                        throw new RuntimeException(e);
+                    }
+
+                // we have a go
+                inProgress.add(activity);
+                inUse = ResourceList.union(inUse,resources);
+            }
+        });
 
         try {
             task.run();
         } finally {
-            // TODO if AsynchronousExecution, do that later
-            synchronized(this) {
-                inProgress.remove(activity);
-                inUse = ResourceList.union(resourceView);
-                notifyAll();
-            }
+           // TODO if AsynchronousExecution, do that later
+            _withLock(new Runnable() {
+                @Override
+                public void run() {
+                    inProgress.remove(activity);
+                    inUse = ResourceList.union(resourceView);
+                    _signalAll();
+                }
+            });
         }
     }
 
@@ -106,8 +118,17 @@ public class ResourceController {
      * another activity might acquire resources before the caller
      * gets to call {@link #execute(Runnable, ResourceActivity)}.
      */
-    public synchronized boolean canRun(ResourceList resources) {
-        return !inUse.isCollidingWith(resources);
+    public boolean canRun(final ResourceList resources) {
+        try {
+            return _withLock(new Callable<Boolean>() {
+                @Override
+                public Boolean call() {
+                    return !inUse.isCollidingWith(resources);
+                }
+            });
+        } catch (Exception e) {
+            throw new IllegalStateException("Inner callable does not throw exception");
+        }
     }
 
     /**
@@ -118,8 +139,17 @@ public class ResourceController {
      * If more than one such resource exists, one is chosen and returned.
      * This method is used for reporting what's causing the blockage.
      */
-    public synchronized Resource getMissingResource(ResourceList resources) {
-        return resources.getConflict(inUse);
+    public Resource getMissingResource(final ResourceList resources) {
+        try {
+            return _withLock(new Callable<Resource>() {
+                @Override
+                public Resource call() {
+                    return resources.getConflict(inUse);
+                }
+            });
+        } catch (Exception e) {
+            throw new IllegalStateException("Inner callable does not throw exception");
+        }
     }
 
     /**
@@ -134,5 +164,25 @@ public class ResourceController {
                 return a;
         return null;
     }
+
+    protected void _await() throws InterruptedException {
+        wait();
+    }
+
+    protected void _signalAll() {
+        notifyAll();
+    }
+
+    protected void _withLock(Runnable runnable) {
+        synchronized (this) {
+            runnable.run();
+        }
+    }
+
+    protected <V> V _withLock(java.util.concurrent.Callable<V> callable) throws Exception {
+        synchronized (this) {
+            return callable.call();
+        }
+    }
 }
 

core/src/main/java/hudson/slaves/AbstractCloudSlave.java

@@ -23,6 +23,7 @@
  */
 package hudson.slaves;
 
+import hudson.model.Computer;
 import hudson.model.Descriptor.FormException;
 import jenkins.model.Jenkins;
 import hudson.model.Slave;
@@ -57,6 +58,10 @@ public abstract class AbstractCloudSlave extends Slave {
      * Releases and removes this slave.
      */
     public void terminate() throws InterruptedException, IOException {
+        final Computer computer = toComputer();
+        if (computer != null) {
+            computer.recordTermination();
+        }
         try {
             // TODO: send the output to somewhere real
             _terminate(new StreamTaskListener(System.out, Charset.defaultCharset()));

core/src/main/java/hudson/slaves/CloudRetentionStrategy.java

@@ -28,6 +28,7 @@ import hudson.model.Node;
 import hudson.model.Queue;
 import jenkins.model.Jenkins;
 
+import javax.annotation.concurrent.GuardedBy;
 import java.io.IOException;
 import java.util.logging.Logger;
 
@@ -50,30 +51,20 @@ public class CloudRetentionStrategy extends RetentionStrategy<AbstractCloudCompu
     }
 
     @Override
-    public synchronized long check(final AbstractCloudComputer c) {
+    @GuardedBy("hudson.model.Queue.lock")
+    public long check(final AbstractCloudComputer c) {
         final AbstractCloudSlave computerNode = c.getNode();
         if (c.isIdle() && !disabled && computerNode != null) {
             final long idleMilliseconds = System.currentTimeMillis() - c.getIdleStartMilliseconds();
             if (idleMilliseconds > MINUTES.toMillis(idleMinutes)) {
-                Queue.withLock(new Runnable() {
-                    @Override
-                    public void run() {
-                        // re-check idle now that we are within the Queue lock
-                        if (c.isIdle()) {
-                            final long idleMilliseconds = System.currentTimeMillis() - c.getIdleStartMilliseconds();
-                            if (idleMilliseconds > MINUTES.toMillis(idleMinutes)) {
-                                LOGGER.log(Level.INFO, "Disconnecting {0}", c.getName());
-                                try {
-                                    computerNode.terminate();
-                                } catch (InterruptedException e) {
-                                    LOGGER.log(WARNING, "Failed to terminate " + c.getName(), e);
-                                } catch (IOException e) {
-                                    LOGGER.log(WARNING, "Failed to terminate " + c.getName(), e);
-                                }
-                            }
-                        }
-                    }
-                });
+                LOGGER.log(Level.INFO, "Disconnecting {0}", c.getName());
+                try {
+                    computerNode.terminate();
+                } catch (InterruptedException e) {
+                    LOGGER.log(WARNING, "Failed to terminate " + c.getName(), e);
+                } catch (IOException e) {
+                    LOGGER.log(WARNING, "Failed to terminate " + c.getName(), e);
+                }
             }
         }
         return 1;

core/src/main/java/hudson/slaves/CloudSlaveRetentionStrategy.java

@@ -5,6 +5,7 @@ import hudson.model.Node;
 import hudson.util.TimeUnit2;
 import jenkins.model.Jenkins;
 
+import javax.annotation.concurrent.GuardedBy;
 import java.io.IOException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -22,6 +23,7 @@ import java.util.logging.Logger;
 public class CloudSlaveRetentionStrategy<T extends Computer> extends RetentionStrategy<T> {
 
     @Override
+    @GuardedBy("hudson.model.Queue.lock")
     public long check(T c) {
         if (!c.isConnecting() && c.isAcceptingTasks()) {
             if (isIdleForTooLong(c)) {

core/src/main/java/hudson/slaves/ComputerRetentionWork.java

 /*
  * The MIT License
- * 
+ *
  * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, Stephen Connolly
- * 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
  * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
  * copies of the Software, and to permit persons to whom the Software is
  * furnished to do so, subject to the following conditions:
- * 
+ *
  * The above copyright notice and this permission notice shall be included in
  * all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -27,6 +27,7 @@ import java.util.Map;
 import java.util.WeakHashMap;
 
 import hudson.model.Computer;
+import hudson.model.Queue;
 import jenkins.model.Jenkins;
 import hudson.model.Node;
 import hudson.model.PeriodicWork;
@@ -56,16 +57,21 @@ public class ComputerRetentionWork extends PeriodicWork {
     @SuppressWarnings("unchecked")
     protected void doRun() {
         final long startRun = System.currentTimeMillis();
-        for (Computer c : Jenkins.getInstance().getComputers()) {
-            Node n = c.getNode();
-            if (n!=null && n.isHoldOffLaunchUntilSave())
-                continue;
-            if (!nextCheck.containsKey(c) || startRun > nextCheck.get(c)) {
-                // at the moment I don't trust strategies to wait more than 60 minutes
-                // strategies need to wait at least one minute
-                final long waitInMins = Math.max(1, Math.min(60, c.getRetentionStrategy().check(c)));
-                nextCheck.put(c, startRun + waitInMins*1000*60 /*MINS->MILLIS*/);
-            }
+        for (final Computer c : Jenkins.getInstance().getComputers()) {
+            Queue.withLock(new Runnable() {
+                @Override
+                public void run() {
+                    Node n = c.getNode();
+                    if (n!=null && n.isHoldOffLaunchUntilSave())
+                        return;
+                    if (!nextCheck.containsKey(c) || startRun > nextCheck.get(c)) {
+                        // at the moment I don't trust strategies to wait more than 60 minutes
+                        // strategies need to wait at least one minute
+                        final long waitInMins = Math.max(1, Math.min(60, c.getRetentionStrategy().check(c)));
+                        nextCheck.put(c, startRun + waitInMins*1000*60 /*MINS->MILLIS*/);
+                    }
+                }
+            });
         }
     }
 }

core/src/main/java/hudson/slaves/NodeProvisioner.java

@@ -30,9 +30,9 @@ import jenkins.model.Jenkins;
 import static hudson.model.LoadStatistics.DECAY;
 import hudson.model.MultiStageTimeSeries.TimeScale;
 import hudson.Extension;
-import net.jcip.annotations.GuardedBy;
 
 import javax.annotation.Nonnull;
+import javax.annotation.concurrent.GuardedBy;
 import java.awt.Color;
 import java.util.Arrays;
 import java.util.concurrent.Future;
@@ -42,6 +42,8 @@ import java.util.Collection;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.logging.Logger;
 import java.util.logging.Level;
 import java.io.IOException;
@@ -120,9 +122,14 @@ public class NodeProvisioner {
      */
     private final Label label;
 
-    @GuardedBy("self")
+    @GuardedBy("provisioningLock")
     private final List<PlannedNode> pendingLaunches = new ArrayList<PlannedNode>();
 
+    private final Lock provisioningLock = new ReentrantLock();
+
+    @GuardedBy("provisioningLock")
+    private StrategyState provisioningState = null;
+
     private transient volatile long lastSuggestedReview;
 
     /**
@@ -148,8 +155,11 @@ public class NodeProvisioner {
      * @since 1.401
      */
     public List<PlannedNode> getPendingLaunches() {
-        synchronized (pendingLaunches) {
+        provisioningLock.lock();
+        try {
             return new ArrayList<PlannedNode>(pendingLaunches);
+        } finally {
+            provisioningLock.unlock();
         }
     }
 
@@ -173,81 +183,114 @@ public class NodeProvisioner {
     /**
      * Periodically invoked to keep track of the load.
      * Launches additional nodes if necessary.
+     *
+     * Note: This method will obtain a lock on {@link #provisioningLock} first (to ensure that one and only one
+     * instance of this provisioner is running at a time) and then a lock on {@link Queue#lock}
      */
-    private synchronized void update() {
-        Jenkins jenkins = Jenkins.getInstance();
-        lastSuggestedReview = System.currentTimeMillis();
+    private void update() {
+        provisioningLock.lock();
+        try {
+            lastSuggestedReview = System.currentTimeMillis();
 
-        // clean up the cancelled launch activity, then count the # of executors that we are about to bring up.
-        int plannedCapacitySnapshot = 0;
-        List<PlannedNode> completedLaunches = new ArrayList<PlannedNode>();
+            // We need to get the lock on Queue for two reasons:
+            // 1. We will potentially adding a lot of nodes and we don't want to fight with Queue#maintain to acquire
+            //    the Queue#lock in order to add each node. Much better is to hold the Queue#lock until all nodes
+            //    that were provisioned since last we checked have been added.
+            // 2. We want to know the idle executors count, which can only be measured if you hold the Queue#lock
+            //    Strictly speaking we don't need an accurate measure for this, but as we had to get the Queue#lock
+            //    anyway, we might as well get an accurate measure.
+            //
+            // We do not need the Queue#lock to get the count of items in the queue as that is a lock-free call
+            // Since adding a node should not (in principle) confuse Queue#maintain (it is only removal of nodes
+            // that causes issues in Queue#maintain) we should be able to remove the need for Queue#lock
+            //
+            // TODO once Nodes#addNode is made lock free, we should be able to remove the requirement for Queue#lock
+            Queue.withLock(new Runnable() {
+                @Override
+                public void run() {
+                    Jenkins jenkins = Jenkins.getInstance();
+                    // clean up the cancelled launch activity, then count the # of executors that we are about to
+                    // bring up.
+
+                    int plannedCapacitySnapshot = 0;
+                    List<PlannedNode> completedLaunches = new ArrayList<PlannedNode>();
+
+                    for (Iterator<PlannedNode> itr = pendingLaunches.iterator(); itr.hasNext(); ) {
+                        PlannedNode f = itr.next();
+                        if (f.future.isDone()) {
+                            completedLaunches.add(f);
+                            itr.remove();
+                        } else {
+                            plannedCapacitySnapshot += f.numExecutors;
+                        }
+                    }
 
-        synchronized (pendingLaunches) {
-            for (Iterator<PlannedNode> itr = pendingLaunches.iterator(); itr.hasNext(); ) {
-                PlannedNode f = itr.next();
-                if (f.future.isDone()) {
-                    completedLaunches.add(f);
-                    itr.remove();
-                } else {
-                    plannedCapacitySnapshot += f.numExecutors;
-                }
-            }
-        }
+                    for (PlannedNode f : completedLaunches) {
+                        try {
+                            Node node = f.future.get();
+                            for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
+                                cl.onComplete(f, node);
+                            }
 
-        for (PlannedNode f : completedLaunches) {
-            try {
-                Node node = f.future.get();
-                for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
-                    cl.onComplete(f, node);
-                }
+                            jenkins.addNode(node);
+                            LOGGER.log(Level.INFO,
+                                    "{0} provisioning successfully completed. We have now {1,number,integer} computer"
+                                            + "(s)",
+                                    new Object[]{f.displayName, jenkins.getComputers().length});
+                        } catch (InterruptedException e) {
+                            throw new AssertionError(e); // since we confirmed that the future is already done
+                        } catch (ExecutionException e) {
+                            LOGGER.log(Level.WARNING, "Provisioned slave " + f.displayName + " failed to launch",
+                                    e.getCause());
+                            for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
+                                cl.onFailure(f, e.getCause());
+                            }
+                        } catch (IOException e) {
+                            LOGGER.log(Level.WARNING, "Provisioned slave " + f.displayName + " failed to launch", e);
+                            for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
+                                cl.onFailure(f, e);
+                            }
+                        }
 
-                jenkins.addNode(node);
-                LOGGER.log(Level.INFO,
-                        "{0} provisioning successfully completed. We have now {1,number,integer} computer(s)",
-                        new Object[]{f.displayName, jenkins.getComputers().length});
-            } catch (InterruptedException e) {
-                throw new AssertionError(e); // since we confirmed that the future is already done
-            } catch (ExecutionException e) {
-                LOGGER.log(Level.WARNING, "Provisioned slave " + f.displayName + " failed to launch", e.getCause());
-                for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
-                    cl.onFailure(f, e.getCause());
-                }
-            } catch (IOException e) {
-                LOGGER.log(Level.WARNING, "Provisioned slave " + f.displayName + " failed to launch", e);
-                for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
-                    cl.onFailure(f, e);
-                }
-            }
+                        f.spent();
+                    }
 
-            f.spent();
-        }
+                    float plannedCapacity = plannedCapacitySnapshot;
+                    plannedCapacitiesEMA.update(plannedCapacity);
+
+                    int idleSnapshot = stat.computeIdleExecutors();
+                    int queueLengthSnapshot = stat.computeQueueLength();
+
+                    if (queueLengthSnapshot <= idleSnapshot) {
+                        LOGGER.log(Level.FINE,
+                                "Queue length {0} is less than the idle capacity {1}. No provisioning strategy "
+                                        + "required",
+                                new Object[]{queueLengthSnapshot, idleSnapshot});
+                        provisioningState = null;
+                    } else {
+                        provisioningState = new StrategyState(queueLengthSnapshot, label, idleSnapshot,
+                                stat.computeTotalExecutors(),
+                                plannedCapacitySnapshot);
+                    }
+                }
+            });
 
-        float plannedCapacity = plannedCapacitySnapshot;
-        plannedCapacitiesEMA.update(plannedCapacity);
-
-        int idleSnapshot = stat.computeIdleExecutors();
-        int queueLengthSnapshot = stat.computeQueueLength();
-
-        if (queueLengthSnapshot <= idleSnapshot) {
-            LOGGER.log(Level.FINE,
-                    "Queue length {0} is less than the idle capacity {1}. No provisioning strategy required",
-                    new Object[]{queueLengthSnapshot, idleSnapshot});
-        } else {
-            StrategyState state =
-                    new StrategyState(queueLengthSnapshot, label, idleSnapshot, stat.computeTotalExecutors(),
-                            plannedCapacitySnapshot);
-            List<Strategy> strategies = Jenkins.getInstance().getExtensionList(Strategy.class);
-            for (Strategy strategy : strategies.isEmpty()
-                    ? Arrays.<Strategy>asList(new StandardStrategyImpl())
-                    : strategies) {
-                LOGGER.log(Level.FINER, "Consulting {0} provisioning strategy with state {1}",
-                        new Object[]{strategy, state});
-                if (StrategyDecision.PROVISIONING_COMPLETED == strategy.apply(state)) {
-                    LOGGER.log(Level.FINER, "Provisioning strategy {0} declared provisioning complete",
-                            strategy);
-                    break;
+            if (provisioningState != null) {
+                List<Strategy> strategies = Jenkins.getInstance().getExtensionList(Strategy.class);
+                for (Strategy strategy : strategies.isEmpty()
+                        ? Arrays.<Strategy>asList(new StandardStrategyImpl())
+                        : strategies) {
+                    LOGGER.log(Level.FINER, "Consulting {0} provisioning strategy with state {1}",
+                            new Object[]{strategy, provisioningState});
+                    if (StrategyDecision.PROVISIONING_COMPLETED == strategy.apply(provisioningState)) {
+                        LOGGER.log(Level.FINER, "Provisioning strategy {0} declared provisioning complete",
+                                strategy);
+                        break;
+                    }
                 }
             }
+        } finally {
+            provisioningLock.unlock();
         }
     }
 
@@ -283,7 +326,7 @@ public class NodeProvisioner {
          * Called by {@link NodeProvisioner#update()} to apply this strategy against the specified state.
          * Any provisioning activities should be recorded by calling
          * {@link hudson.slaves.NodeProvisioner.StrategyState#recordPendingLaunches(java.util.Collection)}
-         * This method will be called by a thread that is holding a lock on {@link hudson.slaves.NodeProvisioner}
+         * This method will be called by a thread that is holding {@link hudson.slaves.NodeProvisioner#provisioningLock}
          * @param state the current state.
          * @return the decision.
          */
@@ -384,8 +427,13 @@ public class NodeProvisioner {
          * The additional planned capacity for this {@link #getLabel()} and provisioned by previous strategies during
          * the current updating of the {@link NodeProvisioner}.
          */
-        public synchronized int getAdditionalPlannedCapacity() {
-            return additionalPlannedCapacity;
+        public int getAdditionalPlannedCapacity() {
+            provisioningLock.lock();
+            try {
+                return additionalPlannedCapacity;
+            } finally {
+                provisioningLock.unlock();
+            }
         }
 
         /**
@@ -451,13 +499,14 @@ public class NodeProvisioner {
                     additionalPlannedCapacity += f.numExecutors;
                 }
             }
-            synchronized (pendingLaunches) {
+            provisioningLock.lock();
+            try {
                 pendingLaunches.addAll(plannedNodes);
-            }
-            if (additionalPlannedCapacity > 0) {
-                synchronized (this) {
-                    this.additionalPlannedCapacity += additionalPlannedCapacity;
+                if (additionalPlannedCapacity > 0) {
+                        this.additionalPlannedCapacity += additionalPlannedCapacity;
                 }
+            } finally {
+                provisioningLock.unlock();
             }
         }
 

core/src/main/java/hudson/slaves/RetentionStrategy.java

@@ -35,6 +35,8 @@ import java.util.HashMap;
 import jenkins.model.Jenkins;
 import org.kohsuke.stapler.DataBoundConstructor;
 
+import javax.annotation.concurrent.GuardedBy;
+import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -54,6 +56,7 @@ public abstract class RetentionStrategy<T extends Computer> extends AbstractDesc
      * @return The number of minutes after which the strategy would like to be checked again. The strategy may be
      *         rechecked earlier or later that this!
      */
+    @GuardedBy("hudson.model.Queue.lock")
     public abstract long check(T c);
 
     /**
@@ -91,8 +94,13 @@ public abstract class RetentionStrategy<T extends Computer> extends AbstractDesc
      *
      * @since 1.275
      */
-    public void start(T c) {
-        check(c);
+    public void start(final T c) {
+        Queue.withLock(new Runnable() {
+            @Override
+            public void run() {
+                check(c);
+            }
+        });
     }
 
     /**
@@ -113,6 +121,7 @@ public abstract class RetentionStrategy<T extends Computer> extends AbstractDesc
      * Dummy instance that doesn't do any attempt to retention.
      */
     public static final RetentionStrategy<Computer> NOOP = new RetentionStrategy<Computer>() {
+        @GuardedBy("hudson.model.Queue.lock")
         public long check(Computer c) {
             return 60;
         }
@@ -152,6 +161,7 @@ public abstract class RetentionStrategy<T extends Computer> extends AbstractDesc
         public Always() {
         }
 
+        @GuardedBy("hudson.model.Queue.lock")
         public long check(SlaveComputer c) {
             if (c.isOffline() && !c.isConnecting() && c.isLaunchSupported())
                 c.tryReconnect();
@@ -208,7 +218,8 @@ public abstract class RetentionStrategy<T extends Computer> extends AbstractDesc
         }
 
         @Override
-        public synchronized long check(final SlaveComputer c) {
+        @GuardedBy("hudson.model.Queue.lock")
+        public long check(final SlaveComputer c) {
             if (c.isOffline() && c.isLaunchSupported()) {
                 final HashMap<Computer, Integer> availableComputers = new HashMap<Computer, Integer>();
                 for (Computer o : Jenkins.getInstance().getComputers()) {
@@ -257,21 +268,13 @@ public abstract class RetentionStrategy<T extends Computer> extends AbstractDesc
             } else if (c.isIdle()) {
                 final long idleMilliseconds = System.currentTimeMillis() - c.getIdleStartMilliseconds();
                 if (idleMilliseconds > idleDelay * 1000 * 60 /*MINS->MILLIS*/) {
-                    Queue.withLock(new Runnable() {
-                        @Override
-                        public void run() {
-                            // re-check idle now that we are within the Queue lock
-                            if (c.isIdle()) {
-                                final long idleMilliseconds = System.currentTimeMillis() - c.getIdleStartMilliseconds();
-                                if (idleMilliseconds > idleDelay * 1000 * 60 /*MINS->MILLIS*/) {
-                                    // we've been idle for long enough
-                                    logger.log(Level.INFO, "Disconnecting computer {0} as it has been idle for {1}",
-                                            new Object[]{c.getName(), Util.getTimeSpanString(idleMilliseconds)});
-                                    c.disconnect(OfflineCause.create(Messages._RetentionStrategy_Demand_OfflineIdle()));
-                                }
-                            }
-                        }
-                    });
+                    // we've been idle for long enough
+                    logger.log(Level.INFO, "Disconnecting computer {0} as it has been idle for {1}",
+                            new Object[]{c.getName(), Util.getTimeSpanString(idleMilliseconds)});
+                    c.disconnect(OfflineCause.create(Messages._RetentionStrategy_Demand_OfflineIdle()));
+                } else {
+                    // no point revisiting until we can be confident we will be idle
+                    return TimeUnit.MILLISECONDS.toMinutes(TimeUnit.MINUTES.toMillis(idleDelay) - idleMilliseconds);
                 }
             }
             return 1;

core/src/main/java/hudson/slaves/SimpleScheduledRetentionStrategy.java

@@ -34,6 +34,7 @@ import hudson.util.FormValidation;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
 
+import javax.annotation.concurrent.GuardedBy;
 import java.io.InvalidObjectException;
 import java.io.ObjectStreamException;
 import java.util.Calendar;
@@ -163,6 +164,7 @@ public class SimpleScheduledRetentionStrategy extends RetentionStrategy<SlaveCom
         return isOnlineScheduled();
     }
 
+    @GuardedBy("hudson.model.Queue.lock")
     public synchronized long check(final SlaveComputer c) {
         boolean shouldBeOnline = isOnlineScheduled();
         LOGGER.log(Level.FINE, "Checking computer {0} against schedule. online = {1}, shouldBeOnline = {2}",

core/src/main/java/hudson/slaves/SlaveComputer.java

@@ -678,7 +678,7 @@ public class SlaveComputer extends Computer {
     }
 
     @Override
-    protected void setNode(Node node) {
+    protected void setNode(final Node node) {
         super.setNode(node);
         launcher = grabLauncher(node);
 
@@ -686,10 +686,16 @@ public class SlaveComputer extends Computer {
         // "constructed==null" test is an ugly hack to avoid launching before the object is fully
         // constructed.
         if(constructed!=null) {
-            if (node instanceof Slave)
-                ((Slave)node).getRetentionStrategy().check(this);
-            else
+            if (node instanceof Slave) {
+                Queue.withLock(new Runnable() {
+                    @Override
+                    public void run() {
+                        ((Slave)node).getRetentionStrategy().check(SlaveComputer.this);
+                    }
+                });
+            } else {
                 connect(false);
+            }
         }
     }
 

core/src/main/java/jenkins/model/Jenkins.java

@@ -288,6 +288,7 @@ import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.logging.Level;
 import java.util.logging.LogRecord;
 import java.util.logging.Logger;
@@ -497,17 +498,18 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
     }
 
     /**
-     * Set of installed cluster nodes.
-     * <p>
-     * We use this field with copy-on-write semantics.
-     * This field has mutable list (to keep the serialization look clean),
-     * but it shall never be modified. Only new completely populated slave
-     * list can be set here.
-     * <p>
-     * The field name should be really {@code nodes}, but again the backward compatibility
-     * prevents us from renaming.
+     * Legacy store of the set of installed cluster nodes.
+     * @deprecated in favour of {@link Nodes}
+     */
+    @Deprecated
+    protected transient volatile NodeList slaves;
+
+    /**
+     * The holder of the set of installed cluster nodes.
+     *
+     * @since 1.FIXME
      */
-    protected volatile NodeList slaves;
+    private transient final Nodes nodes = new Nodes(this);
 
     /**
      * Quiet period.
@@ -1213,7 +1215,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
         return null;
     }
 
-    protected void updateComputerList() throws IOException {
+    protected void updateComputerList() {
         updateComputerList(AUTOMATIC_SLAVE_LAUNCH);
     }
 
@@ -1662,7 +1664,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
      * Gets the slave node of the give name, hooked under this Hudson.
      */
     public @CheckForNull Node getNode(String name) {
-        return slaves.getNode(name);
+        return nodes.getNode(name);
     }
 
     /**
@@ -1681,38 +1683,25 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
      * represents the master.
      */
     public List<Node> getNodes() {
-        return slaves;
+        return nodes.getNodes();
     }
 
     /**
      * Adds one more {@link Node} to Hudson.
      */
-    public synchronized void addNode(Node n) throws IOException {
-        if(n==null)     throw new IllegalArgumentException();
-        ArrayList<Node> nl = new ArrayList<Node>(this.slaves);
-        if(!nl.contains(n)) // defensive check
-            nl.add(n);
-        setNodes(nl);
+    public void addNode(Node n) throws IOException {
+        nodes.addNode(n);
     }
 
     /**
      * Removes a {@link Node} from Hudson.
      */
-    public synchronized void removeNode(@Nonnull Node n) throws IOException {
-        Computer c = n.toComputer();
-        if (c!=null)
-            c.disconnect(OfflineCause.create(Messages._Hudson_NodeBeingRemoved()));
-
-        ArrayList<Node> nl = new ArrayList<Node>(this.slaves);
-        nl.remove(n);
-        setNodes(nl);
+    public void removeNode(@Nonnull Node n) throws IOException {
+        nodes.removeNode(n);
     }
 
-    public void setNodes(final List<? extends Node> nodes) throws IOException {
-        Jenkins.this.slaves = new NodeList(nodes);
-        updateComputerList();
-        trimLabels();
-        save();
+    public void setNodes(final List<? extends Node> n) throws IOException {
+        nodes.setNodes(n);
     }
 
     public DescribableList<NodeProperty<?>, NodePropertyDescriptor> getNodeProperties() {
@@ -1729,7 +1718,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
      * This should be called when the assumptions behind label cache computation changes,
      * but we also call this periodically to self-heal any data out-of-sync issue.
      */
-    private void trimLabels() {
+    /*package*/ void trimLabels() {
         for (Iterator<Label> itr = labels.values().iterator(); itr.hasNext();) {
             Label l = itr.next();
             resetLabel(l);
@@ -2318,8 +2307,11 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
     }
 
     public void setNumExecutors(int n) throws IOException {
-        this.numExecutors = n;
-        save();
+        if (this.numExecutors != n) {
+            this.numExecutors = n;
+            updateComputerList();
+            save();
+        }
     }
 
 
@@ -2626,10 +2618,6 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
         TaskGraphBuilder g = new TaskGraphBuilder();
         Handle loadHudson = g.requires(EXTENSIONS_AUGMENTED).attains(JOB_LOADED).add("Loading global config", new Executable() {
             public void run(Reactor session) throws Exception {
-                // JENKINS-8043: some slaves (eg. swarm slaves) are not saved into the config file
-                // and will get overwritten when reloading. Make a backup copy now, and re-add them later
-                NodeList oldSlaves = slaves;
-
                 XmlFile cfg = getConfigFile();
                 if (cfg.exists()) {
                     // reset some data that may not exist in the disk file
@@ -2642,23 +2630,14 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
                 }
 
                 // if we are loading old data that doesn't have this field
-                if (slaves == null) slaves = new NodeList();
+                if (slaves != null && !slaves.isEmpty() && nodes.isLegacy()) {
+                    nodes.setNodes(slaves);
+                    slaves = null;
+                } else {
+                    nodes.load();
+                }
 
                 clouds.setOwner(Jenkins.this);
-
-                // JENKINS-8043: re-add the slaves which were not saved into the config file
-                // and are now missing, but still connected.
-                if (oldSlaves != null) {
-                    ArrayList<Node> newSlaves = new ArrayList<Node>(slaves);
-                    for (Node n: oldSlaves) {
-                        if (n instanceof EphemeralNode) {
-                            if(!newSlaves.contains(n)) {
-                                newSlaves.add(n);
-                            }
-                        }
-                    }
-                    setNodes(newSlaves);
-                }
             }
         });
 
@@ -2696,7 +2675,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
                 rebuildDependencyGraph();
 
                 {// recompute label objects - populates the labels mapping.
-                    for (Node slave : slaves)
+                    for (Node slave : nodes.getNodes())
                         // Note that not all labels are visible until the slaves have connected.
                         slave.getAssignedLabels();
                     getAssignedLabels();

core/src/main/java/jenkins/model/Nodes.java

+/*
+ * The MIT License
+ *
+ * Copyright (c) 2015, CloudBees, Inc., Stephen Connolly
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package jenkins.model;
+
+import hudson.BulkChange;
+import hudson.Util;
+import hudson.XmlFile;
+import hudson.model.Computer;
+import hudson.model.ItemGroupMixIn;
+import hudson.model.Node;
+import hudson.model.Queue;
+import hudson.model.Saveable;
+import hudson.model.listeners.SaveableListener;
+import hudson.slaves.EphemeralNode;
+import hudson.slaves.OfflineCause;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+import java.io.File;
+import java.io.FileFilter;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ConcurrentSkipListMap;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+/**
+ * Manages all the nodes for Jenkins.
+ *
+ * @since 1.FIXME
+ */
+@Restricted(NoExternalUse.class) // for now, we may make it public later
+public class Nodes implements Saveable {
+
+    /**
+     * The {@link Jenkins} instance that we are tracking nodes for.
+     */
+    @Nonnull
+    private final Jenkins jenkins;
+
+    /**
+     * The map of nodes.
+     */
+    private final ConcurrentMap<String, Node> nodes = new ConcurrentSkipListMap<String, Node>();
+
+    /**
+     * Constructor, intended to be called only from {@link Jenkins}.
+     *
+     * @param jenkins A reference to the {@link Jenkins} that this instance is tracking nodes for, beware not to
+     *                let this reference escape from a partially constructed {@link Nodes} as when we are passed the
+     *                reference the {@link Jenkins} instance has not completed instantiation.
+     */
+    /*package*/ Nodes(@Nonnull Jenkins jenkins) {
+        this.jenkins = jenkins;
+    }
+
+    /**
+     * Returns the list of nodes.
+     *
+     * @return the list of nodes.
+     */
+    @Nonnull
+    public List<Node> getNodes() {
+        return new ArrayList<Node>(nodes.values());
+    }
+
+    /**
+     * Sets the list of nodes.
+     *
+     * @param nodes the new list of nodes.
+     * @throws IOException if the new list of nodes could not be persisted.
+     */
+    public void setNodes(final @Nonnull Collection<? extends Node> nodes) throws IOException {
+        Queue.withLock(new Runnable() {
+            @Override
+            public void run() {
+                Set<String> toRemove = new HashSet<String>(Nodes.this.nodes.keySet());
+                for (Node n : nodes) {
+                    final String name = n.getNodeName();
+                    toRemove.remove(name);
+                    Nodes.this.nodes.put(name, n);
+                }
+                Nodes.this.nodes.keySet().removeAll(toRemove); // directory clean up will be handled by save
+                jenkins.updateComputerList();
+                jenkins.trimLabels();
+            }
+        });
+        save();
+    }
+
+    /**
+     * Adds a node. If a node of the same name already exists then that node will be replaced.
+     *
+     * @param node the new node.
+     * @throws IOException if the list of nodes could not be persisted.
+     */
+    public void addNode(final @Nonnull Node node) throws IOException {
+        if (node != nodes.get(node.getNodeName())) {
+            // TODO we should not need to lock the queue for adding nodes but until we have a way to update the
+            // computer list for just the new node
+            Queue.withLock(new Runnable() {
+                @Override
+                public void run() {
+                    nodes.put(node.getNodeName(), node);
+                    jenkins.updateComputerList();
+                    jenkins.trimLabels();
+                }
+            });
+            // no need for a full save() so we just do the minimum
+            if (node instanceof EphemeralNode) {
+                Util.deleteRecursive(new File(getNodesDir(), node.getNodeName()));
+            } else {
+                XmlFile xmlFile = new XmlFile(Jenkins.XSTREAM,
+                        new File(new File(getNodesDir(), node.getNodeName()), "config.xml"));
+                xmlFile.write(node);
+            }
+        }
+    }
+
+    /**
+     * Removes a node. If the node instance is not in the list of nodes, then this will be a no-op, even if
+     * there is another instance with the same {@link Node#getNodeName()}.
+     *
+     * @param node the node instance to remove.
+     * @throws IOException if the list of nodes could not be persisted.
+     */
+    public void removeNode(final @Nonnull Node node) throws IOException {
+        if (node == nodes.get(node.getNodeName())) {
+            Queue.withLock(new Runnable() {
+                @Override
+                public void run() {
+                    Computer c = node.toComputer();
+                    if (c != null) {
+                        c.recordTermination();
+                        c.disconnect(OfflineCause.create(hudson.model.Messages._Hudson_NodeBeingRemoved()));
+                    }
+                    if (node == nodes.remove(node.getNodeName())) {
+                        jenkins.updateComputerList();
+                        jenkins.trimLabels();
+                    }
+                }
+            });
+            // no need for a full save() so we just do the minimum
+            Util.deleteRecursive(new File(getNodesDir(), node.getNodeName()));
+        }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public void save() throws IOException {
+        if (BulkChange.contains(this)) {
+            return;
+        }
+        final File nodesDir = getNodesDir();
+        final Set<String> existing = new HashSet<String>();
+        for (Node n : nodes.values()) {
+            if (n instanceof EphemeralNode) {
+                continue;
+            }
+            existing.add(n.getNodeName());
+            XmlFile xmlFile = new XmlFile(Jenkins.XSTREAM, new File(new File(nodesDir, n.getNodeName()), "config.xml"));
+            xmlFile.write(n);
+            SaveableListener.fireOnChange(this, xmlFile);
+        }
+        for (File forDeletion : nodesDir.listFiles(new FileFilter() {
+            @Override
+            public boolean accept(File pathname) {
+                return pathname.isDirectory() && !existing.contains(pathname.getName());
+            }
+        })) {
+            Util.deleteRecursive(forDeletion);
+        }
+    }
+
+    /**
+     * Returns the named node.
+     *
+     * @param name the {@link Node#getNodeName()} of the node to retrieve.
+     * @return the {@link Node} or {@code null} if the node could not be found.
+     */
+    @CheckForNull
+    public Node getNode(String name) {
+        return nodes.get(name);
+    }
+
+    /**
+     * Loads the nodes from disk.
+     *
+     * @throws IOException if the nodes could not be deserialized.
+     */
+    public void load() throws IOException {
+        final File nodesDir = getNodesDir();
+        final File[] subdirs = nodesDir.listFiles(new FileFilter() {
+            public boolean accept(File child) {
+                return child.isDirectory();
+            }
+        });
+        final Map<String, Node> newNodes = new TreeMap<String, Node>();
+        if (subdirs != null) {
+            for (File subdir : subdirs) {
+                try {
+                    XmlFile xmlFile = new XmlFile(Jenkins.XSTREAM, new File(subdir, "config.xml"));
+                    if (xmlFile.exists()) {
+                        Node node = (Node) xmlFile.read();
+                        newNodes.put(node.getNodeName(), node);
+                    }
+                } catch (IOException e) {
+                    Logger.getLogger(ItemGroupMixIn.class.getName()).log(Level.WARNING, "could not load " + subdir, e);
+                }
+            }
+        }
+        Queue.withLock(new Runnable() {
+            @Override
+            public void run() {
+                for (Iterator<Map.Entry<String, Node>> i = nodes.entrySet().iterator(); i.hasNext(); ) {
+                    if (!(i.next().getValue() instanceof EphemeralNode)) {
+                        i.remove();
+                    }
+                }
+                nodes.putAll(newNodes);
+                jenkins.updateComputerList();
+                jenkins.trimLabels();
+            }
+        });
+    }
+
+    /**
+     * Returns the directory that the nodes are stored in.
+     *
+     * @return the directory that the nodes are stored in.
+     * @throws IOException
+     */
+    private File getNodesDir() throws IOException {
+        final File nodesDir = new File(jenkins.getRootDir(), "nodes");
+        if (!nodesDir.isDirectory() && !nodesDir.mkdirs()) {
+            throw new IOException(String.format("Could not mkdirs %s", nodesDir));
+        }
+        return nodesDir;
+    }
+
+    /**
+     * Returns {@code true} if and only if the list of nodes is stored in the legacy location.
+     *
+     * @return {@code true} if and only if the list of nodes is stored in the legacy location.
+     */
+    public boolean isLegacy() {
+        return !new File(jenkins.getRootDir(), "nodes").isDirectory();
+    }
+}

core/src/main/java/jenkins/util/AtmostOneTaskExecutor.java

@@ -2,6 +2,8 @@ package jenkins.util;
 
 import com.google.common.util.concurrent.SettableFuture;
 import hudson.remoting.AtmostOneThreadExecutor;
+import hudson.util.DaemonThreadFactory;
+import hudson.util.NamingThreadFactory;
 
 import java.util.concurrent.Callable;
 import java.util.concurrent.Executor;
@@ -63,7 +65,12 @@ public class AtmostOneTaskExecutor<V> {
     }
 
     public AtmostOneTaskExecutor(Callable<V> task) {
-        this(new AtmostOneThreadExecutor(),task);
+        this(new AtmostOneThreadExecutor(new NamingThreadFactory(
+                        new DaemonThreadFactory(),
+                        String.format("AtmostOneTaskExecutor[%s]", task)
+                )),
+                task
+        );
     }
 
     public synchronized Future<V> submit() {

core/src/main/resources/hudson/model/Messages.properties

@@ -315,6 +315,7 @@ RunParameterDefinition.DisplayName=Run Parameter
 PasswordParameterDefinition.DisplayName=Password Parameter
 
 Node.BecauseNodeIsReserved={0} is reserved for jobs with matching label expression
+Node.BecauseNodeIsNotAcceptingTasks={0} is not accepting tasks
 Node.LabelMissing={0} doesn\u2019t have label {1}
 Node.LackingBuildPermission={0} doesn\u2019t have a permission to run on {1}
 Node.Mode.NORMAL=Utilize this node as much as possible

core/src/main/resources/lib/hudson/executors.jelly

 <!--
 The MIT License
 
-Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi,
+Copyright (c) 2004-2015, Sun Microsystems, Inc., Kohsuke Kawaguchi,
 Stephen Connolly, id:cactusman, Yahoo! Inc., Alan Harder
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
@@ -40,8 +40,7 @@ THE SOFTWARE.
     </d:tag>
     <d:tag name="executor">
       <j:set var="ae" value="${e.asynchronousExecution}"/>
-      <j:choose>
-        <j:when test="${(!c.offline or (c.offline and !e.idle)) and (ae == null || ae.displayCell())}">
+      <j:if test="${(!c.offline or (c.offline and !e.idle)) and (ae == null || ae.displayCell())}">
           <tr>
             <td class="pane" align="right" style="vertical-align: top">
               ${name}
@@ -77,6 +76,25 @@ THE SOFTWARE.
                   <td class="pane">
                     <div style="white-space: normal">
                       <j:set var="exe" value="${e.currentExecutable}" />
+                      <j:set var="wu" value="${e.currentWorkUnit}" />
+                      <j:choose>
+                        <j:when test="${exe == null and wu != null}">
+                          <j:set var="exeparent" value="${wu.work}"/>
+                          <j:choose>
+                            <j:when test="${h.hasPermission(exeparent,exeparent.READ)}">
+                              <a href="${rootURL}/${exeparent.url}"><l:breakable value="${exeparent.fullDisplayName}"/></a>
+                              <t:progressBar tooltip="${%Pending}" pos="-1" href="${rootURL}/${exeparent.url}"/>
+                            </j:when>
+                            <j:otherwise>
+                              <span>${%Unknown Task}</span>
+                            </j:otherwise>
+                          </j:choose>
+                        </j:when>
+                        <j:when test="${exe == null and wu == null}">
+                          <!-- went idle concurrent with testing for idle -->
+                          ${%Idle}
+                        </j:when>
+                        <j:otherwise>
                       <j:invokeStatic var="exeparent"
                          className="hudson.model.queue.Executables" method="getParentOf">
                         <j:arg type="hudson.model.Queue$Executable" value="${exe}" />
@@ -90,6 +108,8 @@ THE SOFTWARE.
                           <span>${%Unknown Task}</span>
                         </j:otherwise>
                       </j:choose>
+                        </j:otherwise>
+                      </j:choose>
                     </div>
                   </td>
                 <td class="pane">
@@ -106,29 +126,28 @@ THE SOFTWARE.
               </j:otherwise>
             </j:choose>
           </tr>
-        </j:when>
-      </j:choose>
+      </j:if>
     </d:tag>
   </d:taglib>
   <j:set var="computers" value="${attrs.computers?:app.computers}" />
+  <j:set var="computersSize" value="${computers.size()}"/>
     <l:pane width="3" id="executors"
         title="&lt;a href='${rootURL}/computer/'>${%Build Executor Status}&lt;/a>"
-        collapsedText="${%Computers(computers.size() - 1, app.unlabeledLoad.computeTotalExecutors() - app.unlabeledLoad.computeIdleExecutors(), app.unlabeledLoad.computeTotalExecutors())}">
+        collapsedText="${%Computers(computersSize - 1, app.unlabeledLoad.computeTotalExecutors() - app.unlabeledLoad.computeIdleExecutors(), app.unlabeledLoad.computeTotalExecutors())}">
       <colgroup><col width="30"/><col width="200*"/><col width="24"/></colgroup>
 
       <j:forEach var="c" items="${computers}">
+        <j:set var="cDisplayExecutors" value="${c.displayExecutors}"/>
         <tr>
-          <j:if test="${computers.size() gt 1 and (c.executors.size()!=0 or c.oneOffExecutors.size()!=0)}">
+          <j:if test="${computersSize gt 1 and !cDisplayExecutors.isEmpty()}">
             <th class="pane" colspan="3">
               <local:computerCaption title="${c.displayName}" />
             </th>
           </j:if>
         </tr>
-        <j:forEach var="e" items="${c.executors}" varStatus="eloop">
-          <local:executor name="${eloop.index+1}" url="executors/${eloop.index}" />
-        </j:forEach>
-        <j:forEach var="e" items="${c.oneOffExecutors}" varStatus="eloop">
-          <local:executor name="" url="oneOffExecutors/${eloop.index}" />
+        <j:forEach var="de" items="${cDisplayExecutors}" varStatus="eloop">
+          <j:set var="e" value="${de.executor}"/>
+          <local:executor name="${de.displayName}" url="${de.url}" />
         </j:forEach>
       </j:forEach>
     </l:pane>

core/src/main/resources/lib/layout/layout.jelly

@@ -69,11 +69,14 @@ ${h.initPageVariables(context)}
 <j:set var="isMSIE" value="${userAgent.contains('MSIE')}"/>
 <j:set var="_" value="${request.getSession()}"/>
 <j:set var="_" value="${h.configureAutoRefresh(request, response, attrs.norefresh!=null and !attrs.norefresh.equals(false))}"/>
+  <j:set var="extensionsAvailable" value="${h.extensionsAvailable}"/>
   <j:if test="${request.servletPath=='/' || request.servletPath==''}">
     ${h.advertiseHeaders(response)}
-    <j:forEach var="pd" items="${h.pageDecorators}">
-      <st:include it="${pd}" page="httpHeaders.jelly" optional="true"/>
-    </j:forEach>
+    <j:if test="${extensionsAvailable}">
+      <j:forEach var="pd" items="${h.pageDecorators}">
+        <st:include it="${pd}" page="httpHeaders.jelly" optional="true"/>
+      </j:forEach>
+    </j:if>
   </j:if>
 <x:doctype name="html" />
 <html>
@@ -126,9 +129,11 @@ ${h.initPageVariables(context)}
     <script src="${resURL}/scripts/hudson-behavior.js" type="text/javascript"></script>
     <script src="${resURL}/scripts/sortable.js" type="text/javascript"/>
 
-    <script>
-        crumb.init("${h.getCrumbRequestField()}", "${h.getCrumb(request)}");
-    </script>
+    <j:if test="${extensionsAvailable}">
+      <script>
+          crumb.init("${h.getCrumbRequestField()}", "${h.getCrumb(request)}");
+      </script>
+    </j:if>
 
     <link rel="stylesheet" href="${resURL}/scripts/yui/container/assets/container.css" type="text/css"/>
     <link rel="stylesheet" href="${resURL}/scripts/yui/assets/skins/sam/skin.css" type="text/css" />
@@ -143,9 +148,11 @@ ${h.initPageVariables(context)}
     <meta name="ROBOTS" content="INDEX,NOFOLLOW" />
     <j:set var="mode" value="header" />
     <d:invokeBody />
-    <j:forEach var="pd" items="${h.pageDecorators}">
-      <st:include it="${pd}" page="header.jelly" optional="true" />
-    </j:forEach>
+    <j:if test="${extensionsAvailable}">
+      <j:forEach var="pd" items="${h.pageDecorators}">
+        <st:include it="${pd}" page="header.jelly" optional="true" />
+      </j:forEach>
+    </j:if>
 
     <j:if test="${isMSIE}">
       <script src="${resURL}/scripts/msie.js" type="text/javascript"/>
@@ -259,9 +266,11 @@ ${h.initPageVariables(context)}
         <span class="jenkins_ver">
           <a href="${h.getFooterURL()}">Jenkins ver. ${h.version}</a>
         </span>
+        <j:if test="${extensionsAvailable}">
         <j:forEach var="pd" items="${h.pageDecorators}">
           <st:include it="${pd}" page="footer.jelly" optional="true" />
         </j:forEach>
+        </j:if>
       </div>
     </div>
   </body>

test/src/test/groovy/hudson/model/AbstractProjectTest.groovy

 /*
  * The MIT License
- * 
+ *
  * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi
- * 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
  * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
  * copies of the Software, and to permit persons to whom the Software is
  * furnished to do so, subject to the following conditions:
- * 
+ *
  * The above copyright notice and this permission notice shall be included in
  * all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -400,7 +400,6 @@ public class AbstractProjectTest extends HudsonTestCase {
     public void testQueueSuccessBehavior() {
         // prevent any builds to test the behaviour
         jenkins.numExecutors = 0;
-        jenkins.updateComputerList(false);
 
         def p = createFreeStyleProject()
         def f = p.scheduleBuild2(0)
@@ -419,7 +418,6 @@ public class AbstractProjectTest extends HudsonTestCase {
     public void testQueueSuccessBehaviorOverHTTP() {
         // prevent any builds to test the behaviour
         jenkins.numExecutors = 0;
-        jenkins.updateComputerList(false);
 
         def p = createFreeStyleProject()
         def wc = createWebClient();

test/src/test/java/hudson/model/ExecutorTest.java

@@ -34,7 +34,6 @@ public class ExecutorTest {
     @Test
     public void yank() throws Exception {
         j.jenkins.setNumExecutors(1);
-        j.jenkins.updateComputerList(true);
         Computer c = j.jenkins.toComputer();
         final Executor e = c.getExecutors().get(0);
 
@@ -59,7 +58,6 @@ public class ExecutorTest {
     @Issue("JENKINS-4756")
     public void whenAnExecutorIsYankedANewExecutorTakesItsPlace() throws Exception {
         j.jenkins.setNumExecutors(1);
-        j.jenkins.updateComputerList(true);
 
         Computer c = j.jenkins.toComputer();
         Executor e = getExecutorByNumber(c, 0);

test/src/test/java/hudson/model/GetEnvironmentOutsideBuildTest.java

@@ -20,27 +20,27 @@ import org.jvnet.hudson.test.HudsonTestCase;
 
 /**
  * Tests that getEnvironment() calls outside of builds are safe.
- * 
+ *
  * @author kutzi
  */
 @Issue("JENKINS-11592")
 public class GetEnvironmentOutsideBuildTest extends HudsonTestCase {
 
     private int oldExecNum;
-    
+
     @Override
     protected void runTest() throws Throwable {
         // Disable tests
-        
+
         // It's unfortunately not working, yet, as whenJenkinsMasterHasNoExecutors is not working as expected
     }
 
     public void setUp() throws Exception {
         super.setUp();
-        
+
         this.oldExecNum = Jenkins.getInstance().getNumExecutors();
     }
-    
+
     public void tearDown() throws Exception {
         restoreOldNumExecutors();
         super.tearDown();
@@ -48,10 +48,9 @@ public class GetEnvironmentOutsideBuildTest extends HudsonTestCase {
 
     private void restoreOldNumExecutors() throws IOException {
         Jenkins.getInstance().setNumExecutors(this.oldExecNum);
-        Jenkins.getInstance().setNodes(Jenkins.getInstance().getNodes());
         assertNotNull(Jenkins.getInstance().toComputer());
     }
-    
+
     private MavenModuleSet createSimpleMavenProject() throws Exception {
         MavenModuleSet project = createMavenProject();
         MavenInstallation mi = configureMaven3();
@@ -61,36 +60,34 @@ public class GetEnvironmentOutsideBuildTest extends HudsonTestCase {
         project.setGoals("validate");
         return project;
     }
-    
+
     private void whenJenkinsMasterHasNoExecutors() throws IOException {
         Jenkins.getInstance().setNumExecutors(0);
-        // force update of nodes:
-        Jenkins.getInstance().setNodes(Jenkins.getInstance().getNodes());
         assertNull(Jenkins.getInstance().toComputer());
     }
-    
+
     public void testMaven() throws Exception {
         MavenModuleSet m = createSimpleMavenProject();
-        
+
         assertGetEnvironmentCallOutsideBuildWorks(m);
     }
-    
+
     public void testFreestyle() throws Exception {
         FreeStyleProject project = createFreeStyleProject();
-        
+
         assertGetEnvironmentCallOutsideBuildWorks(project);
     }
-    
+
     public void testMatrix() throws Exception {
         MatrixProject createMatrixProject = createMatrixProject();
-        
+
         assertGetEnvironmentCallOutsideBuildWorks(createMatrixProject);
     }
-    
+
     @SuppressWarnings({"rawtypes", "unchecked"})
     private void assertGetEnvironmentCallOutsideBuildWorks(AbstractProject job) throws Exception {
         AbstractBuild build = buildAndAssertSuccess(job);
-        
+
         assertGetEnvironmentWorks(build);
     }
 

test/src/test/java/hudson/model/QueueTest.java

 /*
  * The MIT License
- * 
+ *
  * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi
- * 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
  * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
  * copies of the Software, and to permit persons to whom the Software is
  * furnished to do so, subject to the following conditions:
- * 
+ *
  * The above copyright notice and this permission notice shall be included in
  * all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -113,7 +113,6 @@ public class QueueTest {
 
         // prevent execution to push stuff into the queue
         r.jenkins.setNumExecutors(0);
-        r.jenkins.setNodes(r.jenkins.getNodes());
 
         FreeStyleProject testProject = r.createFreeStyleProject("test");
         testProject.scheduleBuild(new UserIdCause());
@@ -130,7 +129,7 @@ public class QueueTest {
         assertEquals(1, q.getItems().length);
 
         // did it bind back to the same object?
-        assertSame(q.getItems()[0].task,testProject);        
+        assertSame(q.getItems()[0].task,testProject);
     }
 
     /**
@@ -162,7 +161,6 @@ public class QueueTest {
 
         // prevent execution to push stuff into the queue
         r.jenkins.setNumExecutors(0);
-        r.jenkins.setNodes(r.jenkins.getNodes());
 
         FreeStyleProject testProject = r.createFreeStyleProject("test");
         testProject.scheduleBuild(new UserIdCause());
@@ -393,7 +391,7 @@ public class QueueTest {
         assertEquals(1, runs.size());
         assertEquals("slave0", runs.get(0).getBuiltOnStr());
     }
-    
+
     @Issue("JENKINS-10944")
     @Test public void flyweightTasksBlockedByShutdown() throws Exception {
         r.jenkins.doQuietDown(true, 0);
@@ -607,8 +605,8 @@ public class QueueTest {
                      @Override
                     public Label getAssignedLabel(){
                         throw new IllegalArgumentException("Test exception"); //cause dead of executor
-                    }   
-                     
+                    }
+
                     @Override
                      public void save(){
                          //do not need save
@@ -635,7 +633,7 @@ public class QueueTest {
                     break; // executor is dead due to exception
              }
              if(e.isIdle()){
-                 assertTrue("Node went to idle before project had" + project2.getDisplayName() + " been started", v.isDone());   
+                 assertTrue("Node went to idle before project had" + project2.getDisplayName() + " been started", v.isDone());
              }
                 Thread.sleep(1000);
         }
@@ -644,37 +642,37 @@ public class QueueTest {
         Queue.getInstance().cancel(projectError); // cancel job which cause dead of executor
         e.doYank(); //restart executor
         while(!e.isIdle()){ //executor should take project2 from queue
-            Thread.sleep(1000); 
+            Thread.sleep(1000);
         }
         //project2 should not be in pendings
         List<Queue.BuildableItem> items = Queue.getInstance().getPendingItems();
         for(Queue.BuildableItem item : items){
-            assertFalse("Project " + project2.getDisplayName() + " stuck in pendings",item.task.getName().equals(project2.getName())); 
+            assertFalse("Project " + project2.getDisplayName() + " stuck in pendings",item.task.getName().equals(project2.getName()));
         }
     }
-    
+
     @Test public void cancelInQueue() throws Exception {
         // parepare an offline slave.
         DumbSlave slave = r.createOnlineSlave();
         assertFalse(slave.toComputer().isOffline());
         slave.toComputer().disconnect(null).get();
         assertTrue(slave.toComputer().isOffline());
-        
+
         FreeStyleProject p = r.createFreeStyleProject();
         p.setAssignedNode(slave);
-        
+
         QueueTaskFuture<FreeStyleBuild> f = p.scheduleBuild2(0);
         try {
             f.get(3, TimeUnit.SECONDS);
             fail("Should time out (as the slave is offline).");
         } catch (TimeoutException e) {
         }
-        
+
         Queue.Item item = Queue.getInstance().getItem(p);
         assertNotNull(item);
         Queue.getInstance().doCancelItem(item.getId());
         assertNull(Queue.getInstance().getItem(p));
-        
+
         try {
             f.get(10, TimeUnit.SECONDS);
             fail("Should not get (as it is cancelled).");

test/src/test/java/jenkins/model/JenkinsReloadConfigurationTest.java

@@ -47,7 +47,7 @@ public class JenkinsReloadConfigurationTest {
     }
 
     private void modifyNode(Node node) throws Exception {
-        replace("config.xml", "oldLabel", "newLabel");
+        replace(node.getNodeName().equals("") ? "config.xml" : String.format("nodes/%s/config.xml",node.getNodeName()), "oldLabel", "newLabel");
 
         assertEquals("oldLabel", node.getLabelString());