[FIXED HUDSON-4235] Applied a patch, with a bit of changes. This will be in 1.321.

git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@20863 71c3de6d-444a-0410-be80-ed276b4c234a

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

+/*
+ * The MIT License
+ *
+ * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, Erik Ramfelt, Koichi Fujikawa, Red Hat, Inc., Seiji Sogabe, Stephen Connolly, Tom Huybrechts, Yahoo! Inc, Robert Collins
+ *
+ * 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 hudson.model;
+
+import hudson.remoting.VirtualChannel;
+import hudson.tasks.DynamicLabeler;
+import hudson.tasks.LabelFinder;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.Collections;
+
+/**
+ * Dynamic labels (with its recomputing logic.)
+ *
+ * @author Robert Collins <robertc@robertcollins.net>
+ */
+public final class DynamicLabels {
+    /**
+     * Stores the computer object for which dynamic labels have been
+     * computed, so that we can detect when we need to recompute.
+     */
+    private final Computer computer;
+    /**
+     * Stores the last set of dynamically calculated labels. Read-only.
+     */
+    public final Set<Label> labels;
+
+    /**
+     * Create dynamic labels for a computer.
+     */
+    public DynamicLabels(Computer comp) {
+        labels = buildLabels(comp!=null?comp.getChannel():null);
+        computer = comp;
+    }
+
+    /**
+     * Determine if the labels cached in this DynamicLabels is up to date.
+     */
+    public boolean isChanged(Computer comp) {
+        return comp!=computer;
+    }
+
+    /**
+     * Read labels from channel into this.labels.
+     */
+    private static Set<Label> buildLabels(VirtualChannel channel) {
+        if (null == channel)
+            return Collections.emptySet();
+
+        Set<Label> r = new HashSet<Label>();
+        for (DynamicLabeler labeler : LabelFinder.LABELERS)
+            for (String label : labeler.findLabels(channel))
+                r.add(Hudson.getInstance().getLabel(label));
+        return r;
+    }
+}

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

@@ -91,8 +91,6 @@ import hudson.slaves.NodeProvisioner;
 import hudson.slaves.OfflineCause;
 import hudson.tasks.BuildWrapper;
 import hudson.tasks.Builder;
-import hudson.tasks.DynamicLabeler;
-import hudson.tasks.LabelFinder;
 import hudson.tasks.Mailer;
 import hudson.tasks.Publisher;
 import hudson.triggers.Trigger;
@@ -428,7 +426,6 @@ public final class Hudson extends Node implements ItemGroup<TopLevelItem>, Stapl
      */
     private transient final ConcurrentHashMap<String,Label> labels = new ConcurrentHashMap<String,Label>();
     private transient volatile Set<Label> labelSet;
