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 2010/02/11 19:09:38 UTC

svn commit: r909097 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoader.java

Author: markt
Date: Thu Feb 11 18:09:08 2010
New Revision: 909097

URL: http://svn.apache.org/viewvc?rev=909097&view=rev
Log:
Improved memory leak prevention for resource ResourceBundle

Modified:
    tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties
    tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java

Modified: tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties?rev=909097&r1=909096&r2=909097&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties Thu Feb 11 18:09:08 2010
@@ -35,6 +35,8 @@
 webappClassLoader.stopped=Illegal access: this web application instance has been stopped already.  Could not load {0}.  The eventual following stack trace is caused by an error thrown for debugging purposes as well as to attempt to terminate the thread which caused the illegal access, and has no functional impact.
 webappClassLoader.readError=Resource read error: Could not load {0}.
 webappClassLoader.clearJbdc=A web application registered the JBDC driver [{0}] but failed to unregister it when the web application was stopped. To prevent a memory leak, the JDBC Driver has been forcibly unregistered.
+webappClassLoader.clearReferencesResourceBundlesCount=Removed [{0}] ResourceBundle references from the cache
+webappClassLoader.clearReferencesResourceBundlesFail=Failed to clear ResourceBundle references
 webappClassLoader.clearRmiInfo=Failed to find class sun.rmi.transport.Target to clear context class loader. This is expected on non-Sun JVMs.
 webappClassLoader.clearRmiFail=Failed to clear context class loader referenced from sun.rmi.transport.Target 
 webappClassLoader.clearThreadLocalDebug=A web application created a ThreadLocal with key of type [{0}] (value [{1}]). The ThreadLocal has been correctly set to null and the key will be removed by GC. However, to simplify the process of tracing memory leaks, the key has been forcibly removed.

Modified: tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java?rev=909097&r1=909096&r2=909097&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java Thu Feb 11 18:09:08 2010
@@ -25,6 +25,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.lang.ref.Reference;
+import java.lang.ref.WeakReference;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -47,6 +48,8 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.ResourceBundle;
+import java.util.Set;
 import java.util.Vector;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.jar.Attributes;
@@ -66,6 +69,7 @@
 import org.apache.catalina.LifecycleException;
 import org.apache.catalina.LifecycleListener;
 import org.apache.tomcat.util.res.StringManager;
+import org.apache.jasper.servlet.JasperLoader;
 import org.apache.naming.JndiPermission;
 import org.apache.naming.resources.Resource;
 import org.apache.naming.resources.ResourceAttributes;
@@ -1773,6 +1777,9 @@
             org.apache.juli.logging.LogFactory.release(this);
         }
         
+        // Clear the resource bundle cache
+        clearReferencesResourceBundles();
+
         // Clear the classloader reference in the VM's bean introspector
         java.beans.Introspector.flushCaches();
 
