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:44 UTC
[tomcat] branch 9.0.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 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 4f31c2a Avoid synchronization on roles verification
4f31c2a is described below
commit 4f31c2a56fa6a07640ecee344f1eeb19e1eaf66e
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 93dce18..d348693 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;
@@ -71,7 +72,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
@@ -82,9 +83,7 @@ public class MemoryGroup extends AbstractGroup {
*/
@Override
public Iterator<Role> getRoles() {
- synchronized (roles) {
- return roles.iterator();
- }
+ return roles.iterator();
}
@@ -124,11 +123,7 @@ public class MemoryGroup extends AbstractGroup {
*/
@Override
public void addRole(Role role) {
- synchronized (roles) {
- if (!roles.contains(role)) {
- roles.add(role);
- }
- }
+ roles.addIfAbsent(role);
}
@@ -139,9 +134,7 @@ public class MemoryGroup extends AbstractGroup {
*/
@Override
public boolean isInRole(Role role) {
- synchronized (roles) {
- return roles.contains(role);
- }
+ return roles.contains(role);
}
@@ -152,9 +145,7 @@ public class MemoryGroup extends AbstractGroup {
*/
@Override
public void removeRole(Role role) {
- synchronized (roles) {
- roles.remove(role);
- }
+ roles.remove(role);
}
@@ -163,9 +154,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 84eb77d..3609764 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;
@@ -72,13 +72,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
@@ -89,9 +89,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public Iterator<Group> getGroups() {
- synchronized (groups) {
- return groups.iterator();
- }
+ return groups.iterator();
}
@@ -100,9 +98,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public Iterator<Role> getRoles() {
- synchronized (roles) {
- return roles.iterator();
- }
+ return roles.iterator();
}
@@ -125,13 +121,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void addGroup(Group group) {
-
- synchronized (groups) {
- if (!groups.contains(group)) {
- groups.add(group);
- }
- }
-
+ groups.addIfAbsent(group);
}
@@ -142,13 +132,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void addRole(Role role) {
-
- synchronized (roles) {
- if (!roles.contains(role)) {
- roles.add(role);
- }
- }
-
+ roles.addIfAbsent(role);
}
@@ -159,9 +143,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public boolean isInGroup(Group group) {
- synchronized (groups) {
- return groups.contains(group);
- }
+ return groups.contains(group);
}
@@ -174,9 +156,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public boolean isInRole(Role role) {
- synchronized (roles) {
- return roles.contains(role);
- }
+ return roles.contains(role);
}
@@ -187,11 +167,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void removeGroup(Group group) {
-
- synchronized (groups) {
- groups.remove(group);
- }
-
+ groups.remove(group);
}
@@ -200,11 +176,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void removeGroups() {
-
- synchronized (groups) {
- groups.clear();
- }
-
+ groups.clear();
}
@@ -215,11 +187,7 @@ public class MemoryUser extends AbstractUser {
*/
@Override
public void removeRole(Role role) {
-
- synchronized (roles) {
- roles.remove(role);
- }
-
+ roles.remove(role);
}
@@ -228,11 +196,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 9c525d6..5fe4189 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