This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.0.x by this push:
new 2fe9eb5 Avoid synchronization on roles verification
2fe9eb5 is described below
commit 2fe9eb5ce61b3153ca13d32d9b000db8322d8ffd
Author: remm <r...@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 07f3e5b..8069594 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -149,6 +149,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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
