Skip to content

Commit

Permalink
Prevent deadlock when onConfigurationChange() is called frequently (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
jonesbusy authored Jul 12, 2024
1 parent 07df00c commit 5444054
Showing 1 changed file with 31 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public final void preOnline(final Computer computer, Channel channel, FilePath r
throws IOException, InterruptedException {
try {
cacheAndRefreshModel(computer, channel);
saveNodeLabel(computer.getNode());
} catch (Exception e) {
String name = "unnamed agent"; // built-in (and others) may not have a name during preOnline
if (computer != null && !computer.getName().isEmpty()) {
Expand Down Expand Up @@ -115,6 +116,7 @@ public final void cacheAndRefreshModel(final Computer computer, VirtualChannel c
/** When any computer has changed, update the platform labels according to the configuration. */
@Override
public final void onConfigurationChange() {
LOGGER.log(Level.FINEST, "onConfigurationChange() called to refresh platform labels");
synchronized (nodePlatformProperties) {
nodePlatformProperties.forEach((node, labels) -> {
refreshModel(node);
Expand Down Expand Up @@ -144,15 +146,13 @@ private void logUpdateNodeException(Node node, IOException e) {
}

@SuppressFBWarnings(value = "CRLF_INJECTION_LOGS", justification = "CRLF not allowed in label display names")
private void logUpdateNodeResult(
boolean result, Node node, Collection<LabelAtom> labels, Set<LabelAtom> assignedLabels) {
private void logUpdateNodeResult(boolean result, Node node, Set<LabelAtom> assignedLabels) {
LOGGER.log(
Level.FINEST,
String.format(
"Update of node '%s' %s with labels %s and assigned labels %s",
"Update of node '%s' %s with assigned labels %s",
node.getDisplayName(),
result ? "succeeded" : "failed",
Arrays.toString(labels.toArray()),
Arrays.toString(assignedLabels.toArray())));
}
/**
Expand All @@ -166,21 +166,37 @@ final void refreshModel(final Computer computer) {
if (node != null) {
Collection<LabelAtom> labels = getLabelsForNode(node);
nodeLabels.put(node, labels);
Set<LabelAtom> assignedLabels = node.getAssignedLabels();
try {
// Save the node to ensure label will see the node updated when platform details are added (or
// updated).
// This will ensure a node has the same state if we were adding labels via the UI.
// See JENKINS-72224
boolean result = Jenkins.get().updateNode(node);
logUpdateNodeResult(result, node, labels, assignedLabels);
} catch (IOException e) {
logUpdateNodeException(node, e);
}
node.getAssignedLabels();
}
}
}

/**
* Save the node to ensure label will see the node updated when platform details are added (or
* updated).
* This will ensure a node has the same state if we were adding labels via the UI.
* See JENKINS-72224
*
* @param node Node whose labels should be saved
*/
final void saveNodeLabel(Node node) {
if (node == null) {

Check warning on line 183 in src/main/java/org/jvnet/hudson/plugins/platformlabeler/NodeLabelCache.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 183 is only partially covered, one branch is missing
LOGGER.log(Level.FINEST, "Node is null. Unable to save labels and update node.");
return;

Check warning on line 185 in src/main/java/org/jvnet/hudson/plugins/platformlabeler/NodeLabelCache.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 184-185 are not covered by tests
}
Set<LabelAtom> assignedLabels = node.getAssignedLabels();
try {
// Save the node to ensure label will see the node updated when platform details are added (or
// updated).
// This will ensure a node has the same state if we were adding labels via the UI.
// See JENKINS-72224
boolean result = Jenkins.get().updateNode(node);
logUpdateNodeResult(result, node, assignedLabels);
} catch (IOException e) {
logUpdateNodeException(node, e);
}
}

/**
* Return PlatformDetails of the computer.
*
Expand Down

0 comments on commit 5444054

Please sign in to comment.