You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/06/24 16:23:38 UTC

[tomcat] branch 8.5.x updated (912d12f -> f83eae6)

This is an automated email from the ASF dual-hosted git repository.

markt pushed a change to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.


    from 912d12f  Clean-up. No functional change.
     new 5090469  Clean-up. No functional change.
     new 0c86919  Review thread-safety Document locking strategy
     new f83eae6  Clean-up. No functional change.

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/catalina/realm/UserDatabaseRealm.java   |  95 ++++----
 .../apache/catalina/users/MemoryUserDatabase.java  | 252 ++++++++++++++-------
 2 files changed, 203 insertions(+), 144 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 01/03: Clean-up. No functional change.

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 50904699381831df6ea6bede68f56ee288dede34
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Sep 14 07:27:09 2018 +0000

    Clean-up. No functional change.
---
 .../apache/catalina/users/MemoryUserDatabase.java  | 32 ++++++++++------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/java/org/apache/catalina/users/MemoryUserDatabase.java b/java/org/apache/catalina/users/MemoryUserDatabase.java
index ea99d6d..95fdb2f 100644
--- a/java/org/apache/catalina/users/MemoryUserDatabase.java
+++ b/java/org/apache/catalina/users/MemoryUserDatabase.java
@@ -111,11 +111,10 @@ public class MemoryUserDatabase implements UserDatabase {
      */
     protected final HashMap<String, Role> roles = new HashMap<>();
 
-
     /**
      * The set of {@link User}s defined in this database, keyed by user name.
      */
-    protected final HashMap<String,User> users = new HashMap<>();
+    protected final HashMap<String, User> users = new HashMap<>();
 
 
     // ------------------------------------------------------------- Properties
@@ -304,8 +303,8 @@ public class MemoryUserDatabase implements UserDatabase {
 
 
     /**
-     * Return the {@link Role} with the specified role name, if any;
-     * otherwise return <code>null</code>.
+     * Return the {@link Role} with the specified role name, if any; otherwise
+     * return <code>null</code>.
      *
      * @param rolename Name of the role to return
      */
@@ -318,8 +317,8 @@ public class MemoryUserDatabase implements UserDatabase {
 
 
     /**
-     * Return the {@link User} with the specified user name, if any;
-     * otherwise return <code>null</code>.
+     * Return the {@link User} with the specified user name, if any; otherwise
+     * return <code>null</code>.
      *
      * @param username Name of the user to return
      */
@@ -480,8 +479,10 @@ public class MemoryUserDatabase implements UserDatabase {
             // Print the file prolog
             writer.println("<?xml version='1.0' encoding='utf-8'?>");
             writer.println("<tomcat-users xmlns=\"http://tomcat.apache.org/xml\"");
-            writer.println("              xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"");
-            writer.println("              xsi:schemaLocation=\"http://tomcat.apache.org/xml tomcat-users.xsd\"");
+            writer.print("              ");
+            writer.println("xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"");
+            writer.print("              ");
+            writer.println("xsi:schemaLocation=\"http://tomcat.apache.org/xml tomcat-users.xsd\"");
             writer.println("              version=\"1.0\">");
 
             // Print entries for each defined role, group, and user
@@ -509,9 +510,8 @@ public class MemoryUserDatabase implements UserDatabase {
             if (writer.checkError()) {
                 writer.close();
                 fileNew.delete();
-                throw new IOException
-                    (sm.getString("memoryUserDatabase.writeException",
-                                  fileNew.getAbsolutePath()));
+                throw new IOException(sm.getString("memoryUserDatabase.writeException",
+                        fileNew.getAbsolutePath()));
             }
             writer.close();
         } catch (IOException e) {
@@ -536,18 +536,16 @@ public class MemoryUserDatabase implements UserDatabase {
         if (fileOrig.exists()) {
             fileOld.delete();
             if (!fileOrig.renameTo(fileOld)) {
-                throw new IOException
-                    (sm.getString("memoryUserDatabase.renameOld",
-                                  fileOld.getAbsolutePath()));
+                throw new IOException(sm.getString("memoryUserDatabase.renameOld",
+                        fileOld.getAbsolutePath()));
             }
         }
         if (!fileNew.renameTo(fileOrig)) {
             if (fileOld.exists()) {
                 fileOld.renameTo(fileOrig);
             }
-            throw new IOException
-                (sm.getString("memoryUserDatabase.renameNew",
-                              fileOrig.getAbsolutePath()));
+            throw new IOException(sm.getString("memoryUserDatabase.renameNew",
+                    fileOrig.getAbsolutePath()));
         }
         fileOld.delete();
 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 03/03: Clean-up. No functional change.

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit f83eae67d88224c05b780255b832f68ae6531ad6
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Jun 24 16:42:28 2019 +0100

    Clean-up. No functional change.
---
 .../apache/catalina/realm/UserDatabaseRealm.java   | 95 +++++++++-------------
 1 file changed, 39 insertions(+), 56 deletions(-)

diff --git a/java/org/apache/catalina/realm/UserDatabaseRealm.java b/java/org/apache/catalina/realm/UserDatabaseRealm.java
index 9ec96af..064ac59 100644
--- a/java/org/apache/catalina/realm/UserDatabaseRealm.java
+++ b/java/org/apache/catalina/realm/UserDatabaseRealm.java
@@ -14,11 +14,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
-
 package org.apache.catalina.realm;
 
-
 import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Iterator;
@@ -34,31 +31,26 @@ import org.apache.catalina.UserDatabase;
 import org.apache.catalina.Wrapper;
 import org.apache.tomcat.util.ExceptionUtils;
 
-
 /**
- * <p>Implementation of {@link org.apache.catalina.Realm} that is based on an implementation of
- * {@link UserDatabase} made available through the global JNDI resources
- * configured for this instance of Catalina.  Set the <code>resourceName</code>
- * parameter to the global JNDI resources name for the configured instance
- * of <code>UserDatabase</code> that we should consult.</p>
+ * Implementation of {@link org.apache.catalina.Realm} that is based on an
+ * implementation of {@link UserDatabase} made available through the global JNDI
+ * resources configured for this instance of Catalina. Set the
+ * <code>resourceName</code> parameter to the global JNDI resources name for the
+ * configured instance of <code>UserDatabase</code> that we should consult.
  *
  * @author Craig R. McClanahan
  * @since 4.1
  */
-public class UserDatabaseRealm
-    extends RealmBase {
-
+public class UserDatabaseRealm extends RealmBase {
 
     // ----------------------------------------------------- Instance Variables
 
-
     /**
-     * The <code>UserDatabase</code> we will use to authenticate users
-     * and identify associated roles.
+     * The <code>UserDatabase</code> we will use to authenticate users and
+     * identify associated roles.
      */
     protected UserDatabase database = null;
 
-
     /**
      * Descriptive information about this Realm implementation.
      * @deprecated This will be removed in Tomcat 9 onwards.
@@ -68,8 +60,8 @@ public class UserDatabaseRealm
 
 
     /**
-     * The global JNDI name of the <code>UserDatabase</code> resource
-     * we will be utilizing.
+     * The global JNDI name of the <code>UserDatabase</code> resource we will be
+     * utilizing.
      */
     protected String resourceName = "UserDatabase";
 
@@ -77,38 +69,33 @@ public class UserDatabaseRealm
     // ------------------------------------------------------------- Properties
 
     /**
-     * @return the global JNDI name of the <code>UserDatabase</code> resource
-     * we will be using.
+     * @return the global JNDI name of the <code>UserDatabase</code> resource we
+     *         will be using.
      */
     public String getResourceName() {
-
         return resourceName;
-
     }
 
 
     /**
-     * Set the global JNDI name of the <code>UserDatabase</code> resource
-     * we will be using.
+     * Set the global JNDI name of the <code>UserDatabase</code> resource we
+     * will be using.
      *
      * @param resourceName The new global JNDI name
      */
     public void setResourceName(String resourceName) {
-
         this.resourceName = resourceName;
-
     }
 
 
     // --------------------------------------------------------- Public Methods
 
-
     /**
      * Return <code>true</code> if the specified Principal has the specified
      * security role, within the context of this Realm; otherwise return
-     * <code>false</code>. This implementation returns <code>true</code>
-     * if the <code>User</code> has the role, or if any <code>Group</code>
-     * that the <code>User</code> is a member of has the role.
+     * <code>false</code>. This implementation returns <code>true</code> if the
+     * <code>User</code> has the role, or if any <code>Group</code> that the
+     * <code>User</code> is a member of has the role.
      *
      * @param principal Principal for whom the role is to be checked
      * @param role Security role to be checked
@@ -121,41 +108,41 @@ public class UserDatabaseRealm
             if (realRole != null)
                 role = realRole;
         }
-        if( principal instanceof GenericPrincipal) {
-            GenericPrincipal gp = (GenericPrincipal)principal;
-            if(gp.getUserPrincipal() instanceof User) {
+        if (principal instanceof GenericPrincipal) {
+            GenericPrincipal gp = (GenericPrincipal) principal;
+            if (gp.getUserPrincipal() instanceof User) {
                 principal = gp.getUserPrincipal();
             }
         }
-        if(! (principal instanceof User) ) {
-            //Play nice with SSO and mixed Realms
+        if (!(principal instanceof User)) {
+            // Play nice with SSO and mixed Realms
             return super.hasRole(null, principal, role);
         }
-        if("*".equals(role)) {
+        if ("*".equals(role)) {
             return true;
-        } else if(role == null) {
+        } else if (role == null) {
             return false;
         }
-        User user = (User)principal;
+        User user = (User) principal;
         Role dbrole = database.findRole(role);
-        if(dbrole == null) {
+        if (dbrole == null) {
             return false;
         }
-        if(user.isInRole(dbrole)) {
+        if (user.isInRole(dbrole)) {
             return true;
         }
         Iterator<Group> groups = user.getGroups();
-        while(groups.hasNext()) {
+        while (groups.hasNext()) {
             Group group = groups.next();
-            if(group.isInRole(dbrole)) {
+            if (group.isInRole(dbrole)) {
                 return true;
             }
         }
         return false;
     }
 
-    // ------------------------------------------------------ Protected Methods
 
+    // ------------------------------------------------------ Protected Methods
 
     @Override
     @Deprecated
@@ -186,21 +173,21 @@ public class UserDatabaseRealm
     protected Principal getPrincipal(String username) {
 
         User user = database.findUser(username);
-        if(user == null) {
+        if (user == null) {
             return null;
         }
 
         List<String> roles = new ArrayList<>();
         Iterator<Role> uroles = user.getRoles();
-        while(uroles.hasNext()) {
+        while (uroles.hasNext()) {
             Role role = uroles.next();
             roles.add(role.getName());
         }
         Iterator<Group> groups = user.getGroups();
-        while(groups.hasNext()) {
+        while (groups.hasNext()) {
             Group group = groups.next();
             uroles = group.getRoles();
-            while(uroles.hasNext()) {
+            while (uroles.hasNext()) {
                 Role role = uroles.next();
                 roles.add(role.getName());
             }
@@ -211,14 +198,13 @@ public class UserDatabaseRealm
 
     // ------------------------------------------------------ Lifecycle Methods
 
-
     /**
      * Prepare for the beginning of active use of the public methods of this
      * component and implement the requirements of
      * {@link org.apache.catalina.util.LifecycleBase#startInternal()}.
      *
      * @exception LifecycleException if this component detects a fatal error
-     *  that prevents this component from being used
+     *                that prevents this component from being used
      */
     @Override
     protected void startInternal() throws LifecycleException {
@@ -228,14 +214,12 @@ public class UserDatabaseRealm
             database = (UserDatabase) context.lookup(resourceName);
         } catch (Throwable e) {
             ExceptionUtils.handleThrowable(e);
-            containerLog.error(sm.getString("userDatabaseRealm.lookup",
-                                            resourceName),
-                               e);
+            containerLog.error(sm.getString("userDatabaseRealm.lookup", resourceName), e);
             database = null;
         }
         if (database == null) {
-            throw new LifecycleException
-                (sm.getString("userDatabaseRealm.noDatabase", resourceName));
+            throw new LifecycleException(
+                    sm.getString("userDatabaseRealm.noDatabase", resourceName));
         }
 
         super.startInternal();
@@ -248,7 +232,7 @@ public class UserDatabaseRealm
      * {@link org.apache.catalina.util.LifecycleBase#stopInternal()}.
      *
      * @exception LifecycleException if this component detects a fatal error
-     *  that needs to be reported
+     *                that needs to be reported
      */
     @Override
     protected void stopInternal() throws LifecycleException {
@@ -258,6 +242,5 @@ public class UserDatabaseRealm
 
         // Release reference to our user database
         database = null;
-
     }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 02/03: Review thread-safety Document locking strategy

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 0c86919128fd90a4904597403c6ffe24469f78d0
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Sep 14 08:41:02 2018 +0000

    Review thread-safety Document locking strategy
    
    Fix a few issues:
    - Iterators could throw ConcurrentModificationException
    - Syncs on open/save didn't lock roles Map
    Update with a view to supporting runtime reloading (BZ 58590)
---
 .../apache/catalina/users/MemoryUserDatabase.java  | 222 ++++++++++++++-------
 1 file changed, 150 insertions(+), 72 deletions(-)

diff --git a/java/org/apache/catalina/users/MemoryUserDatabase.java b/java/org/apache/catalina/users/MemoryUserDatabase.java
index 95fdb2f..dccfff0 100644
--- a/java/org/apache/catalina/users/MemoryUserDatabase.java
+++ b/java/org/apache/catalina/users/MemoryUserDatabase.java
@@ -22,8 +22,12 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
-import java.util.HashMap;
+import java.util.ArrayList;
 import java.util.Iterator;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.apache.catalina.Globals;
 import org.apache.catalina.Group;
@@ -42,10 +46,34 @@ import org.xml.sax.Attributes;
  * Concrete implementation of {@link UserDatabase} that loads all defined users,
  * groups, and roles into an in-memory data structure, and uses a specified XML
  * file for its persistent storage.
+ * <p>
+ * This class is thread-safe.
+ * <p>
+ * This class does not enforce what, in an RDBMS, would be called referential
+ * integrity. Concurrent modifications may result in inconsistent data such as
+ * a User retaining a reference to a Role that has been removed from the
+ * database.
  *
  * @author Craig R. McClanahan
  * @since 4.1
  */
+/*
+ * Implementation notes:
+ *
+ * Any operation that acts on a single element of the database (e.g. operations
+ * that create, read, update or delete a user, role or group) must first obtain
+ * the read lock. Operations that return iterators for users, roles or groups
+ * also fall into this category.
+ *
+ * Iterators must always be created from copies of the data to prevent possible
+ * corruption of the iterator due to the remove of all elements from the
+ * underlying Map that would occur during a subsequent re-loading of the
+ * database.
+ *
+ * Any operation that acts on multiple elements and expects the database to
+ * remain consistent during the operation (e.g. saving or loading the database)
+ * must first obtain the write lock.
+ */
 public class MemoryUserDatabase implements UserDatabase {
 
     private static final Log log = LogFactory.getLog(MemoryUserDatabase.class);
@@ -76,7 +104,7 @@ public class MemoryUserDatabase implements UserDatabase {
     /**
      * The set of {@link Group}s defined in this database, keyed by group name.
      */
-    protected final HashMap<String, Group> groups = new HashMap<>();
+    protected final Map<String, Group> groups = new ConcurrentHashMap<>();
 
     /**
      * The unique global identifier of this user database.
@@ -109,12 +137,16 @@ public class MemoryUserDatabase implements UserDatabase {
     /**
      * The set of {@link Role}s defined in this database, keyed by role name.
      */
-    protected final HashMap<String, Role> roles = new HashMap<>();
+    protected final Map<String, Role> roles = new ConcurrentHashMap<>();
 
     /**
      * The set of {@link User}s defined in this database, keyed by user name.
      */
-    protected final HashMap<String, User> users = new HashMap<>();
+    protected final Map<String, User> users = new ConcurrentHashMap<>();
+
+    private final ReentrantReadWriteLock dbLock = new ReentrantReadWriteLock();
+    private final Lock readLock = dbLock.readLock();
+    private final Lock writeLock = dbLock.writeLock();
 
 
     // ------------------------------------------------------------- Properties
@@ -124,8 +156,11 @@ public class MemoryUserDatabase implements UserDatabase {
      */
     @Override
     public Iterator<Group> getGroups() {
-        synchronized (groups) {
-            return groups.values().iterator();
+        readLock.lock();
+        try {
+            return new ArrayList<>(groups.values()).iterator();
+        } finally {
+            readLock.unlock();
         }
     }
 
@@ -182,8 +217,11 @@ public class MemoryUserDatabase implements UserDatabase {
      */
     @Override
     public Iterator<Role> getRoles() {
-        synchronized (roles) {
-            return roles.values().iterator();
+        readLock.lock();
+        try {
+            return new ArrayList<>(roles.values()).iterator();
+        } finally {
+            readLock.unlock();
         }
     }
 
@@ -193,8 +231,11 @@ public class MemoryUserDatabase implements UserDatabase {
      */
     @Override
     public Iterator<User> getUsers() {
-        synchronized (users) {
-            return users.values().iterator();
+        readLock.lock();
+        try {
+            return new ArrayList<>(users.values()).iterator();
+        } finally {
+            readLock.unlock();
         }
     }
 
@@ -209,13 +250,14 @@ public class MemoryUserDatabase implements UserDatabase {
     @Override
     public void close() throws Exception {
 
-        save();
-
-        synchronized (groups) {
-            synchronized (users) {
-                users.clear();
-                groups.clear();
-            }
+        writeLock.lock();
+        try {
+            save();
+            users.clear();
+            groups.clear();
+            roles.clear();
+        } finally {
+            writeLock.unlock();
         }
     }
 
@@ -235,8 +277,11 @@ public class MemoryUserDatabase implements UserDatabase {
         }
 
         MemoryGroup group = new MemoryGroup(this, groupname, description);
-        synchronized (groups) {
+        readLock.lock();
+        try {
             groups.put(group.getGroupname(), group);
+        } finally {
+            readLock.unlock();
         }
         return group;
     }
@@ -257,8 +302,11 @@ public class MemoryUserDatabase implements UserDatabase {
         }
 
         MemoryRole role = new MemoryRole(this, rolename, description);
-        synchronized (roles) {
+        readLock.lock();
+        try {
             roles.put(role.getRolename(), role);
+        } finally {
+            readLock.unlock();
         }
         return role;
     }
@@ -281,8 +329,11 @@ public class MemoryUserDatabase implements UserDatabase {
         }
 
         MemoryUser user = new MemoryUser(this, username, password, fullName);
-        synchronized (users) {
+        readLock.lock();
+        try {
             users.put(user.getUsername(), user);
+        } finally {
+            readLock.unlock();
         }
         return user;
     }
@@ -296,8 +347,11 @@ public class MemoryUserDatabase implements UserDatabase {
      */
     @Override
     public Group findGroup(String groupname) {
-        synchronized (groups) {
+        readLock.lock();
+        try {
             return groups.get(groupname);
+        } finally {
+            readLock.unlock();
         }
     }
 
@@ -310,8 +364,11 @@ public class MemoryUserDatabase implements UserDatabase {
      */
     @Override
     public Role findRole(String rolename) {
-        synchronized (roles) {
+        readLock.lock();
+        try {
             return roles.get(rolename);
+        } finally {
+            readLock.unlock();
         }
     }
 
@@ -324,8 +381,11 @@ public class MemoryUserDatabase implements UserDatabase {
      */
     @Override
     public User findUser(String username) {
-        synchronized (users) {
+        readLock.lock();
+        try {
             return users.get(username);
+        } finally {
+            readLock.unlock();
         }
     }
 
@@ -337,38 +397,43 @@ public class MemoryUserDatabase implements UserDatabase {
      */
     @Override
     public void open() throws Exception {
-        synchronized (groups) {
-            synchronized (users) {
-
-                // Erase any previous groups and users
+        writeLock.lock();
+        try {
+            // Erase any previous groups and users
+            users.clear();
+            groups.clear();
+            roles.clear();
+
+            String pathName = getPathname();
+            try (InputStream is = ConfigFileLoader.getInputStream(getPathname())) {
+                // Construct a digester to read the XML input file
+                Digester digester = new Digester();
+                try {
+                    digester.setFeature(
+                            "http://apache.org/xml/features/allow-java-encodings", true);
+                } catch (Exception e) {
+                    log.warn(sm.getString("memoryUserDatabase.xmlFeatureEncoding"), e);
+                }
+                digester.addFactoryCreate("tomcat-users/group",
+                        new MemoryGroupCreationFactory(this), true);
+                digester.addFactoryCreate("tomcat-users/role",
+                        new MemoryRoleCreationFactory(this), true);
+                digester.addFactoryCreate("tomcat-users/user",
+                        new MemoryUserCreationFactory(this), true);
+
+                // Parse the XML input to load this database
+                digester.parse(is);
+            } catch (IOException ioe) {
+                log.error(sm.getString("memoryUserDatabase.fileNotFound", pathName));
+            } catch (Exception e) {
+                // Fail safe on error
                 users.clear();
                 groups.clear();
                 roles.clear();
-
-                String pathName = getPathname();
-                try (InputStream is = ConfigFileLoader.getInputStream(getPathname())) {
-                    // Construct a digester to read the XML input file
-                    Digester digester = new Digester();
-                    try {
-                        digester.setFeature(
-                                "http://apache.org/xml/features/allow-java-encodings", true);
-                    } catch (Exception e) {
-                        log.warn(sm.getString("memoryUserDatabase.xmlFeatureEncoding"), e);
-                    }
-                    digester.addFactoryCreate("tomcat-users/group",
-                            new MemoryGroupCreationFactory(this), true);
-                    digester.addFactoryCreate("tomcat-users/role",
-                            new MemoryRoleCreationFactory(this), true);
-                    digester.addFactoryCreate("tomcat-users/user",
-                            new MemoryUserCreationFactory(this), true);
-
-                    // Parse the XML input to load this database
-                    digester.parse(is);
-                } catch (IOException ioe) {
-                    log.error(sm.getString("memoryUserDatabase.fileNotFound", pathName));
-                    return;
-                }
+                throw e;
             }
+        } finally {
+            writeLock.unlock();
         }
     }
 
@@ -380,13 +445,16 @@ public class MemoryUserDatabase implements UserDatabase {
      */
     @Override
     public void removeGroup(Group group) {
-        synchronized (groups) {
+        readLock.lock();
+        try {
             Iterator<User> users = getUsers();
             while (users.hasNext()) {
                 User user = users.next();
                 user.removeGroup(group);
             }
             groups.remove(group.getGroupname());
+        } finally {
+            readLock.unlock();
         }
     }
 
@@ -398,7 +466,8 @@ public class MemoryUserDatabase implements UserDatabase {
      */
     @Override
     public void removeRole(Role role) {
-        synchronized (roles) {
+        readLock.lock();
+        try {
             Iterator<Group> groups = getGroups();
             while (groups.hasNext()) {
                 Group group = groups.next();
@@ -410,6 +479,8 @@ public class MemoryUserDatabase implements UserDatabase {
                 user.removeRole(role);
             }
             roles.remove(role.getRolename());
+        } finally {
+            readLock.unlock();
         }
     }
 
@@ -421,8 +492,11 @@ public class MemoryUserDatabase implements UserDatabase {
      */
     @Override
     public void removeUser(User user) {
-        synchronized (users) {
+        readLock.lock();
+        try {
             users.remove(user.getUsername());
+        } finally {
+            readLock.unlock();
         }
     }
 
@@ -485,22 +559,27 @@ public class MemoryUserDatabase implements UserDatabase {
             writer.println("xsi:schemaLocation=\"http://tomcat.apache.org/xml tomcat-users.xsd\"");
             writer.println("              version=\"1.0\">");
 
-            // Print entries for each defined role, group, and user
-            Iterator<?> values = null;
-            values = getRoles();
-            while (values.hasNext()) {
-                writer.print("  ");
-                writer.println(values.next());
-            }
-            values = getGroups();
-            while (values.hasNext()) {
-                writer.print("  ");
-                writer.println(values.next());
-            }
-            values = getUsers();
-            while (values.hasNext()) {
-                writer.print("  ");
-                writer.println(((MemoryUser) values.next()).toXml());
+            writeLock.lock();
+            try {
+                // Print entries for each defined role, group, and user
+                Iterator<?> values = null;
+                values = getRoles();
+                while (values.hasNext()) {
+                    writer.print("  ");
+                    writer.println(values.next());
+                }
+                values = getGroups();
+                while (values.hasNext()) {
+                    writer.print("  ");
+                    writer.println(values.next());
+                }
+                values = getUsers();
+                while (values.hasNext()) {
+                    writer.print("  ");
+                    writer.println(((MemoryUser) values.next()).toXml());
+                }
+            } finally {
+                writeLock.unlock();
             }
 
             // Print the file epilog
@@ -530,8 +609,7 @@ public class MemoryUserDatabase implements UserDatabase {
         fileOld.delete();
         File fileOrig = new File(pathname);
         if (!fileOrig.isAbsolute()) {
-            fileOrig =
-                new File(System.getProperty(Globals.CATALINA_BASE_PROP), pathname);
+            fileOrig = new File(System.getProperty(Globals.CATALINA_BASE_PROP), pathname);
         }
         if (fileOrig.exists()) {
             fileOld.delete();


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org