@@ -2313,6 +2320,83 @@
     
     
     /**
+     * Clear the {@link ResourceBundle} cache of any bundles loaded by this
+     * class loader or any class loader where this loader is a parent class
+     * loader. Whilst {@link ResourceBundle#clearCache()} could be used there
+     * are complications around the {@link JasperLoader} that mean a reflection
+     * based approach is more likely to be complete.
+     * 
+     * The ResourceBundle is using WeakReferences so it shouldn't be pinning the
+     * class loader in memory. However, it is. Therefore clear ou the
+     * references.
+     */
+    private void clearReferencesResourceBundles() {
+        // Get a reference to the cache
+        try {
+            Field cacheListField =
+                ResourceBundle.class.getDeclaredField("cacheList");
+            cacheListField.setAccessible(true);
+
+            // Java 6 uses ConcurrentMap
+            // Java 5 uses SoftCache extends Abstract Map
+            // So use Map and it *should* work with both
+            Map<?,?> cacheList = (Map<?,?>) cacheListField.get(null);
+            
+            // Get the keys (loader references are in the key)
+            Set<?> keys = cacheList.keySet();
+            
+            Field loaderRefField = null;
+            
+            // Iterate over the keys looking at the loader instances
+            Iterator<?> keysIter = keys.iterator();
+            
+            int countRemoved = 0;
+            
+            while (keysIter.hasNext()) {
+                Object key = keysIter.next();
+                
+                if (loaderRefField == null) {
+                    loaderRefField =
+                        key.getClass().getDeclaredField("loaderRef");
+                    loaderRefField.setAccessible(true);
+                }
+                WeakReference<?> loaderRef =
+                    (WeakReference<?>) loaderRefField.get(key);
+                
+                ClassLoader loader = (ClassLoader) loaderRef.get();
+                
+                while (loader != null && loader != this) {
+                    loader = loader.getParent();
+                }
+                
+                if (loader != null) {
+                    keysIter.remove();
+                    countRemoved++;
+                }
+            }
+            
+            if (countRemoved > 0 && log.isDebugEnabled()) {
+                log.debug(sm.getString(
+                        "webappClassLoader.clearReferencesResourceBundlesCount",
+                        Integer.valueOf(countRemoved)));
+            }
+        } catch (SecurityException e) {
+            log.error(sm.getString(
+                    "webappClassLoader.clearReferencesResourceBundlesFail"), e);
+        } catch (NoSuchFieldException e) {
+            log.error(sm.getString(
+                    "webappClassLoader.clearReferencesResourceBundlesFail"), e);
+        } catch (IllegalArgumentException e) {
+            log.error(sm.getString(
+                    "webappClassLoader.clearReferencesResourceBundlesFail"), e);
+        } catch (IllegalAccessException e) {
+            log.error(sm.getString(
+                    "webappClassLoader.clearReferencesResourceBundlesFail"), e);
+        }
+    }
+
+
+    /**
      * Determine whether a class was loaded by this class loader or one of
      * its child class loaders.
      */



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


Re: svn commit: r909097 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoader.java

