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 2021/09/28 07:26:38 UTC

[tomcat] branch 9.0.x updated: Fix BZ 65553 - add workaround for JRE triggered memory leak

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

markt 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 30aa6a9  Fix BZ 65553 - add workaround for JRE triggered memory leak
30aa6a9 is described below

commit 30aa6a99b3c896235330c783e056b8e82c87a38c
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Sep 28 08:19:14 2021 +0100

    Fix BZ 65553 - add workaround for JRE triggered memory leak
    
    https://bz.apache.org/bugzilla/show_bug.cgi?id=65553
    https://bugs.openjdk.java.net/browse/JDK-8273874
---
 java/org/apache/catalina/realm/JNDIRealm.java | 242 +++++++++++++++++++++-----
 webapps/docs/changelog.xml                    |   5 +
 2 files changed, 204 insertions(+), 43 deletions(-)

diff --git a/java/org/apache/catalina/realm/JNDIRealm.java b/java/org/apache/catalina/realm/JNDIRealm.java
index dd43c0c..6312b1f 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -22,6 +22,7 @@ import java.net.URISyntaxException;
 import java.security.KeyManagementException;
 import java.security.NoSuchAlgorithmException;
 import java.security.Principal;
+import java.security.cert.X509Certificate;
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -64,6 +65,7 @@ import javax.net.ssl.SSLSocketFactory;
 
 import org.apache.catalina.LifecycleException;
 import org.apache.tomcat.util.collections.SynchronizedStack;
+import org.ietf.jgss.GSSContext;
 import org.ietf.jgss.GSSCredential;
 import org.ietf.jgss.GSSName;
 
@@ -1215,10 +1217,19 @@ public class JNDIRealm extends RealmBase {
     @Override
     public Principal authenticate(String username, String credentials) {
 
+        ClassLoader ocl = null;
         JNDIConnection connection = null;
         Principal principal = null;
 
         try {
+            // https://bz.apache.org/bugzilla/show_bug.cgi?id=65553
+            // This can move back to open() once it is known that Tomcat must be
+            // running on a JVM that includes a fix for
+            // https://bugs.openjdk.java.net/browse/JDK-8273874
+            if (!isUseContextClassLoader()) {
+                ocl = Thread.currentThread().getContextClassLoader();
+                Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader());
+            }
 
             // Ensure that we have a directory context available
             connection = get();
@@ -1282,6 +1293,10 @@ public class JNDIRealm extends RealmBase {
                 containerLog.debug("Returning null principal.");
             }
             return null;
+        } finally {
+            if (!isUseContextClassLoader()) {
+                Thread.currentThread().setContextClassLoader(ocl);
+            }
         }
     }
 
@@ -1308,52 +1323,189 @@ public class JNDIRealm extends RealmBase {
             return null;
         }
 
