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

commit 43c379104db4c30d80df32c6beb8581e25e98735
Author: remm <r...@apache.org>
AuthorDate: Thu Aug 26 16:40:44 2021 +0200

    Add lock use similar to the memory user database
    
    Remove duplicated code, I verified the memory MBean works fine.
---
 .../catalina/mbeans/MemoryUserDatabaseMBean.java   | 277 +----------
 .../catalina/mbeans/SparseUserDatabaseMBean.java   |  59 +--
 .../apache/catalina/realm/UserDatabaseRealm.java   |   2 +-
 .../catalina/users/DataSourceUserDatabase.java     | 553 ++++++++++++---------
 4 files changed, 354 insertions(+), 537 deletions(-)

diff --git a/java/org/apache/catalina/mbeans/MemoryUserDatabaseMBean.java 
b/java/org/apache/catalina/mbeans/MemoryUserDatabaseMBean.java
index b8fc248..7105cf8 100644
--- a/java/org/apache/catalina/mbeans/MemoryUserDatabaseMBean.java
+++ b/java/org/apache/catalina/mbeans/MemoryUserDatabaseMBean.java
@@ -16,21 +16,7 @@
  */
 package org.apache.catalina.mbeans;
 
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.List;
-
-import javax.management.MalformedObjectNameException;
-import javax.management.ObjectName;
-
-import org.apache.catalina.Group;
-import org.apache.catalina.Role;
-import org.apache.catalina.User;
-import org.apache.catalina.UserDatabase;
-import org.apache.tomcat.util.modeler.BaseModelMBean;
 import org.apache.tomcat.util.modeler.ManagedBean;
-import org.apache.tomcat.util.modeler.Registry;
-import org.apache.tomcat.util.res.StringManager;
 
 /**
  * <p>A <strong>ModelMBean</strong> implementation for the
@@ -38,272 +24,11 @@ import org.apache.tomcat.util.res.StringManager;
  *
  * @author Craig R. McClanahan
  */