Posted by Mark Thomas <ma...@apache.org>.
On 12/02/2010 12:32, Mark Thomas wrote:
> On 12/02/2010 02:52, Bill Barker wrote:
>>
>>
>> <ma...@apache.org> wrote in message
>> news:20100211180956.BFD5F23888E2@eris.apache.org...
>>> Author: markt
>>> Date: Thu Feb 11 18:09:08 2010
>>> New Revision: 909097
>>>
>>> URL: http://svn.apache.org/viewvc?rev=909097&view=rev
>>> Log:
>>> Improved memory leak prevention for resource ResourceBundle
> 
> <snip/>
> 
>>> +    private void clearReferencesResourceBundles() {
>>> +        // Get a reference to the cache
>>> +        try {
>>> +            Field cacheListField =
>>> +                ResourceBundle.class.getDeclaredField("cacheList");
>>> +            cacheListField.setAccessible(true);
>>> +
>>
>> I don't see anywhere in the documentation that says that there is a
>> cacheList field.  Have you tried this with a non-Sun JVM?
> 
> It works with the 1.6.0 SR7 64-bit JDK on linux from IBM. I haven't
> tested any further.

I take that back. It looks like I get an error. Not sure why my first
test missed it. I'll see what I can do to handle that more gracefully.

Mark



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


Re: svn commit: r909097 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoader.java

Posted by Mark Thomas <ma...@apache.org>.
On 12/02/2010 02:52, Bill Barker wrote:
> 
> 
> <ma...@apache.org> wrote in message
> news:20100211180956.BFD5F23888E2@eris.apache.org...
>> Author: markt
>> Date: Thu Feb 11 18:09:08 2010
>> New Revision: 909097
>>
>> URL: http://svn.apache.org/viewvc?rev=909097&view=rev
>> Log:
>> Improved memory leak prevention for resource ResourceBundle

<snip/>

>> +    private void clearReferencesResourceBundles() {
>> +        // Get a reference to the cache
>> +        try {
>> +            Field cacheListField =
>> +                ResourceBundle.class.getDeclaredField("cacheList");
>> +            cacheListField.setAccessible(true);
>> +
> 
> I don't see anywhere in the documentation that says that there is a
> cacheList field.  Have you tried this with a non-Sun JVM?

It works with the 1.6.0 SR7 64-bit JDK on linux from IBM. I haven't
tested any further.

Granted it is undocumented, but most of the memory leak protection
involves using reflection to dig at internals that aren't normally
exposed there is always going to be a risk. This should be fairly same
as we don't need to dig too far to get to what we need.

Mark



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


Re: svn commit: r909097 - in /tomcat/trunk/java/org/apache/catalina/loader: LocalStrings.properties WebappClassLoader.java

Posted by Bill Barker <bi...@verizon.net>.

<ma...@apache.org> wrote in message 
news:20100211180956.BFD5F23888E2@eris.apache.org...
> Author: markt
> Date: Thu Feb 11 18:09:08 2010
> New Revision: 909097
>
> URL: http://svn.apache.org/viewvc?rev=909097&view=rev
> Log:
> Improved memory leak prevention for resource ResourceBundle
>
> Modified:
>    tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties
>    tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
>
> Modified: 
> tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties
> URL: 
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties?rev=909097&r1=909096&r2=909097&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties 
> (original)
> +++ tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties 
> Thu Feb 11 18:09:08 2010
> @@ -35,6 +35,8 @@
> webappClassLoader.stopped=Illegal access: this web application instance 
> has been stopped already.  Could not load {0}.  The eventual following 
> stack trace is caused by an error thrown for debugging purposes as well as 
> to attempt to terminate the thread which caused the illegal access, and 
> has no functional impact.
> webappClassLoader.readError=Resource read error: Could not load {0}.
> webappClassLoader.clearJbdc=A web application registered the JBDC driver 
> [{0}] but failed to unregister it when the web application was stopped. To 
> prevent a memory leak, the JDBC Driver has been forcibly unregistered.
> +webappClassLoader.clearReferencesResourceBundlesCount=Removed [{0}] 
> ResourceBundle references from the cache
> +webappClassLoader.clearReferencesResourceBundlesFail=Failed to clear 
> ResourceBundle references
> webappClassLoader.clearRmiInfo=Failed to find class 
> sun.rmi.transport.Target to clear context class loader. This is expected 
> on non-Sun JVMs.
> webappClassLoader.clearRmiFail=Failed to clear context class loader 
> referenced from sun.rmi.transport.Target
> webappClassLoader.clearThreadLocalDebug=A web application created a 
> ThreadLocal with key of type [{0}] (value [{1}]). The ThreadLocal has been 
> correctly set to null and the key will be removed by GC. However, to 
> simplify the process of tracing memory leaks, the key has been forcibly 
> removed.
>
> Modified: 
> tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
> URL: 
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java?rev=909097&r1=909096&r2=909097&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java 
> (original)
> +++ tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java 
> Thu Feb 11 18:09:08 2010
> @@ -25,6 +25,7 @@
> import java.io.IOException;
> import java.io.InputStream;
> import java.lang.ref.Reference;
> +import java.lang.ref.WeakReference;
> import java.lang.reflect.Field;
> import java.lang.reflect.InvocationTargetException;
> import java.lang.reflect.Method;
> @@ -47,6 +48,8 @@
> import java.util.LinkedHashMap;
> import java.util.List;
> import java.util.Map;
> +import java.util.ResourceBundle;
> +import java.util.Set;
> import java.util.Vector;
> import java.util.concurrent.ThreadPoolExecutor;
> import java.util.jar.Attributes;
> @@ -66,6 +69,7 @@
> import org.apache.catalina.LifecycleException;
> import org.apache.catalina.LifecycleListener;
> import org.apache.tomcat.util.res.StringManager;
> +import org.apache.jasper.servlet.JasperLoader;
> import org.apache.naming.JndiPermission;
> import org.apache.naming.resources.Resource;
> import org.apache.naming.resources.ResourceAttributes;
> @@ -1773,6 +1777,9 @@
>             org.apache.juli.logging.LogFactory.release(this);
>         }
>
> +        // Clear the resource bundle cache
> +        clearReferencesResourceBundles();
> +
>         // Clear the classloader reference in the VM's bean introspector
>         java.beans.Introspector.flushCaches();
>
> @@ -2313,6 +2320,83 @@
>
>
>     /**
> +     * Clear the {@link ResourceBundle} cache of any bundles loaded by 
> this
> +     * class loader or any class loader where this loader is a parent 
> class
> +     * loader. Whilst {@link ResourceBundle#clearCache()} could be used 
> there
> +     * are complications around the {@link JasperLoader} that mean a 
> reflection
> +     * based approach is more likely to be complete.
> +     *
> +     * The ResourceBundle is using WeakReferences so it shouldn't be 
> pinning the
> +     * class loader in memory. However, it is. Therefore clear ou the
> +     * references.
> +     */
> +    private void clearReferencesResourceBundles() {
> +        // Get a reference to the cache
> +        try {
> +            Field cacheListField =
> +                ResourceBundle.class.getDeclaredField("cacheList");
> +            cacheListField.setAccessible(true);
> +

I don't see anywhere in the documentation that says that there is a 
cacheList field.  Have you tried this with a non-Sun JVM?

> +            // Java 6 uses ConcurrentMap
> +            // Java 5 uses SoftCache extends Abstract Map
> +            // So use Map and it *should* work with both
> +            Map<?,?> cacheList = (Map<?,?>) cacheListField.get(null);
> +
> +            // Get the keys (loader references are in the key)
> +            Set<?> keys = cacheList.keySet();
> +
> +            Field loaderRefField = null;
> +
> +            // Iterate over the keys looking at the loader instances
> +            Iterator<?> keysIter = keys.iterator();
> +
> +            int countRemoved = 0;
> +
> +            while (keysIter.hasNext()) {
> +                Object key = keysIter.next();
> +
> +                if (loaderRefField == null) {
> +                    loaderRefField =
> +                        key.getClass().getDeclaredField("loaderRef");
> +                    loaderRefField.setAccessible(true);
> +                }
> +                WeakReference<?> loaderRef =
> +                    (WeakReference<?>) loaderRefField.get(key);
> +
> +                ClassLoader loader = (ClassLoader) loaderRef.get();
> +
> +                while (loader != null && loader != this) {
> +                    loader = loader.getParent();
> +                }
> +
> +                if (loader != null) {
> +                    keysIter.remove();
> +                    countRemoved++;
> +                }
> +            }
> +
> +            if (countRemoved > 0 && log.isDebugEnabled()) {
> +                log.debug(sm.getString(
> + 
> "webappClassLoader.clearReferencesResourceBundlesCount",
> +                        Integer.valueOf(countRemoved)));
> +            }
> +        } catch (SecurityException e) {
> +            log.error(sm.getString(
> + 
> "webappClassLoader.clearReferencesResourceBundlesFail"), e);
> +        } catch (NoSuchFieldException e) {
> +            log.error(sm.getString(
> + 
> "webappClassLoader.clearReferencesResourceBundlesFail"), e);
> +        } catch (IllegalArgumentException e) {
> +            log.error(sm.getString(
> + 
> "webappClassLoader.clearReferencesResourceBundlesFail"), e);
> +        } catch (IllegalAccessException e) {
> +            log.error(sm.getString(
> + 
> "webappClassLoader.clearReferencesResourceBundlesFail"), e);
> +        }
> +    }
> +
> +
> +    /**
>      * Determine whether a class was loaded by this class loader or one of
>      * its child class loaders.
>      */ 



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