Skip to content

Commit 5be7bec

Browse files
authored
[JENKINS-75960] avoid synchronization in a roles permissions map (#419)
2 parents b986f15 + a13e875 commit 5be7bec

File tree

1 file changed

+10
-22
lines changed
  • src/main/java/com/michelin/cio/hudson/plugins/rolestrategy

1 file changed

+10
-22
lines changed

src/main/java/com/michelin/cio/hudson/plugins/rolestrategy/Role.java

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public final class Role implements Comparable {
7878
/**
7979
* {@link Permission}s hold by the role.
8080
*/
81-
private final Set<Permission> permissions = new HashSet<>();
81+
private Set<Permission> permissions;
8282

8383
private transient Integer cachedHashCode = null;
8484

@@ -143,13 +143,15 @@ public Role(String name, Pattern pattern, Set<Permission> permissions, @CheckFor
143143
this.pattern = pattern;
144144
this.description = description;
145145
this.templateName = templateName;
146+
this.permissions = new HashSet<>();
146147
for (Permission perm : permissions) {
147148
if (perm == null) {
148149
LOGGER.log(Level.WARNING, "Found some null permission(s) in role " + this.name, new IllegalArgumentException());
149150
} else {
150151
this.permissions.add(perm);
151152
}
152153
}
154+
cachedHashCode = _hashCode();
153155
}
154156

155157
public void setTemplateName(@CheckForNull String templateName) {
@@ -192,12 +194,9 @@ public Set<Permission> getPermissions() {
192194
*
193195
* Internal use only.
194196
*/
195-
private void setPermissions(Set<Permission> permissions) {
196-
synchronized (this.permissions) {
197-
this.permissions.clear();
198-
this.permissions.addAll(permissions);
199-
cachedHashCode = _hashCode();
200-
}
197+
private synchronized void setPermissions(Set<Permission> permissions) {
198+
this.permissions = new HashSet<>(permissions);
199+
cachedHashCode = _hashCode();
201200
}
202201

203202
/**
@@ -254,9 +253,7 @@ public String getDescription() {
254253
* @return True if the role holds this permission
255254
*/
256255
public Boolean hasPermission(Permission permission) {
257-
synchronized (this.permissions) {
258-
return permissions.contains(permission);
259-
}
256+
return permissions.contains(permission);
260257
}
261258

262259
/**
@@ -266,9 +263,7 @@ public Boolean hasPermission(Permission permission) {
266263
* @return True if the role holds any of the given {@link Permission}s
267264
*/
268265
public Boolean hasAnyPermission(Set<Permission> permissions) {
269-
synchronized (this.permissions) {
270-
return CollectionUtils.containsAny(this.permissions, permissions);
271-
}
266+
return CollectionUtils.containsAny(this.permissions, permissions);
272267
}
273268

274269
/**
@@ -301,9 +296,7 @@ private int _hashCode() {
301296
int hash = 7;
302297
hash = 53 * hash + (this.name != null ? this.name.hashCode() : 0);
303298
hash = 53 * hash + (this.pattern != null ? this.pattern.hashCode() : 0);
304-
synchronized (this.permissions) {
305-
hash = 53 * hash + this.permissions.hashCode();
306-
}
299+
hash = 53 * hash + this.permissions.hashCode();
307300
return hash;
308301
}
309302

@@ -325,11 +318,6 @@ public boolean equals(Object obj) {
325318
if (!Objects.equals(this.pattern, other.pattern)) {
326319
return false;
327320
}
328-
synchronized (this.permissions) {
329-
if (!Objects.equals(this.permissions, other.permissions)) {
330-
return false;
331-
}
332-
}
333-
return true;
321+
return Objects.equals(this.permissions, other.permissions);
334322
}
335323
}

0 commit comments

Comments
 (0)