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 2020/10/19 19:19:01 UTC

[tomcat] branch 8.5.x updated: Drop pooled connections on error

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 ea3e05d  Drop pooled connections on error
ea3e05d is described below

commit ea3e05d399db31842e5f70089c92d70b01326f3c
Author: remm <re...@apache.org>
AuthorDate: Mon Oct 19 20:30:54 2020 +0200

    Drop pooled connections on error
    
    This avoids piling up on stale connections.
    Also getPassword will use the retry logic. It is only called when using
    digest it seems, so it's not used often.
---
 java/org/apache/catalina/realm/JNDIRealm.java | 76 ++++++++++++++++++---------
 webapps/docs/changelog.xml                    |  3 +-
 2 files changed, 54 insertions(+), 25 deletions(-)

diff --git a/java/org/apache/catalina/realm/JNDIRealm.java b/java/org/apache/catalina/realm/JNDIRealm.java
index 98007f7..e5d6faf 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -1309,9 +1309,10 @@ public class JNDIRealm extends RealmBase {
 
                 // close the connection so we know it will be reopened.
                 close(connection);
+                closePooledConnections();
 
                 // open a new directory context.
-                connection = get(true);
+                connection = get();
 
                 // Try the authentication again.
                 principal = authenticate(connection, username, credentials);
@@ -2198,6 +2199,20 @@ public class JNDIRealm extends RealmBase {
 
     }
 
+    /**
+     * Close all pooled connections.
+     */
+    protected void closePooledConnections() {
+        if (connectionPool != null) {
+            // Close any pooled connections as they might be bad as well
+            synchronized (connectionPool) {
+                JNDIConnection connection = null;
+                while ((connection = connectionPool.pop()) != null) {
+                    close(connection);
+                }
+            }
+        }
+    }
 
     @Override
     @Deprecated
@@ -2218,8 +2233,36 @@ public class JNDIRealm extends RealmBase {
             return null;
         }
 
+        JNDIConnection connection = null;
+        User user = null;
         try {
-            User user = getUser(get(), username, null);
+
+            // Ensure that we have a directory context available
+            connection = get();
+
+            // Occasionally the directory context will timeout.  Try one more
+            // time before giving up.
+            try {
+                user = getUser(connection, username, null);
+            } catch (NullPointerException | NamingException e) {
+                // log the exception so we know it's there.
+                containerLog.info(sm.getString("jndiRealm.exception.retry"), e);
+
+                // close the connection so we know it will be reopened.
+                close(connection);
+                closePooledConnections();
+
+                // open a new directory context.
+                connection = get();
+
+                // Try the authentication again.
+                user = getUser(connection, username, null);
+            }
+
+
+            // Release this context
+            release(connection);
+
             if (user == null) {
                 // User should be found...
                 return null;
@@ -2227,7 +2270,10 @@ public class JNDIRealm extends RealmBase {
                 // ... and have a password
                 return user.getPassword();
             }
+
         } catch (NamingException e) {
+            // Log the problem for posterity
+            containerLog.error(sm.getString("jndiRealm.exception"), e);
             return null;
         }
 
@@ -2285,6 +2331,7 @@ public class JNDIRealm extends RealmBase {
 
                 // close the connection so we know it will be reopened.
                 close(connection);
+                closePooledConnections();
 
                 // open a new directory context.
                 connection = get();
@@ -2389,28 +2436,12 @@ public class JNDIRealm extends RealmBase {
      * @exception NamingException if a directory server error occurs
      */
     protected JNDIConnection get() throws NamingException {
-        return get(false);
-    }
-
-    /**
-     * Open (if necessary) and return a connection to the configured
-     * directory server for this Realm.
-     * @param create when pooling, this forces creation of a new connection,
-     *   for example after an error
-     * @return the connection
-     * @exception NamingException if a directory server error occurs
-     */
-    protected JNDIConnection get(boolean create) throws NamingException {
         JNDIConnection connection = null;
         // Use the pool if available, otherwise use the single connection
         if (connectionPool != null) {
-            if (create) {
+            connection = connectionPool.pop();
+            if (connection == null) {
                 connection = create();
-            } else {
-                connection = connectionPool.pop();
-                if (connection == null) {
-                    connection = create();
-                }
             }
         } else {
             singleConnectionLock.lock();
@@ -2699,10 +2730,7 @@ public class JNDIRealm extends RealmBase {
             singleConnectionLock.lock();
             close(singleConnection);
         } else {
-            JNDIConnection connection = null;
-            while ((connection = connectionPool.pop()) != null) {
-                close(connection);
-            }
+            closePooledConnections();
             connectionPool = null;
         }
     }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b02f143..108a9e1 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -64,7 +64,8 @@
       <fix>
         Fix JNDIRealm pooling problems retrying on another bad connection. Any
         retries are made on a new connection, just like with the single
-        connection scenario. (remm)
+        connection scenario. Also remove all connections from the pool after
+        an error. (remm)
       </fix>
     </changelog>
   </subsection>


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