Skip to content

Commit

Permalink
Eliminate ConcurrentModificationException in free style projects (#604)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mPokornyETM
mPokornyETM committed Jan 11, 2024
1 parent 70ea6f5 commit 1b23794
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 46 deletions.
5 changes: 5 additions & 0 deletions src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,11 @@ public synchronized boolean lock(List<LockableResource> resources, Run<?, ?> bui
return lock(resources, build, context, null, null, false);
}

@Restricted(NoExternalUse.class)
public synchronized boolean lock(List<LockableResource> resources, Run<?, ?> build) {
return lock(resources, build, null);
}

/** Try to lock the resource and return true if locked. */
public synchronized boolean lock(
List<LockableResource> resources,
Expand Down
108 changes: 62 additions & 46 deletions src/main/java/org/jenkins/plugins/lockableresources/queue/LockRunListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,48 +40,53 @@ public void onStarted(Run<?, ?> build, TaskListener listener) {
if (build instanceof MatrixBuild) return;

if (build instanceof AbstractBuild) {
Job<?, ?> proj = Utils.getProject(build);
List<LockableResource> required = new ArrayList<>();
LockableResourcesStruct resources = Utils.requiredResources(proj);
LockableResourcesManager lrm = LockableResourcesManager.get();
synchronized (lrm.syncResources) {
Job<?, ?> proj = Utils.getProject(build);
List<LockableResource> required = new ArrayList<>();
LockableResourcesStruct resources = Utils.requiredResources(proj);

if (resources != null) {
if (resources.requiredNumber != null
|| !resources.label.isEmpty()
|| resources.getResourceMatchScript() != null) {
required.addAll(LockableResourcesManager.get().getResourcesFromProject(proj.getFullName()));
} else {
required.addAll(resources.required);
}
if (resources != null) {
if (resources.requiredNumber != null
|| !resources.label.isEmpty()
|| resources.getResourceMatchScript() != null) {
required.addAll(lrm.getResourcesFromProject(proj.getFullName()));
} else {
required.addAll(resources.required);
}

if (LockableResourcesManager.get().lock(required, build, null)) {
build.addAction(LockedResourcesBuildAction.fromResources(required));
listener.getLogger().printf("%s acquired lock on %s%n", LOG_PREFIX, required);
LOGGER.fine(build.getFullDisplayName() + " acquired lock on " + required);
if (resources.requiredVar != null) {
List<StringParameterValue> envsToSet = new ArrayList<>();
if (lrm.lock(required, build)) {

Check warning on line 58 in src/main/java/org/jenkins/plugins/lockableresources/queue/LockRunListener.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 58 is only partially covered, one branch is missing
build.addAction(LockedResourcesBuildAction.fromResources(required));
listener.getLogger().printf("%s acquired lock on %s%n", LOG_PREFIX, required);
LOGGER.info(build.getFullDisplayName() + " acquired lock on " + required);
if (resources.requiredVar != null) {
List<StringParameterValue> envsToSet = new ArrayList<>();

// add the comma separated list of names acquired
envsToSet.add(new StringParameterValue(
resources.requiredVar,
required.stream().map(LockableResource::getName).collect(Collectors.joining(","))));
// add the comma separated list of names acquired
envsToSet.add(new StringParameterValue(
resources.requiredVar,
required.stream()
.map(LockableResource::getName)
.collect(Collectors.joining(","))));

// also add a numbered variable for each acquired lock along with properties of the lock
int index = 0;
for (LockableResource lr : required) {
String lockEnvName = resources.requiredVar + index;
envsToSet.add(new StringParameterValue(lockEnvName, lr.getName()));
for (LockableResourceProperty lockProperty : lr.getProperties()) {
String propEnvName = lockEnvName + "_" + lockProperty.getName();
envsToSet.add(new StringParameterValue(propEnvName, lockProperty.getValue()));
// also add a numbered variable for each acquired lock along with properties of the lock
int index = 0;
for (LockableResource lr : required) {
String lockEnvName = resources.requiredVar + index;
envsToSet.add(new StringParameterValue(lockEnvName, lr.getName()));
for (LockableResourceProperty lockProperty : lr.getProperties()) {
String propEnvName = lockEnvName + "_" + lockProperty.getName();
envsToSet.add(new StringParameterValue(propEnvName, lockProperty.getValue()));
}
++index;
}
++index;
}

build.addAction(new ResourceVariableNameAction(envsToSet));
build.addAction(new ResourceVariableNameAction(envsToSet));
}
} else {
listener.getLogger().printf("%s failed to lock %s%n", LOG_PREFIX, required);
LOGGER.warning(build.getFullDisplayName() + " failed to lock " + required);

Check warning on line 88 in src/main/java/org/jenkins/plugins/lockableresources/queue/LockRunListener.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line 

Line 88 is not covered by tests
}
} else {
listener.getLogger().printf("%s failed to lock %s%n", LOG_PREFIX, required);
LOGGER.fine(build.getFullDisplayName() + " failed to lock " + required);
}
}
}
Expand All @@ -93,12 +98,18 @@ public void onCompleted(Run<?, ?> build, @NonNull TaskListener listener) {
// only the child jobs will actually unlock resources.
if (build instanceof MatrixBuild) return;

// obviously project name cannot be obtained here
List<LockableResource> required = LockableResourcesManager.get().getResourcesFromBuild(build);
if (!required.isEmpty()) {
LockableResourcesManager.get().unlock(required, build);
listener.getLogger().printf("%s released lock on %s%n", LOG_PREFIX, required);
LOGGER.fine(build.getFullDisplayName() + " released lock on " + required);
LockableResourcesManager lrm = LockableResourcesManager.get();
synchronized (lrm.syncResources) {
// obviously project name cannot be obtained here
List<LockableResource> required = lrm.getResourcesFromBuild(build);
if (!required.isEmpty()) {
lrm.unlock(required, build);
listener.getLogger().printf("%s released lock on %s%n", LOG_PREFIX, required);
LOGGER.info(build.getFullDisplayName()
+ " released lock on "
+ required
+ ", because the build has been finished.");
}
}
}

Expand All @@ -107,11 +118,16 @@ public void onDeleted(Run<?, ?> build) {
// Skip unlocking for multiple configuration projects,
// only the child jobs will actually unlock resources.
if (build instanceof MatrixBuild) return;

List<LockableResource> required = LockableResourcesManager.get().getResourcesFromBuild(build);
if (!required.isEmpty()) {
LockableResourcesManager.get().unlock(required, build);
LOGGER.fine(build.getFullDisplayName() + " released lock on " + required);
LockableResourcesManager lrm = LockableResourcesManager.get();
synchronized (lrm.syncResources) {
List<LockableResource> required = lrm.getResourcesFromBuild(build);
if (!required.isEmpty()) {
lrm.unlock(required, build);
LOGGER.warning(build.getFullDisplayName()
+ " released lock on "
+ required
+ ", because the build has been deleted.");
}
}
}
}

0 comments on commit 1b23794

Please sign in to comment.