-public class MemoryUserDatabaseMBean extends BaseModelMBean {
-
-    private static final StringManager sm = 
StringManager.getManager(MemoryUserDatabaseMBean.class);
-
-    // ----------------------------------------------------- Instance Variables
-
-    /**
-     * The configuration information registry for our managed beans.
-     */
-    protected final Registry registry = MBeanUtils.createRegistry();
-
+public class MemoryUserDatabaseMBean extends SparseUserDatabaseMBean {
 
     /**
      * The <code>ManagedBean</code> information describing this MBean.
      */
     protected final ManagedBean managed = 
registry.findManagedBean("MemoryUserDatabase");
 
-
-    /**
-     * The <code>ManagedBean</code> information describing Group MBeans.
-     */
-    protected final ManagedBean managedGroup = 
registry.findManagedBean("Group");
-
-
-    /**
-     * The <code>ManagedBean</code> information describing Group MBeans.
-     */
-    protected final ManagedBean managedRole = registry.findManagedBean("Role");
-
-
-    /**
-     * The <code>ManagedBean</code> information describing User MBeans.
-     */
-    protected final ManagedBean managedUser = registry.findManagedBean("User");
-
-
-    // ------------------------------------------------------------- Attributes
-
-    /**
-     * @return the MBean Names of all groups defined in this database.
-     */
-    public String[] getGroups() {
-        UserDatabase database = (UserDatabase) this.resource;
-        List<String> results = new ArrayList<>();
-        Iterator<Group> groups = database.getGroups();
-        while (groups.hasNext()) {
-            Group group = groups.next();
-            results.add(findGroup(group.getGroupname()));
-        }
-        return results.toArray(new String[0]);
-    }
-
-
-    /**
-     * @return the MBean Names of all roles defined in this database.
-     */
-    public String[] getRoles() {
-        UserDatabase database = (UserDatabase) this.resource;
-        List<String> results = new ArrayList<>();
-        Iterator<Role> roles = database.getRoles();
-        while (roles.hasNext()) {
-            Role role = roles.next();
-            results.add(findRole(role.getRolename()));
-        }
-        return results.toArray(new String[0]);
-    }
-
-
-    /**
-     * @return the MBean Names of all users defined in this database.
-     */
-    public String[] getUsers() {
-        UserDatabase database = (UserDatabase) this.resource;
-        List<String> results = new ArrayList<>();
-        Iterator<User> users = database.getUsers();
-        while (users.hasNext()) {
-            User user = users.next();
-            results.add(findUser(user.getUsername()));
-        }
-        return results.toArray(new String[0]);
-    }
-
-
-    // ------------------------------------------------------------- Operations
-
-    /**
-     * Create a new Group and return the corresponding MBean Name.
-     *
-     * @param groupname Group name of the new group
-     * @param description Description of the new group
-     * @return the new group object name
-     */
-    public String createGroup(String groupname, String description) {
-        UserDatabase database = (UserDatabase) this.resource;
-        Group group = database.createGroup(groupname, description);
-        try {
-            MBeanUtils.createMBean(group);
-        } catch (Exception e) {
-            throw new 
IllegalArgumentException(sm.getString("userMBean.createMBeanError.group", 
groupname), e);
-        }
-        return findGroup(groupname);
-    }
-
-
-    /**
-     * Create a new Role and return the corresponding MBean Name.
-     *
-     * @param rolename Group name of the new group
-     * @param description Description of the new group
-     * @return the new role object name
-     */
-    public String createRole(String rolename, String description) {
-        UserDatabase database = (UserDatabase) this.resource;
-        Role role = database.createRole(rolename, description);
-        try {
-            MBeanUtils.createMBean(role);
-        } catch (Exception e) {
-            throw new 
IllegalArgumentException(sm.getString("userMBean.createMBeanError.role", 
rolename), e);
-        }
-        return findRole(rolename);
-    }
-
-
-    /**
-     * Create a new User and return the corresponding MBean Name.
-     *
-     * @param username User name of the new user
-     * @param password Password for the new user
-     * @param fullName Full name for the new user
-     * @return the new user object name
-     */
-    public String createUser(String username, String password, String 
fullName) {
-        UserDatabase database = (UserDatabase) this.resource;
-        User user = database.createUser(username, password, fullName);
-        try {
-            MBeanUtils.createMBean(user);
-        } catch (Exception e) {
-            throw new 
IllegalArgumentException(sm.getString("userMBean.createMBeanError.user", 
username), e);
-        }
-        return findUser(username);
-    }
-
-
-    /**
-     * Return the MBean Name for the specified group name (if any);
-     * otherwise return <code>null</code>.
-     *
-     * @param groupname Group name to look up
-     * @return the group object name
-     */
-    public String findGroup(String groupname) {
-        UserDatabase database = (UserDatabase) this.resource;
-        Group group = database.findGroup(groupname);
-        if (group == null) {
-            return null;
-        }
-        try {
-            ObjectName oname = 
MBeanUtils.createObjectName(managedGroup.getDomain(), group);
-            return oname.toString();
-        } catch (MalformedObjectNameException e) {
-            throw new 
IllegalArgumentException(sm.getString("userMBean.createError.group", 
groupname), e);
-        }
-    }
-
-
-    /**
-     * Return the MBean Name for the specified role name (if any);
-     * otherwise return <code>null</code>.
-     *
-     * @param rolename Role name to look up
-     * @return the role object name
-     */
-    public String findRole(String rolename) {
-        UserDatabase database = (UserDatabase) this.resource;
-        Role role = database.findRole(rolename);
-        if (role == null) {
-            return null;
-        }
-        try {
-            ObjectName oname = 
MBeanUtils.createObjectName(managedRole.getDomain(), role);
-            return oname.toString();
-        } catch (MalformedObjectNameException e) {
-            throw new 
IllegalArgumentException(sm.getString("userMBean.createError.role", rolename), 
e);
-        }
-
-    }
-
-
-    /**
-     * Return the MBean Name for the specified user name (if any);
-     * otherwise return <code>null</code>.
-     *
-     * @param username User name to look up
-     * @return the user object name
-     */
-    public String findUser(String username) {
-        UserDatabase database = (UserDatabase) this.resource;
-        User user = database.findUser(username);
-        if (user == null) {
-            return null;
-        }
-        try {
-            ObjectName oname = 
MBeanUtils.createObjectName(managedUser.getDomain(), user);
-            return oname.toString();
-        } catch (MalformedObjectNameException e) {
-            throw new 
IllegalArgumentException(sm.getString("userMBean.createError.user", username), 
e);
-        }
-    }
-
-
-    /**
-     * Remove an existing group and destroy the corresponding MBean.
-     *
-     * @param groupname Group name to remove
-     */
-    public void removeGroup(String groupname) {
-        UserDatabase database = (UserDatabase) this.resource;
-        Group group = database.findGroup(groupname);
-        if (group == null) {
-            return;
-        }
-        try {
-            MBeanUtils.destroyMBean(group);
-            database.removeGroup(group);
-        } catch (Exception e) {
-            throw new 
IllegalArgumentException(sm.getString("userMBean.destroyError.group", 
groupname), e);
-        }
-    }
-
-
-    /**
-     * Remove an existing role and destroy the corresponding MBean.
-     *
-     * @param rolename Role name to remove
-     */
-    public void removeRole(String rolename) {
-        UserDatabase database = (UserDatabase) this.resource;
-        Role role = database.findRole(rolename);
-        if (role == null) {
-            return;
-        }
-        try {
-            MBeanUtils.destroyMBean(role);
-            database.removeRole(role);
-        } catch (Exception e) {
-            throw new 
IllegalArgumentException(sm.getString("userMBean.destroyError.role", rolename), 
e);
-        }
-    }
-
-
-    /**
-     * Remove an existing user and destroy the corresponding MBean.
-     *
-     * @param username User name to remove
-     */
-    public void removeUser(String username) {
-        UserDatabase database = (UserDatabase) this.resource;
-        User user = database.findUser(username);
-        if (user == null) {
-            return;
-        }
-        try {
-            MBeanUtils.destroyMBean(user);
-            database.removeUser(user);
-        } catch (Exception e) {
-            throw new 
IllegalArgumentException(sm.getString("userMBean.destroyError.user", username), 
e);
-        }
-    }
 }
diff --git a/java/org/apache/catalina/mbeans/SparseUserDatabaseMBean.java 
b/java/org/apache/catalina/mbeans/SparseUserDatabaseMBean.java
index 921424d..09710dd 100644
--- a/java/org/apache/catalina/mbeans/SparseUserDatabaseMBean.java
+++ b/java/org/apache/catalina/mbeans/SparseUserDatabaseMBean.java
@@ -207,7 +207,7 @@ public class SparseUserDatabaseMBean extends BaseModelMBean 
{
         }
         try {
             ObjectName oname = 
MBeanUtils.createObjectName(managedGroup.getDomain(), group);
-            if (!mserver.isRegistered(oname)) {
+            if (database.isSparse() && !mserver.isRegistered(oname)) {
                 MBeanUtils.createMBean(group);
             }
             return oname.toString();
@@ -232,7 +232,7 @@ public class SparseUserDatabaseMBean extends BaseModelMBean 
{
         }
         try {
             ObjectName oname = 
MBeanUtils.createObjectName(managedRole.getDomain(), role);
-            if (!mserver.isRegistered(oname)) {
+            if (database.isSparse() && !mserver.isRegistered(oname)) {
                 MBeanUtils.createMBean(role);
             }
             return oname.toString();
@@ -258,7 +258,7 @@ public class SparseUserDatabaseMBean extends BaseModelMBean 
{
         }
         try {
             ObjectName oname = 
MBeanUtils.createObjectName(managedUser.getDomain(), user);
-            if (!mserver.isRegistered(oname)) {
+            if (database.isSparse() && !mserver.isRegistered(oname)) {
                 MBeanUtils.createMBean(user);
             }
             return oname.toString();
@@ -334,33 +334,34 @@ public class SparseUserDatabaseMBean extends 
BaseModelMBean {
     public void save() {
         try {
             UserDatabase database = (UserDatabase) this.resource;
-            ObjectName query = null;
-            Set<ObjectName> results = null;
-
-            // Groups
-            query = new ObjectName(
-                    "Users:type=Group,database=" + database.getId() + ",*");
-            results = mserver.queryNames(query, null);
-            for (ObjectName result : results) {
-                mserver.unregisterMBean(result);
+            if (database.isSparse()) {
+                ObjectName query = null;
+                Set<ObjectName> results = null;
+
+                // Groups
+                query = new ObjectName(
+                        "Users:type=Group,database=" + database.getId() + 
",*");
+                results = mserver.queryNames(query, null);
+                for (ObjectName result : results) {
+                    mserver.unregisterMBean(result);
+                }
+
+                // Roles
+                query = new ObjectName(
+                        "Users:type=Role,database=" + database.getId() + ",*");
+                results = mserver.queryNames(query, null);
+                for (ObjectName result : results) {
+                    mserver.unregisterMBean(result);
+                }
+
+                // Users
+                query = new ObjectName(
+                        "Users:type=User,database=" + database.getId() + ",*");
+                results = mserver.queryNames(query, null);
+                for (ObjectName result : results) {
+                    mserver.unregisterMBean(result);
+                }
             }
-
-            // Roles
-            query = new ObjectName(
-                    "Users:type=Role,database=" + database.getId() + ",*");
-            results = mserver.queryNames(query, null);
-            for (ObjectName result : results) {
-                mserver.unregisterMBean(result);
-            }
-
-            // Users
-            query = new ObjectName(
-                    "Users:type=User,database=" + database.getId() + ",*");
-            results = mserver.queryNames(query, null);
-            for (ObjectName result : results) {
-                mserver.unregisterMBean(result);
-            }
-
             database.save();
         } catch (Exception e) {
             throw new 
IllegalArgumentException(sm.getString("userMBean.saveError"), e);
diff --git a/java/org/apache/catalina/realm/UserDatabaseRealm.java 
b/java/org/apache/catalina/realm/UserDatabaseRealm.java
index d7806df..c8aff94 100644
--- a/java/org/apache/catalina/realm/UserDatabaseRealm.java
+++ b/java/org/apache/catalina/realm/UserDatabaseRealm.java
@@ -202,7 +202,7 @@ public class UserDatabaseRealm extends RealmBase {
             return null;
         } else {
             if (useStaticPrincipal) {
-                return new GenericPrincipal(username, 
Arrays.asList(getRoles(user))); 
+                return new GenericPrincipal(username, 
Arrays.asList(getRoles(user)));
             } else {
                 return new UserDatabasePrincipal(user, database);
             }
diff --git a/java/org/apache/catalina/users/DataSourceUserDatabase.java 
b/java/org/apache/catalina/users/DataSourceUserDatabase.java
index 0420e51..f90d1b0 100644
--- a/java/org/apache/catalina/users/DataSourceUserDatabase.java
+++ b/java/org/apache/catalina/users/DataSourceUserDatabase.java
@@ -24,6 +24,8 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import javax.naming.Context;
 import javax.sql.DataSource;
@@ -219,6 +221,11 @@ public class DataSourceUserDatabase extends 
SparseUserDatabase {
     protected boolean readonly = true;
 
 
+    private final ReentrantReadWriteLock dbLock = new ReentrantReadWriteLock();
+    private final Lock readLock = dbLock.readLock();
+    private final Lock writeLock = dbLock.writeLock();
+
+
     // ------------------------------------------------------------- Properties
 
 
@@ -438,98 +445,110 @@ public class DataSourceUserDatabase extends 
SparseUserDatabase {
 
     @Override
     public Iterator<Group> getGroups() {
-        HashMap<String, Group> groups = new HashMap<>();
-        groups.putAll(createdGroups);
-        groups.putAll(modifiedGroups);
-
-        Connection dbConnection = openConnection();
-        if (dbConnection != null && preparedAllGroups != null) {
-            try (PreparedStatement stmt = 
dbConnection.prepareStatement(preparedAllGroups)) {
-                try (ResultSet rs = stmt.executeQuery()) {
-                    while (rs.next()) {
-                        String groupName = rs.getString(1);
-                        if (groupName != null) {
-                            if (!groups.containsKey(groupName) && 
!removedGroups.containsKey(groupName)) {
-                                Group group = findGroupInternal(dbConnection, 
groupName);
-                                if (group != null) {
-                                    groups.put(groupName, group);
+        readLock.lock();
+        try {
+            HashMap<String, Group> groups = new HashMap<>();
+            groups.putAll(createdGroups);
+            groups.putAll(modifiedGroups);
+
+            Connection dbConnection = openConnection();
+            if (dbConnection != null && preparedAllGroups != null) {
+                try (PreparedStatement stmt = 
dbConnection.prepareStatement(preparedAllGroups)) {
+                    try (ResultSet rs = stmt.executeQuery()) {
+                        while (rs.next()) {
+                            String groupName = rs.getString(1);
+                            if (groupName != null) {
+                                if (!groups.containsKey(groupName) && 
!removedGroups.containsKey(groupName)) {
+                                    Group group = 
findGroupInternal(dbConnection, groupName);
+                                    if (group != null) {
+                                        groups.put(groupName, group);
+                                    }
                                 }
                             }
                         }
                     }
+                } catch (SQLException e) {
+                    
log.error(sm.getString("dataSourceUserDatabase.exception"), e);
+                } finally {
+                    close(dbConnection);
                 }
-            } catch (SQLException e) {
-                log.error(sm.getString("dataSourceUserDatabase.exception"), e);
-            } finally {
-                close(dbConnection);
             }
+            return groups.values().iterator();
+        } finally {
+            readLock.unlock();
         }
-
-        return groups.values().iterator();
     }
 
     @Override
     public Iterator<Role> getRoles() {
-        HashMap<String, Role> roles = new HashMap<>();
-        roles.putAll(createdRoles);
-        roles.putAll(modifiedRoles);
-
-        Connection dbConnection = openConnection();
-        if (dbConnection != null && preparedAllRoles != null) {
-            try (PreparedStatement stmt = 
dbConnection.prepareStatement(preparedAllRoles)) {
-                try (ResultSet rs = stmt.executeQuery()) {
-                    while (rs.next()) {
-                        String roleName = rs.getString(1);
-                        if (roleName != null) {
-                            if (!roles.containsKey(roleName) && 
!removedRoles.containsKey(roleName)) {
-                                Role role = findRoleInternal(dbConnection, 
roleName);
-                                if (role != null) {
-                                    roles.put(roleName, role);
+        readLock.lock();
+        try {
+            HashMap<String, Role> roles = new HashMap<>();
+            roles.putAll(createdRoles);
+            roles.putAll(modifiedRoles);
+
+            Connection dbConnection = openConnection();
+            if (dbConnection != null && preparedAllRoles != null) {
+                try (PreparedStatement stmt = 
dbConnection.prepareStatement(preparedAllRoles)) {
+                    try (ResultSet rs = stmt.executeQuery()) {
+                        while (rs.next()) {
+                            String roleName = rs.getString(1);
+                            if (roleName != null) {
+                                if (!roles.containsKey(roleName) && 
!removedRoles.containsKey(roleName)) {
+                                    Role role = findRoleInternal(dbConnection, 
roleName);
+                                    if (role != null) {
+                                        roles.put(roleName, role);
+                                    }
                                 }
                             }
                         }
                     }
+                } catch (SQLException e) {
+                    
log.error(sm.getString("dataSourceUserDatabase.exception"), e);
+                } finally {
+                    close(dbConnection);
                 }
-            } catch (SQLException e) {
-                log.error(sm.getString("dataSourceUserDatabase.exception"), e);
-            } finally {
-                close(dbConnection);
             }
+            return roles.values().iterator();
+        } finally {
+            readLock.unlock();
         }
-
-        return roles.values().iterator();
     }
 
     @Override
     public Iterator<User> getUsers() {
-        HashMap<String, User> users = new HashMap<>();
-        users.putAll(createdUsers);
-        users.putAll(modifiedUsers);
-
-        Connection dbConnection = openConnection();
-        if (dbConnection != null) {
-            try (PreparedStatement stmt = 
dbConnection.prepareStatement(preparedAllUsers)) {
-                try (ResultSet rs = stmt.executeQuery()) {
-                    while (rs.next()) {
-                        String userName = rs.getString(1);
-                        if (userName != null) {
-                            if (!users.containsKey(userName) && 
!removedUsers.containsKey(userName)) {
-                                User user = findUserInternal(dbConnection, 
userName);
-                                if (user != null) {
-                                    users.put(userName, user);
+        readLock.lock();
+        try {
+            HashMap<String, User> users = new HashMap<>();
+            users.putAll(createdUsers);
+            users.putAll(modifiedUsers);
+
+            Connection dbConnection = openConnection();
+            if (dbConnection != null) {
+                try (PreparedStatement stmt = 
dbConnection.prepareStatement(preparedAllUsers)) {
+                    try (ResultSet rs = stmt.executeQuery()) {
+                        while (rs.next()) {
+                            String userName = rs.getString(1);
+                            if (userName != null) {
+                                if (!users.containsKey(userName) && 
!removedUsers.containsKey(userName)) {
+                                    User user = findUserInternal(dbConnection, 
userName);
+                                    if (user != null) {
+                                        users.put(userName, user);
+                                    }
                                 }
                             }
                         }
                     }
+                } catch (SQLException e) {
+                    
log.error(sm.getString("dataSourceUserDatabase.exception"), e);
+                } finally {
+                    close(dbConnection);
                 }
-            } catch (SQLException e) {
-                log.error(sm.getString("dataSourceUserDatabase.exception"), e);
-            } finally {
-                close(dbConnection);
             }
+            return users.values().iterator();
+        } finally {
+            readLock.unlock();
         }
-
-        return users.values().iterator();
     }
 
     @Override
@@ -538,55 +557,75 @@ public class DataSourceUserDatabase extends 
SparseUserDatabase {
 
     @Override
     public Group createGroup(String groupname, String description) {
-        Group group = new GenericGroup<DataSourceUserDatabase>(this, 
groupname, description, null);
-        createdGroups.put(groupname, group);
-        modifiedGroups.remove(groupname);
-        removedGroups.remove(groupname);
-        return group;
+        readLock.lock();
+        try {
+            Group group = new GenericGroup<DataSourceUserDatabase>(this, 
groupname, description, null);
+            createdGroups.put(groupname, group);
+            modifiedGroups.remove(groupname);
+            removedGroups.remove(groupname);
+            return group;
+        } finally {
+            readLock.unlock();
+        }
     }
 
     @Override
     public Role createRole(String rolename, String description) {
-        Role role = new GenericRole<DataSourceUserDatabase>(this, rolename, 
description);
-        createdRoles.put(rolename, role);
-        modifiedRoles.remove(rolename);
-        removedRoles.remove(rolename);
-        return role;
+        readLock.lock();
+        try {
+            Role role = new GenericRole<DataSourceUserDatabase>(this, 
rolename, description);
+            createdRoles.put(rolename, role);
+            modifiedRoles.remove(rolename);
+            removedRoles.remove(rolename);
+            return role;
+        } finally {
+            readLock.unlock();
+        }
     }
 
     @Override
     public User createUser(String username, String password, String fullName) {
-        User user = new GenericUser<DataSourceUserDatabase>(this, username, 
password, fullName, null, null);
-        createdUsers.put(username, user);
-        modifiedUsers.remove(username);
-        removedUsers.remove(username);
-        return user;
+        readLock.lock();
+        try {
+            User user = new GenericUser<DataSourceUserDatabase>(this, 
username, password, fullName, null, null);
+            createdUsers.put(username, user);
+            modifiedUsers.remove(username);
+            removedUsers.remove(username);
+            return user;
+        } finally {
+            readLock.unlock();
+        }
     }
 
     @Override
     public Group findGroup(String groupname) {
-        // Check local changes first
-        Group group = createdGroups.get(groupname);
-        if (group != null) {
-            return group;
-        }
-        group = modifiedGroups.get(groupname);
-        if (group != null) {
-            return group;
-        }
-        group = removedGroups.get(groupname);
-        if (group != null) {
-            return null;
-        }
-
-        Connection dbConnection = openConnection();
-        if (dbConnection == null) {
-            return null;
-        }
+        readLock.lock();
         try {
-            return findGroupInternal(dbConnection, groupname);
+            // Check local changes first
+            Group group = createdGroups.get(groupname);
+            if (group != null) {
+                return group;
+            }
+            group = modifiedGroups.get(groupname);
+            if (group != null) {
+                return group;
+            }
+            group = removedGroups.get(groupname);
+            if (group != null) {
+                return null;
+            }
+
+            Connection dbConnection = openConnection();
+            if (dbConnection == null) {
+                return null;
+            }
+            try {
+                return findGroupInternal(dbConnection, groupname);
+            } finally {
+                close(dbConnection);
+            }
         } finally {
-            close(dbConnection);
+            readLock.unlock();
         }
     }
 
@@ -630,28 +669,33 @@ public class DataSourceUserDatabase extends 
SparseUserDatabase {
 
     @Override
     public Role findRole(String rolename) {
-        // Check local changes first
-        Role role = createdRoles.get(rolename);
-        if (role != null) {
-            return role;
-        }
-        role = modifiedRoles.get(rolename);
-        if (role != null) {
-            return role;
-        }
-        role = removedRoles.get(rolename);
-        if (role != null) {
-            return null;
-        }
-
-        Connection dbConnection = openConnection();
-        if (dbConnection == null) {
-            return null;
-        }
+        readLock.lock();
         try {
-            return findRoleInternal(dbConnection, rolename);
+            // Check local changes first
+            Role role = createdRoles.get(rolename);
+            if (role != null) {
+                return role;
+            }
+            role = modifiedRoles.get(rolename);
+            if (role != null) {
+                return role;
+            }
+            role = removedRoles.get(rolename);
+            if (role != null) {
+                return null;
+            }
+
+            Connection dbConnection = openConnection();
+            if (dbConnection == null) {
+                return null;
+            }
+            try {
+                return findRoleInternal(dbConnection, rolename);
+            } finally {
+                close(dbConnection);
+            }
         } finally {
-            close(dbConnection);
+            readLock.unlock();
         }
     }
 
@@ -675,28 +719,33 @@ public class DataSourceUserDatabase extends 
SparseUserDatabase {
 
     @Override
     public User findUser(String username) {
-        // Check local changes first
-        User user = createdUsers.get(username);
-        if (user != null) {
-            return user;
-        }
-        user = modifiedUsers.get(username);
-        if (user != null) {
-            return user;
-        }
-        user = removedUsers.get(username);
-        if (user != null) {
-            return null;
-        }
-
-        Connection dbConnection = openConnection();
-        if (dbConnection == null) {
-            return null;
-        }
+        readLock.lock();
         try {
-            return findUserInternal(dbConnection, username);
+            // Check local changes first
+            User user = createdUsers.get(username);
+            if (user != null) {
+                return user;
+            }
+            user = modifiedUsers.get(username);
+            if (user != null) {
+                return user;
+            }
+            user = removedUsers.get(username);
+            if (user != null) {
+                return null;
+            }
+
+            Connection dbConnection = openConnection();
+            if (dbConnection == null) {
+                return null;
+            }
+            try {
+                return findUserInternal(dbConnection, username);
+            } finally {
+                close(dbConnection);
+            }
         } finally {
-            close(dbConnection);
+            readLock.unlock();
         }
     }
 
@@ -769,145 +818,182 @@ public class DataSourceUserDatabase extends 
SparseUserDatabase {
 
     @Override
     public void modifiedGroup(Group group) {
-        String name = group.getName();
-        if (!createdGroups.containsKey(name) && 
!removedGroups.containsKey(name)) {
-            modifiedGroups.put(name, group);
+        readLock.lock();
+        try {
+            String name = group.getName();
+            if (!createdGroups.containsKey(name) && 
!removedGroups.containsKey(name)) {
+                modifiedGroups.put(name, group);
+            }
+        } finally {
+            readLock.unlock();
         }
     }
 
     @Override
     public void modifiedRole(Role role) {
-        String name = role.getName();
-        if (!createdRoles.containsKey(name) && 
!removedRoles.containsKey(name)) {
-            modifiedRoles.put(name, role);
+        readLock.lock();
+        try {
+            String name = role.getName();
+            if (!createdRoles.containsKey(name) && 
!removedRoles.containsKey(name)) {
+                modifiedRoles.put(name, role);
+            }
+        } finally {
+            readLock.unlock();
         }
     }
 
     @Override
     public void modifiedUser(User user) {
-        String name = user.getName();
-        if (!createdUsers.containsKey(name) && 
!removedUsers.containsKey(name)) {
-            modifiedUsers.put(name, user);
+        readLock.lock();
+        try {
+            String name = user.getName();
+            if (!createdUsers.containsKey(name) && 
!removedUsers.containsKey(name)) {
+                modifiedUsers.put(name, user);
+            }
+        } finally {
+            readLock.unlock();
         }
     }
 
     @Override
     public void open() throws Exception {
 
-        StringBuilder temp = new StringBuilder("SELECT ");
-        temp.append(roleNameCol);
-        temp.append(" FROM ");
-        temp.append(userRoleTable);
-        temp.append(" WHERE ");
-        temp.append(userNameCol);
-        temp.append(" = ?");
-        preparedRoles = temp.toString();
+        writeLock.lock();
+        try {
 
-        if (userGroupTable != null && userGroupTable.length() > 0) {
-            temp = new StringBuilder("SELECT ");
-            temp.append(groupNameCol);
+            StringBuilder temp = new StringBuilder("SELECT ");
+            temp.append(roleNameCol);
             temp.append(" FROM ");
-            temp.append(userGroupTable);
+            temp.append(userRoleTable);
             temp.append(" WHERE ");
             temp.append(userNameCol);
             temp.append(" = ?");
-            preparedGroups = temp.toString();
-        }
+            preparedRoles = temp.toString();
+
+            if (userGroupTable != null && userGroupTable.length() > 0) {
+                temp = new StringBuilder("SELECT ");
+                temp.append(groupNameCol);
+                temp.append(" FROM ");
+                temp.append(userGroupTable);
+                temp.append(" WHERE ");
+                temp.append(userNameCol);
+                temp.append(" = ?");
+                preparedGroups = temp.toString();
+            }
 
-        if (groupRoleTable != null && groupRoleTable.length() > 0) {
-            temp = new StringBuilder("SELECT ");
-            temp.append(groupNameCol);
-            temp.append(" FROM ");
-            temp.append(groupRoleTable);
-            temp.append(" WHERE ");
-            temp.append(groupNameCol);
-            temp.append(" = ?");
-            preparedGroupsR = temp.toString();
-        }
+            if (groupRoleTable != null && groupRoleTable.length() > 0) {
+                temp = new StringBuilder("SELECT ");
+                temp.append(groupNameCol);
+                temp.append(" FROM ");
+                temp.append(groupRoleTable);
+                temp.append(" WHERE ");
+                temp.append(groupNameCol);
+                temp.append(" = ?");
+                preparedGroupsR = temp.toString();
+            }
 
-        temp = new StringBuilder("SELECT ");
-        temp.append(userCredCol);
-        if (userFullNameCol != null) {
-            temp.append(",").append(userFullNameCol);
-        }
-        temp.append(" FROM ");
-        temp.append(userTable);
-        temp.append(" WHERE ");
-        temp.append(userNameCol);
-        temp.append(" = ?");
-        preparedUser = temp.toString();
-
-        temp = new StringBuilder("SELECT ");
-        temp.append(userNameCol);
-        temp.append(" FROM ");
-        temp.append(userTable);
-        preparedAllUsers = temp.toString();
-
-        if (groupTable != null && groupTable.length() > 0) {
             temp = new StringBuilder("SELECT ");
-            temp.append(groupNameCol);
-            if (roleAndGroupDescriptionCol != null) {
-                temp.append(",").append(roleAndGroupDescriptionCol);
+            temp.append(userCredCol);
+            if (userFullNameCol != null) {
+                temp.append(",").append(userFullNameCol);
             }
             temp.append(" FROM ");
-            temp.append(groupTable);
+            temp.append(userTable);
             temp.append(" WHERE ");
-            temp.append(groupNameCol);
+            temp.append(userNameCol);
             temp.append(" = ?");
-            preparedGroup = temp.toString();
+            preparedUser = temp.toString();
 
             temp = new StringBuilder("SELECT ");
-            temp.append(groupNameCol);
+            temp.append(userNameCol);
             temp.append(" FROM ");
-            temp.append(groupTable);
-            preparedAllGroups = temp.toString();
-        }
+            temp.append(userTable);
+            preparedAllUsers = temp.toString();
 
-        if (roleTable != null && roleTable.length() > 0) {
-            // Create the role PreparedStatement string
-            temp = new StringBuilder("SELECT ");
-            temp.append(roleNameCol);
-            if (roleAndGroupDescriptionCol != null) {
-                temp.append(",").append(roleAndGroupDescriptionCol);
+            if (groupTable != null && groupTable.length() > 0) {
+                temp = new StringBuilder("SELECT ");
+                temp.append(groupNameCol);
+                if (roleAndGroupDescriptionCol != null) {
+                    temp.append(",").append(roleAndGroupDescriptionCol);
+                }
+                temp.append(" FROM ");
+                temp.append(groupTable);
+                temp.append(" WHERE ");
+                temp.append(groupNameCol);
+                temp.append(" = ?");
+                preparedGroup = temp.toString();
+
+                temp = new StringBuilder("SELECT ");
+                temp.append(groupNameCol);
+                temp.append(" FROM ");
+                temp.append(groupTable);
+                preparedAllGroups = temp.toString();
             }
-            temp.append(" FROM ");
-            temp.append(roleTable);
-            temp.append(" WHERE ");
-            temp.append(roleNameCol);
-            temp.append(" = ?");
-            preparedRole = temp.toString();
 
-            temp = new StringBuilder("SELECT ");
-            temp.append(roleNameCol);
-            temp.append(" FROM ");
-            temp.append(roleTable);
-            preparedAllRoles = temp.toString();
+            if (roleTable != null && roleTable.length() > 0) {
+                // Create the role PreparedStatement string
+                temp = new StringBuilder("SELECT ");
+                temp.append(roleNameCol);
+                if (roleAndGroupDescriptionCol != null) {
+                    temp.append(",").append(roleAndGroupDescriptionCol);
+                }
+                temp.append(" FROM ");
+                temp.append(roleTable);
+                temp.append(" WHERE ");
+                temp.append(roleNameCol);
+                temp.append(" = ?");
+                preparedRole = temp.toString();
+
+                temp = new StringBuilder("SELECT ");
+                temp.append(roleNameCol);
+                temp.append(" FROM ");
+                temp.append(roleTable);
+                preparedAllRoles = temp.toString();
+            }
+
+        } finally {
+            writeLock.unlock();
         }
 
     }
 
     @Override
     public void removeGroup(Group group) {
-        String name = group.getName();
-        createdGroups.remove(name);
-        modifiedGroups.remove(name);
-        removedGroups.put(name, group);
+        readLock.lock();
+        try {
+            String name = group.getName();
+            createdGroups.remove(name);
+            modifiedGroups.remove(name);
+            removedGroups.put(name, group);
+        } finally {
+            readLock.unlock();
+        }
     }
 
     @Override
     public void removeRole(Role role) {
-        String name = role.getName();
-        createdRoles.remove(name);
-        modifiedRoles.remove(name);
-        removedRoles.put(name, role);
+        readLock.lock();
+        try {
+            String name = role.getName();
+            createdRoles.remove(name);
+            modifiedRoles.remove(name);
+            removedRoles.put(name, role);
+        } finally {
+            readLock.unlock();
+        }
     }
 
     @Override
     public void removeUser(User user) {
-        String name = user.getName();
-        createdUsers.remove(name);
-        modifiedUsers.remove(name);
-        removedUsers.put(name, user);
+        readLock.lock();
+        try {
+            String name = user.getName();
+            createdUsers.remove(name);
+            modifiedUsers.remove(name);
+            removedUsers.put(name, user);
+        } finally {
+            readLock.unlock();
+        }
     }
 
     @Override
@@ -921,10 +1007,15 @@ public class DataSourceUserDatabase extends 
SparseUserDatabase {
             return;
         }
 
+        writeLock.lock();
         try {
-            saveInternal(dbConnection);
+            try {
+                saveInternal(dbConnection);
+            } finally {
+                close(dbConnection);
+            }
         } finally {
-            close(dbConnection);
+            writeLock.unlock();
         }
     }
 

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

Reply via email to