Skip to content

Commit a6acb13

Browse files
committed
Prevent deadlock when onConfigurationChange() is called frequently
1 parent 07df00c commit a6acb13

File tree

1 file changed

+31
-15
lines changed

1 file changed

+31
-15
lines changed

src/main/java/org/jvnet/hudson/plugins/platformlabeler/NodeLabelCache.java

+31-15
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ public final void preOnline(final Computer computer, Channel channel, FilePath r
7676
throws IOException, InterruptedException {
7777
try {
7878
cacheAndRefreshModel(computer, channel);
79+
saveNodeLabel(computer.getNode());
7980
} catch (Exception e) {
8081
String name = "unnamed agent"; // built-in (and others) may not have a name during preOnline
8182
if (computer != null && !computer.getName().isEmpty()) {
@@ -115,6 +116,7 @@ public final void cacheAndRefreshModel(final Computer computer, VirtualChannel c
115116
/** When any computer has changed, update the platform labels according to the configuration. */
116117
@Override
117118
public final void onConfigurationChange() {
119+
LOGGER.log(Level.FINEST, "onConfigurationChange() called to refresh platform labels");
118120
synchronized (nodePlatformProperties) {
119121
nodePlatformProperties.forEach((node, labels) -> {
120122
refreshModel(node);
@@ -144,15 +146,13 @@ private void logUpdateNodeException(Node node, IOException e) {
144146
}
145147

146148
@SuppressFBWarnings(value = "CRLF_INJECTION_LOGS", justification = "CRLF not allowed in label display names")
147-
private void logUpdateNodeResult(
148-
boolean result, Node node, Collection<LabelAtom> labels, Set<LabelAtom> assignedLabels) {
149+
private void logUpdateNodeResult(boolean result, Node node, Set<LabelAtom> assignedLabels) {
149150
LOGGER.log(
150151
Level.FINEST,
151152
String.format(
152-
"Update of node '%s' %s with labels %s and assigned labels %s",
153+
"Update of node '%s' %s with assigned labels %s",
153154
node.getDisplayName(),
154155
result ? "succeeded" : "failed",
155-
Arrays.toString(labels.toArray()),
156156
Arrays.toString(assignedLabels.toArray())));
157157
}
158158
/**
@@ -166,21 +166,37 @@ final void refreshModel(final Computer computer) {
166166
if (node != null) {
167167
Collection<LabelAtom> labels = getLabelsForNode(node);
168168
nodeLabels.put(node, labels);
169-
Set<LabelAtom> assignedLabels = node.getAssignedLabels();
170-
try {
171-
// Save the node to ensure label will see the node updated when platform details are added (or
172-
// updated).
173-
// This will ensure a node has the same state if we were adding labels via the UI.
174-
// See JENKINS-72224
175-
boolean result = Jenkins.get().updateNode(node);
176-
logUpdateNodeResult(result, node, labels, assignedLabels);
177-
} catch (IOException e) {
178-
logUpdateNodeException(node, e);
179-
}
169+
node.getAssignedLabels();
180170
}
181171
}
182172
}
183173

174+
/**
175+
* Save the node to ensure label will see the node updated when platform details are added (or
176+
* updated).
177+
* This will ensure a node has the same state if we were adding labels via the UI.
178+
* See JENKINS-72224
179+
*
180+
* @param node Node whose labels should be saved
181+
*/
182+
final void saveNodeLabel(Node node) {
183+
if (node == null) {
184+
LOGGER.log(Level.FINEST, "Node is null. Unable to save labels and update node.");
185+
return;
186+
}
187+
Set<LabelAtom> assignedLabels = node.getAssignedLabels();
188+
try {
189+
// Save the node to ensure label will see the node updated when platform details are added (or
190+
// updated).
191+
// This will ensure a node has the same state if we were adding labels via the UI.
192+
// See JENKINS-72224
193+
boolean result = Jenkins.get().updateNode(node);
194+
logUpdateNodeResult(result, node, assignedLabels);
195+
} catch (IOException e) {
196+
logUpdateNodeException(node, e);
197+
}
198+
}
199+
184200
/**
185201
* Return PlatformDetails of the computer.
186202
*

0 commit comments

Comments
 (0)