-        if (userPatternArray != null) {
-            for (int curUserPattern = 0; curUserPattern < userPatternArray.length; curUserPattern++) {
-                // Retrieve user information
-                User user = getUser(connection, username, credentials, curUserPattern);
-                if (user != null) {
-                    try {
-                        // Check the user's credentials
-                        if (checkCredentials(connection.context, user, credentials)) {
-                            // Search for additional roles
-                            List<String> roles = getRoles(connection, user);
-                            if (containerLog.isDebugEnabled()) {
-                                containerLog.debug("Found roles: " + roles.toString());
+        ClassLoader ocl = null;
+        try {
+            // https://bz.apache.org/bugzilla/show_bug.cgi?id=65553
+            // This can move back to open() once it is known that Tomcat must be
+            // running on a JVM that includes a fix for
+            // https://bugs.openjdk.java.net/browse/JDK-8273874
+            if (!isUseContextClassLoader()) {
+                ocl = Thread.currentThread().getContextClassLoader();
+                Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader());
+            }
+
+            if (userPatternArray != null) {
+                for (int curUserPattern = 0; curUserPattern < userPatternArray.length; curUserPattern++) {
+                    // Retrieve user information
+                    User user = getUser(connection, username, credentials, curUserPattern);
+                    if (user != null) {
+                        try {
+                            // Check the user's credentials
+                            if (checkCredentials(connection.context, user, credentials)) {
+                                // Search for additional roles
+                                List<String> roles = getRoles(connection, user);
+                                if (containerLog.isDebugEnabled()) {
+                                    containerLog.debug("Found roles: " + roles.toString());
+                                }
+                                return new GenericPrincipal(username, credentials, roles);
                             }
-                            return new GenericPrincipal(username, credentials, roles);
+                        } catch (InvalidNameException ine) {
+                            // Log the problem for posterity
+                            containerLog.warn(sm.getString("jndiRealm.exception"), ine);
+                            // ignore; this is probably due to a name not fitting
+                            // the search path format exactly, as in a fully-
+                            // qualified name being munged into a search path
+                            // that already contains cn= or vice-versa
                         }
-                    } catch (InvalidNameException ine) {
-                        // Log the problem for posterity
-                        containerLog.warn(sm.getString("jndiRealm.exception"), ine);
-                        // ignore; this is probably due to a name not fitting
-                        // the search path format exactly, as in a fully-
-                        // qualified name being munged into a search path
-                        // that already contains cn= or vice-versa
                     }
                 }
-            }
-            return null;
-        } else {
-            // Retrieve user information
-            User user = getUser(connection, username, credentials);
-            if (user == null) {
                 return null;
+            } else {
+                // Retrieve user information
+                User user = getUser(connection, username, credentials);
+                if (user == null) {
+                    return null;
+                }
+
+                // Check the user's credentials
+                if (!checkCredentials(connection.context, user, credentials)) {
+                    return null;
+                }
+
+                // Search for additional roles
+                List<String> roles = getRoles(connection, user);
+                if (containerLog.isDebugEnabled()) {
+                    containerLog.debug("Found roles: " + roles.toString());
+                }
+
+                // Create and return a suitable Principal for this user
+                return new GenericPrincipal(username, credentials, roles);
             }
+        } finally {
+            if (!isUseContextClassLoader()) {
+                Thread.currentThread().setContextClassLoader(ocl);
+            }
+        }
+    }
 
-            // Check the user's credentials
-            if (!checkCredentials(connection.context, user, credentials)) {
-                return null;
+
+    /*
+     * https://bz.apache.org/bugzilla/show_bug.cgi?id=65553
+     * This method can be removed and the class loader switch moved back to
+     * open() once it is known that Tomcat must be running on a JVM that
+     * includes a fix for
+     * https://bugs.openjdk.java.net/browse/JDK-8273874
+     */
+    @Override
+    public Principal authenticate(String username) {
+        ClassLoader ocl = null;
+        try {
+            if (!isUseContextClassLoader()) {
+                ocl = Thread.currentThread().getContextClassLoader();
+                Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader());
+            }
+            return super.authenticate(username);
+        } finally {
+            if (!isUseContextClassLoader()) {
+                Thread.currentThread().setContextClassLoader(ocl);
             }
+        }
+    }
 
-            // Search for additional roles
-            List<String> roles = getRoles(connection, user);
-            if (containerLog.isDebugEnabled()) {
-                containerLog.debug("Found roles: " + roles.toString());
+
+    /*
+     * https://bz.apache.org/bugzilla/show_bug.cgi?id=65553
+     * This method can be removed and the class loader switch moved back to
+     * open() once it is known that Tomcat must be running on a JVM that
+     * includes a fix for
+     * https://bugs.openjdk.java.net/browse/JDK-8273874
+     */
+    @Override
+    public Principal authenticate(String username, String clientDigest, String nonce, String nc,
+            String cnonce, String qop, String realm, String md5a2) {
+        ClassLoader ocl = null;
+        try {
+            if (!isUseContextClassLoader()) {
+                ocl = Thread.currentThread().getContextClassLoader();
+                Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader());
+            }
+            return super.authenticate(username, clientDigest, nonce, nc, cnonce, qop, realm, md5a2);
+        } finally {
+            if (!isUseContextClassLoader()) {
+                Thread.currentThread().setContextClassLoader(ocl);
             }
+        }
+    }
+
 
-            // Create and return a suitable Principal for this user
-            return new GenericPrincipal(username, credentials, roles);
+    /*
+     * https://bz.apache.org/bugzilla/show_bug.cgi?id=65553
+     * This method can be removed and the class loader switch moved back to
+     * open() once it is known that Tomcat must be running on a JVM that
+     * includes a fix for
+     * https://bugs.openjdk.java.net/browse/JDK-8273874
+     */
+    @Override
+    public Principal authenticate(X509Certificate[] certs) {
+        ClassLoader ocl = null;
+        try {
+            if (!isUseContextClassLoader()) {
+                ocl = Thread.currentThread().getContextClassLoader();
+                Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader());
+            }
+            return super.authenticate(certs);
+        } finally {
+            if (!isUseContextClassLoader()) {
+                Thread.currentThread().setContextClassLoader(ocl);
+            }
+        }
+    }
+
+
+    /*
+     * https://bz.apache.org/bugzilla/show_bug.cgi?id=65553
+     * This method can be removed and the class loader switch moved back to
+     * open() once it is known that Tomcat must be running on a JVM that
+     * includes a fix for
+     * https://bugs.openjdk.java.net/browse/JDK-8273874
+     */
+    @Override
+    public Principal authenticate(GSSContext gssContext, boolean storeCred) {
+        ClassLoader ocl = null;
+        try {
+            if (!isUseContextClassLoader()) {
+                ocl = Thread.currentThread().getContextClassLoader();
+                Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader());
+            }
+            return super.authenticate(gssContext, storeCred);
+        } finally {
+            if (!isUseContextClassLoader()) {
+                Thread.currentThread().setContextClassLoader(ocl);
+            }
+        }
+    }
+
+
+    /*
+     * https://bz.apache.org/bugzilla/show_bug.cgi?id=65553
+     * This method can be removed and the class loader switch moved back to
+     * open() once it is known that Tomcat must be running on a JVM that
+     * includes a fix for
+     * https://bugs.openjdk.java.net/browse/JDK-8273874
+     */
+    @Override
+    public Principal authenticate(GSSName gssName, GSSCredential gssCredential) {
+        ClassLoader ocl = null;
+        try {
+            if (!isUseContextClassLoader()) {
+                ocl = Thread.currentThread().getContextClassLoader();
+                Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader());
+            }
+            return super.authenticate(gssName, gssCredential);
+        } finally {
+            if (!isUseContextClassLoader()) {
+                Thread.currentThread().setContextClassLoader(ocl);
+            }
         }
     }
 
@@ -2458,12 +2610,7 @@ public class JNDIRealm extends RealmBase {
      * @throws NamingException if a directory server error occurs
      */
     protected void open(JNDIConnection connection) throws NamingException {
-        ClassLoader ocl = null;
         try {
-            if (!isUseContextClassLoader()) {
-                ocl = Thread.currentThread().getContextClassLoader();
-                Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader());
-            }
             // Ensure that we have a directory context available
             connection.context = createDirContext(getDirectoryContextEnvironment());
         } catch (Exception e) {
@@ -2480,9 +2627,6 @@ public class JNDIRealm extends RealmBase {
             // reset it in case the connection times out.
             // the primary may come back.
             connectionAttempt = 0;
-            if (!isUseContextClassLoader()) {
-                Thread.currentThread().setContextClassLoader(ocl);
-            }
         }
     }
 
@@ -2668,8 +2812,17 @@ public class JNDIRealm extends RealmBase {
         }
 
         // Check to see if the connection to the directory can be opened
+        ClassLoader ocl = null;
         JNDIConnection connection = null;
         try {
+            // https://bz.apache.org/bugzilla/show_bug.cgi?id=65553
+            // This can move back to open() once it is known that Tomcat must be
+            // running on a JVM that includes a fix for
+            // https://bugs.openjdk.java.net/browse/JDK-8273874
+            if (!isUseContextClassLoader()) {
+                ocl = Thread.currentThread().getContextClassLoader();
+                Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader());
+            }
             connection = get();
         } catch (NamingException e) {
             // A failure here is not fatal as the directory may be unavailable
@@ -2679,6 +2832,9 @@ public class JNDIRealm extends RealmBase {
             containerLog.error(sm.getString("jndiRealm.open"), e);
         } finally {
             release(connection);
+            if (!isUseContextClassLoader()) {
+                Thread.currentThread().setContextClassLoader(ocl);
+            }
         }
 
         super.startInternal();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 9a0cbda..a06329a 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -116,6 +116,11 @@
         <code>DataSourceUserDatabase</code>. (remm)
       </fix>
       <fix>
+        <bug>65553</bug>: Implement a work-around for a
+        <a href="https://bugs.openjdk.java.net/browse/JDK-8273874">JRE bug</a>
+        that can trigger a memory leak when using the JNDI realm. (markt)
+      </fix>
+      <fix>
         <bug>65586</bug>: Fix the bloom filter used to improve performance of
         archive file look ups in the web resources implementation so it works
         correctly for directory lookups whether or not the provided directory

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