-    private transient volatile Set<Label> dynamicLabels = null;
 
     /**
      * Load statistics of the entire system.
@@ -1259,7 +1256,7 @@ public final class Hudson extends Node implements ItemGroup<TopLevelItem>, Stapl
     /**
      * Gets the label that exists on this system by the name.
      *
-     * @return null if no such label exists.
+     * @return null if no name is null.
      * @see Label#parse(String)
      */
     public Label getLabel(String name) {
@@ -1943,8 +1940,10 @@ public final class Hudson extends Node implements ItemGroup<TopLevelItem>, Stapl
     }
 
     public Set<Label> getAssignedLabels() {
-        Set<Label> lset = labelSet; // labelSet may be set by another thread while we are in this method, so capture it.
-        if (lset == null) {
+        // labelSet may be set by another thread while we are in this method,
+        // so capture it.
+        Set<Label> lset = labelSet;
+        if (lset == null || isChangedDynamicLabels()) {
             Set<Label> r = Label.parse(getLabelString());
             r.addAll(getDynamicLabels());
             r.add(getSelfLabel());
@@ -1959,25 +1958,12 @@ public final class Hudson extends Node implements ItemGroup<TopLevelItem>, Stapl
      * @see hudson.tasks.LabelFinder
      */
     public Set<Label> getDynamicLabels() {
-        if (dynamicLabels == null) {
-            // in the worst cast, two threads end up doing the same computation twice,
-            // but that won't break the semantics.
+        if (dynamicLabels.isChanged(toComputer()))
+            // in the worst cast, two threads end up doing the same computation
+            // twice, but that won't break the semantics.
             // OTOH, not locking prevents dead-lock. See #1390
-            Set<Label> r = new HashSet<Label>();
-            Computer comp = getComputer("");
-            if (comp != null) {
-                VirtualChannel channel = comp.getChannel();
-                if (channel != null) {
-                    for (DynamicLabeler labeler : LabelFinder.LABELERS) {
-                        for (String label : labeler.findLabels(channel)) {
-                            r.add(getLabel(label));
-                        }
-                    }
-                }
-            }
-            dynamicLabels = r;
-        }
-        return dynamicLabels;
+            dynamicLabels = new DynamicLabels(toComputer());
+        return dynamicLabels.labels;
     }
 
     public Label getSelfLabel() {

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

@@ -29,7 +29,6 @@ import hudson.FileSystemProvisioner;
 import hudson.Launcher;
 import hudson.node_monitors.NodeMonitor;
 import hudson.remoting.VirtualChannel;
-import hudson.remoting.Channel;
 import hudson.security.ACL;
 import hudson.security.AccessControlled;
 import hudson.security.Permission;
@@ -65,6 +64,11 @@ public abstract class Node extends AbstractModelObject implements Describable<No
      * is saved once.
      */
     protected volatile transient boolean holdOffLaunchUntilSave;
+    /**
+     * labels assigned to this node dynamically by plugins or other computation
+     * rather than via user configuration.
+     */
+    protected volatile transient DynamicLabels dynamicLabels = new DynamicLabels(null);
 
     public String getDisplayName() {
         return getNodeName(); // default implementation
@@ -257,6 +261,15 @@ public abstract class Node extends AbstractModelObject implements Describable<No
      */
     public abstract ClockDifference getClockDifference() throws IOException, InterruptedException;
 
+    /**
+     * Check if we should rebuild the list of dynamic labels.
+     * @todo make less hacky
+     * @return
+     */
+    protected boolean isChangedDynamicLabels() {
+        return dynamicLabels.isChanged(toComputer());
+    }
+
     /**
      * Constants that control how Hudson allocates jobs to slaves.
      */

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

@@ -39,7 +39,6 @@ import hudson.slaves.NodeProperty;
 import hudson.slaves.NodePropertyDescriptor;
 import hudson.slaves.RetentionStrategy;
 import hudson.slaves.SlaveComputer;
-import hudson.tasks.DynamicLabeler;
 import hudson.tasks.LabelFinder;
 import hudson.util.ClockDifference;
 import hudson.util.DescribableList;
@@ -54,7 +53,6 @@ import java.net.URL;
 import java.net.URLConnection;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -127,9 +125,6 @@ public abstract class Slave extends Node implements Serializable {
      */
     private transient volatile Set<Label> labels;
 
-    private transient volatile Set<Label> dynamicLabels;
-    private transient volatile int dynamicLabelsInstanceHash;
-
     @DataBoundConstructor
     public Slave(String name, String nodeDescription, String remoteFS, String numExecutors,
                  Mode mode, String labelString, ComputerLauncher launcher, RetentionStrategy retentionStrategy, List<? extends NodeProperty<?>> nodeProperties) throws FormException, IOException {
@@ -235,24 +230,6 @@ public abstract class Slave extends Node implements Serializable {
         return labels;
     }
 
-    /**
-     * Check if we should rebuild the list of dynamic labels.
-     * @todo make less hacky
-     * @return
-     */
-    private boolean isChangedDynamicLabels() {
-        Computer comp = getComputer();
-        if (comp == null) {
-            return dynamicLabelsInstanceHash != 0;
-        } else {
-            if (dynamicLabelsInstanceHash == comp.hashCode()) {
-                return false;
-            }
-            dynamicLabels = null; // force a re-calc
-            return true;
-        }
-    }
-
     /**
      * Returns the possibly empty set of labels that it has been determined as supported by this node.
      *
@@ -263,31 +240,14 @@ public abstract class Slave extends Node implements Serializable {
      *      never null.
      */
     public Set<Label> getDynamicLabels() {
-        // another thread may preempt and set dynamicLabels field to null,
-        // so a care needs to be taken to avoid race conditions under all circumstances.
-        Set<Label> labels = dynamicLabels;
-        if (labels != null)     return labels;
-
+        // another thread may preempt and replace dynamicLabels.
+        // so a care needs to be taken to avoid race conditions under all 
+        // circumstances.
         synchronized (this) {
-            labels = dynamicLabels;
-            if (labels != null)     return labels;
-
-            dynamicLabels = labels = new HashSet<Label>();
-            Computer computer = getComputer();
-            VirtualChannel channel;
-            if (computer != null && (channel = computer.getChannel()) != null) {
-                dynamicLabelsInstanceHash = computer.hashCode();
-                for (DynamicLabeler labeler : LabelFinder.LABELERS) {
-                    for (String label : labeler.findLabels(channel)) {
-                        labels.add(Hudson.getInstance().getLabel(label));
-                    }
-                }
-            } else {
-                dynamicLabelsInstanceHash = 0;
-            }
-
-            return labels;
+            if (dynamicLabels.isChanged(toComputer()))
+                dynamicLabels = new DynamicLabels(toComputer());
         }
+        return dynamicLabels.labels;
     }
 
     public ClockDifference getClockDifference() throws IOException, InterruptedException {
@@ -381,7 +341,7 @@ public abstract class Slave extends Node implements Serializable {
      * Gets the corresponding computer object.
      */
     public SlaveComputer getComputer() {
-        return (SlaveComputer)Hudson.getInstance().getComputer(this);
+        return (SlaveComputer)toComputer();
     }
 
     public boolean equals(Object o) {