You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2021/06/04 08:14:56 UTC
[tomcat] branch 8.5.x updated: Avoid synchronization on roles
verification
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 688198f Avoid synchronization on roles verification
688198f is described below
commit 688198f66f9deb831f1c6ea4cf3148139b256e67
Author: remm <re...@apache.org>
AuthorDate: Fri Jun 4 10:08:20 2021 +0200
Avoid synchronization on roles verification
For the memory UserDatabase. The amount of synchronization blocks needed
to browse through groups and check roles looked excessive.
---
java/org/apache/catalina/users/MemoryGroup.java | 25 +++-------
java/org/apache/catalina/users/MemoryUser.java | 62 ++++++-------------------
webapps/docs/changelog.xml | 4 ++
3 files changed, 24 insertions(+), 67 deletions(-)
diff --git a/java/org/apache/catalina/users/MemoryGroup.java b/java/org/apache/catalina/users/MemoryGroup.java
index d773ca2..97d499b 100644
--- a/java/org/apache/catalina/users/MemoryGroup.java
+++ b/java/org/apache/catalina/users/MemoryGroup.java
@@ -20,6 +20,7 @@ package org.apache.catalina.users;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
import org.apache.catalina.Role;
import org.apache.catalina.User;
@@ -72,7 +73,7 @@ public class MemoryGroup extends AbstractGroup {
/**
* The set of {@link Role}s associated with this group.
*/
- protected final ArrayList<Role> roles = new ArrayList<>();
+ protected final CopyOnWriteArrayList<Role> roles = new CopyOnWriteArrayList<>();
// ------------------------------------------------------------- Properties
@@ -83,9 +84,7 @@ public class MemoryGroup extends AbstractGroup {
*/
@Override
public Iterator<Role> getRoles() {
- synchronized (roles) {
- return roles.iterator();
- }
+ return roles.iterator();
}
@@ -125,11 +124,7 @@ public class MemoryGroup extends AbstractGroup {
*/
@Override
public void addRole(Role role) {
- synchronized (roles) {
- if (!roles.contains(role)) {
- roles.add(role);
- }
- }
+ roles.addIfAbsent(role);
}
@@ -140,9 +135,7 @@ public class MemoryGroup extends AbstractGroup {
*/
@Override
public boolean isInRole(Role role) {
- synchronized (roles) {
- return roles.contains(role);
- }
+ return roles.contains(role);
}
@@ -153,9 +146,7 @@ public class MemoryGroup extends AbstractGroup {
*/
@Override
public void removeRole(Role role) {
- synchronized (roles) {
- roles.remove(role);
- }
+ roles.remove(role);
}
@@ -164,9 +155,7 @@ public class MemoryGroup extends AbstractGroup {
*/
@Override
public void removeRoles() {
- synchronized (roles) {
- roles.clear();
- }
+ roles.clear();
}
diff --git a/java/org/apache/catalina/users/MemoryUser.java b/java/org/apache/catalina/users/MemoryUser.java
index 4eb5422..376f0ca 100644
--- a/java/org/apache/catalina/users/MemoryUser.java
+++ b/java/org/apache/catalina/users/MemoryUser.java
@@ -17,8 +17,8 @@
package org.apache.catalina.users;
-import java.util.ArrayList;
import java.util.Iterator;
+import java.util.concurrent.CopyOnWriteArrayList;
import org.apache.catalina.Group;
import org.apache.catalina.Role;
@@ -73,13 +73,13 @@ public class MemoryUser extends AbstractUser {
/**
* The set of {@link Group}s that this user is a member of.
*/
- protected final ArrayList<Group> groups = new ArrayList<>();
+ protected final CopyOnWriteArrayList<Group> groups = new CopyOnWriteArrayList<>();
/**
* The set of {@link Role}s associated with this user.
*/
- protected final ArrayList<Role> roles = new ArrayList<>();
+ protected final CopyOnWriteArrayList<Role> roles = new CopyOnWriteArrayList<>();
// ------------------------------------------------------------- Properties
@@ -90,9 +90,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public Iterator<Group> getGroups() {
- synchronized (groups) {
- return groups.iterator();
- }
+ return groups.iterator();
}
@@ -101,9 +99,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public Iterator<Role> getRoles() {
- synchronized (roles) {
- return roles.iterator();
- }
+ return roles.iterator();
}
@@ -126,13 +122,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void addGroup(Group group) {
-
- synchronized (groups) {
- if (!groups.contains(group)) {
- groups.add(group);
- }
- }
-
+ groups.addIfAbsent(group);
}
@@ -143,13 +133,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void addRole(Role role) {
-
- synchronized (roles) {
- if (!roles.contains(role)) {
- roles.add(role);
- }
- }
-
+ roles.addIfAbsent(role);
}
@@ -160,9 +144,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public boolean isInGroup(Group group) {
- synchronized (groups) {
- return groups.contains(group);
- }
+ return groups.contains(group);
}
@@ -175,9 +157,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public boolean isInRole(Role role) {
- synchronized (roles) {
- return roles.contains(role);
- }
+ return roles.contains(role);
}
@@ -188,11 +168,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void removeGroup(Group group) {
-
- synchronized (groups) {
- groups.remove(group);
- }
-
+ groups.remove(group);
}
@@ -201,11 +177,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void removeGroups() {
-
- synchronized (groups) {
- groups.clear();
- }
-
+ groups.clear();
}
@@ -216,11 +188,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void removeRole(Role role) {
-
- synchronized (roles) {
- roles.remove(role);
- }
-
+ roles.remove(role);
}
@@ -229,11 +197,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void removeRoles() {
-
- synchronized (roles) {
- roles.clear();
- }
-
+ roles.clear();
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index e2b2aae..ab9176d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -158,6 +158,10 @@
length needs to be represented as a long since it is larger than the
maximum value that can be represented by an int. (markt)
</fix>
+ <fix>
+ Avoid synchronization on roles verification for the memory
+ <code>UserDatabase</code>. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org