Author: markt
Date: Wed Dec 23 12:32:02 2009
New Revision: 893492

URL: http://svn.apache.org/viewvc?rev=893492&view=rev
Log:
Log threads that are started but not stopped by web applications

Modified:
    tomcat/tc6.0.x/trunk/   (props changed)
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/LocalStrings.properties
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc6.0.x/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Dec 23 12:32:02 2009
@@ -1,2 +1,2 @@
 /tomcat:883362
-/tomcat/trunk:601180,606992,612607,630314,640888,652744,653247,666232,673796,673820,677910,683969,683982,684001,684081,684234,684269-684270,685177,687503,687645,689402,690781,691392,691805,692748,693378,694992,695053,695311,696780,696782,698012,698227,698236,698613,699427,699634,701355,709294,709811,709816,710063,710066,710125,710205,711126,711600,712461,712467,713953,714002,718360,719119,719124,719602,719626,719628,720046,720069,721040,721286,721708,721886,723404,723738,726052,727303,728032,728768,728947,729057,729567,729569,729571,729681,729809,729815,729934,730250,730590,731651,732859,732863,734734,740675,740684,742677,742697,742714,744160,744238,746321,746384,746425,747834,747863,748344,750258,750291,750921,751286-751287,751289,751295,753039,757335,757774,758249,758365,758596,758616,758664,759074,761601,762868,762929,762936-762937,763166,763183,763193,763228,763262,763298,763302,763325,763599,763611,763654,763681,763706,764985,764997,765662,768335,769979,770716,770809,77
 
0876,772872,776921,776924,776935,776945,777464,777466,777576,777625,778379,778523-778524,781528,781779,782145,782791,783316,783696,783724,783756,783762,783766,783863,783934,784453,784602,784614,785381,785688,785768,785859,786468,786487,786490,786496,786667,787627,787770,787985,789389,790405,791041,791184,791194,791224,791243,791326,791328,791789,792740,793372,793757,793882,793981,794082,794673,794822,795043,795152,795210,795457,795466,797168,797425,797596,797607,802727,802940,804462,804544,804734,805153,809131,809603,810916,810977,812125,812137,812432,813001,813013,813866,814180,814708,814876,815972,816252,817442,817822,819339,819361,820110,820132,820874,820954,821397,828196,828201,828210,828225,828759,830378-830379,830999,831106,831774,831785,831828,831850,831860,832214,832218,833121,833545,834047,835036,835336,836405,881396,881412,883130,883134,883146,883165,883177,883362,883565,884341,885038,885231,885991,886019,888072,889363,889606,889716,890139,890265,890349-890350,8904
 
17,891185-891187,891583,892198,892341,892415,892464,892555,892814,892817,892887,893321
+/tomcat/trunk:601180,606992,612607,630314,640888,652744,653247,666232,673796,673820,677910,683969,683982,684001,684081,684234,684269-684270,685177,687503,687645,689402,690781,691392,691805,692748,693378,694992,695053,695311,696780,696782,698012,698227,698236,698613,699427,699634,701355,709294,709811,709816,710063,710066,710125,710205,711126,711600,712461,712467,713953,714002,718360,719119,719124,719602,719626,719628,720046,720069,721040,721286,721708,721886,723404,723738,726052,727303,728032,728768,728947,729057,729567,729569,729571,729681,729809,729815,729934,730250,730590,731651,732859,732863,734734,740675,740684,742677,742697,742714,744160,744238,746321,746384,746425,747834,747863,748344,750258,750291,750921,751286-751287,751289,751295,753039,757335,757774,758249,758365,758596,758616,758664,759074,761601,762868,762929,762936-762937,763166,763183,763193,763228,763262,763298,763302,763325,763599,763611,763654,763681,763706,764985,764997,765662,768335,769979,770716,770809,77
 
0876,772872,776921,776924,776935,776945,777464,777466,777576,777625,778379,778523-778524,781528,781779,782145,782791,783316,783696,783724,783756,783762,783766,783863,783934,784453,784602,784614,785381,785688,785768,785859,786468,786487,786490,786496,786667,787627,787770,787985,789389,790405,791041,791184,791194,791224,791243,791326,791328,791789,792740,793372,793757,793882,793981,794082,794673,794822,795043,795152,795210,795457,795466,797168,797425,797596,797607,802727,802940,804462,804544,804734,805153,809131,809603,810916,810977,812125,812137,812432,813001,813013,813866,814180,814708,814876,815972,816252,817442,817822,819339,819361,820110,820132,820874,820954,821397,828196,828201,828210,828225,828759,830378-830379,830999,831106,831774,831785,831828,831850,831860,832214,832218,833121,833545,834047,835036,835336,836405,881396,881412,883130,883134,883146,883165,883177,883362,883565,884341,885038,885231,885241,885260,885991,886019,888072,889363,889606,889716,890139,890265,8903
 
49-890350,890417,891185-891187,891583,892198,892341,892415,892464,892555,892814,892817,892887,893321

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=893492&r1=893491&r2=893492&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Wed Dec 23 12:32:02 2009
@@ -263,12 +263,6 @@
   +1: kkolinko, markt, rjung, jim
   -1:
 
-* Log threads that are started but not stopped by web applications
-  http://svn.apache.org/viewvc?view=revision&revision=885241
-  http://svn.apache.org/viewvc?view=revision&revision=885260
-  +1: markt, jim, rjung
-  -1: 
-
 * More memory leak protection: ThreadLocals, RMI references, optionally 
stopping
   threads.
   http://svn.apache.org/viewvc?view=revision&revision=885901

Modified: 
tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/LocalStrings.properties
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/LocalStrings.properties?rev=893492&r1=893491&r2=893492&view=diff
==============================================================================
--- 
tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/LocalStrings.properties 
(original)
+++ 
tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/LocalStrings.properties 
Wed Dec 23 12:32:02 2009
@@ -29,10 +29,13 @@
 standardLoader.starting=Starting this Loader
 standardLoader.stopping=Stopping this Loader
 webappClassLoader.illegalJarPath=Illegal JAR entry detected with name {0}
+webappClassLoader.jdbcRemoveFailed=JDBC driver de-registration failed
+webappClassLoader.jdbcRemoveStreamError=Exception closing input stream during 
JDBC driver de-registration
 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.unclearedReferenceJbdc=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.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.validationErrorJarPath=Unable to validate JAR entry with 
name {0}
+webappClassLoader.warnThread=A web application appears to have started a 
thread named [{0}] but has failed to stop it. This is very likely to create a 
memory leak.
 webappClassLoader.wrongVersion=(unable to load class {0})
 webappLoader.addRepository=Adding repository {0}
 webappLoader.deploy=Deploying class repositories to work directory {0}

Modified: 
tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java?rev=893492&r1=893491&r2=893492&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java 
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java 
Wed Dec 23 12:32:02 2009
@@ -37,6 +37,7 @@
 import java.security.Policy;
 import java.security.PrivilegedAction;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -110,6 +111,13 @@
     protected static org.apache.juli.logging.Log log=
         org.apache.juli.logging.LogFactory.getLog( WebappClassLoader.class );
 
+    /**
+     * List of ThreadGroup names to ignore when scanning for web application
+     * started threads that need to be shut down.
+     */
+    private static final List<String> JVM_THREAD_GROUP_NAMES =
+        new ArrayList<String>();
+
     public static final boolean ENABLE_CLEAR_REFERENCES = 
         
Boolean.valueOf(System.getProperty("org.apache.catalina.loader.WebappClassLoader.ENABLE_CLEAR_REFERENCES",
 "true")).booleanValue();
 
@@ -133,6 +141,11 @@
 
     }
 
+    static {
+        JVM_THREAD_GROUP_NAMES.add("system");
+        JVM_THREAD_GROUP_NAMES.add("RMI Runtime");
+    }
+
     protected class PrivilegedFindResourceByName
         implements PrivilegedAction<ResourceEntry> {
 
@@ -1575,7 +1588,7 @@
         clearReferences();
 
         started = false;
-
+        
         int length = files.length;
         for (int i = 0; i < length; i++) {
             files[i] = null;
@@ -1653,24 +1666,49 @@
      */
     protected void clearReferences() {
 
-        /*
-         * Deregister any JDBC drivers registered by the webapp that the webapp
-         * forgot. This is made unnecessary complex because a) DriverManager
-         * checks the class loader of the calling class (it would be much 
easier
-         * if it checked the context class loader) b) using reflection would
-         * create a dependency on the DriverManager implementation which can,
-         * and has, changed.
-         * 
-         * We can't just create an instance of JdbcLeakPrevention as it will be
-         * loaded by the common class loader (since it's .class file is in the
-         * $CATALINA_HOME/lib directory). This would fail DriverManager's check
-         * on the class loader of the calling class. So, we load the bytes via
-         * our parent class loader but define the class with this class loader
-         * so the JdbcLeakPrevention looks like a webapp class to the
-         * DriverManager.
-         * 
-         * If only apps cleaned up after themselves...
-         */
+        // De-register any remaining JDBC drivers
+        clearReferencesJdbc();
+
+        // Stop any threads the web application started
+        clearReferencesThreads();
+        
+        // Null out any static or final fields from loaded classes,
+        // as a workaround for apparent garbage collection bugs
+        if (ENABLE_CLEAR_REFERENCES) {
+            clearReferencesStaticFinal();
+        }
+        
+         // Clear the IntrospectionUtils cache.
+        IntrospectionUtils.clear();
+        
+        // Clear the classloader reference in common-logging
+        org.apache.juli.logging.LogFactory.release(this);
+        
+        // Clear the classloader reference in the VM's bean introspector
+        java.beans.Introspector.flushCaches();
+
+    }
+
+
+    /**
+     * Deregister any JDBC drivers registered by the webapp that the webapp
+     * forgot. This is made unnecessary complex because a) DriverManager
+     * checks the class loader of the calling class (it would be much easier
+     * if it checked the context class loader) b) using reflection would
+     * create a dependency on the DriverManager implementation which can,
+     * and has, changed.
+     * 
+     * We can't just create an instance of JdbcLeakPrevention as it will be
+     * loaded by the common class loader (since it's .class file is in the
+     * $CATALINA_HOME/lib directory). This would fail DriverManager's check
+     * on the class loader of the calling class. So, we load the bytes via
+     * our parent class loader but define the class with this class loader
+     * so the JdbcLeakPrevention looks like a webapp class to the
+     * DriverManager.
+     * 
+     * If only apps cleaned up after themselves...
+     */
+    private final void clearReferencesJdbc() {
         InputStream is = getResourceAsStream(
                 "org/apache/catalina/loader/JdbcLeakPrevention.class");
         // We know roughly how big the class will be (~ 1K) so allow 2k as a
@@ -1697,8 +1735,7 @@
             List<String> driverNames = (List<String>) obj.getClass().getMethod(
                     "clearJdbcDriverRegistrations").invoke(obj);
             for (String name : driverNames) {
-                log.error(sm.getString(
-                        "webappClassLoader.unclearedReferenceJbdc", name));
+                log.error(sm.getString("webappClassLoader.clearJbdc", name));
             }
         } catch (Exception e) {
             // So many things to go wrong above...
@@ -1710,96 +1747,88 @@
                 } catch (IOException ioe) {
                     log.warn(sm.getString(
                             "webappClassLoader.jdbcRemoveStreamError"), ioe);
+                }
             }
         }
-        }
+    }
+
+
+    private final void clearReferencesStaticFinal() {
         
-        // Null out any static or final fields from loaded classes,
-        // as a workaround for apparent garbage collection bugs
-        if (ENABLE_CLEAR_REFERENCES) {
-            java.util.Collection<ResourceEntry> values =
-                ((HashMap<String,ResourceEntry>) 
resourceEntries.clone()).values();
-            Iterator<ResourceEntry> loadedClasses = values.iterator();
-            //
-            // walk through all loaded class to trigger initialization for
-            //    any uninitialized classes, otherwise initialization of
-            //    one class may call a previously cleared class.
-            while (loadedClasses.hasNext()) {
-                ResourceEntry entry = loadedClasses.next();
-                if (entry.loadedClass != null) {
-                    Class<?> clazz = entry.loadedClass;
-                    try {
-                        Field[] fields = clazz.getDeclaredFields();
-                        for (int i = 0; i < fields.length; i++) {
-                            if(Modifier.isStatic(fields[i].getModifiers())) {
-                                fields[i].get(null);
-                                break;
-                            }
+        @SuppressWarnings("unchecked")
+        Collection<ResourceEntry> values =
+            ((HashMap<String,ResourceEntry>) resourceEntries.clone()).values();
+        Iterator<ResourceEntry> loadedClasses = values.iterator();
+        //
+        // walk through all loaded class to trigger initialization for
+        //    any uninitialized classes, otherwise initialization of
+        //    one class may call a previously cleared class.
+        while(loadedClasses.hasNext()) {
+            ResourceEntry entry = loadedClasses.next();
+            if (entry.loadedClass != null) {
+                Class<?> clazz = entry.loadedClass;
+                try {
+                    Field[] fields = clazz.getDeclaredFields();
+                    for (int i = 0; i < fields.length; i++) {
+                        if(Modifier.isStatic(fields[i].getModifiers())) {
+                            fields[i].get(null);
+                            break;
                         }
-                    } catch(Throwable t) {
-                        // Ignore
                     }
+                } catch(Throwable t) {
+                    // Ignore
                 }
             }
-            loadedClasses = values.iterator();
-            while (loadedClasses.hasNext()) {
-                ResourceEntry entry = loadedClasses.next();
-                if (entry.loadedClass != null) {
-                    Class<?> clazz = entry.loadedClass;
-                    try {
-                        Field[] fields = clazz.getDeclaredFields();
-                        for (int i = 0; i < fields.length; i++) {
-                            Field field = fields[i];
-                            int mods = field.getModifiers();
-                            if (field.getType().isPrimitive() 
-                                    || (field.getName().indexOf("$") != -1)) {
-                                continue;
-                            }
-                            if (Modifier.isStatic(mods)) {
-                                try {
-                                    field.setAccessible(true);
-                                    if (Modifier.isFinal(mods)) {
-                                        if 
(!((field.getType().getName().startsWith("java."))
-                                                || 
(field.getType().getName().startsWith("javax.")))) {
-                                            nullInstance(field.get(null));
-                                        }
-                                    } else {
-                                        field.set(null, null);
-                                        if (log.isDebugEnabled()) {
-                                            log.debug("Set field " + 
field.getName() 
-                                                    + " to null in class " + 
clazz.getName());
-                                        }
+        }
+        loadedClasses = values.iterator();
+        while (loadedClasses.hasNext()) {
+            ResourceEntry entry = loadedClasses.next();
+            if (entry.loadedClass != null) {
+                Class<?> clazz = entry.loadedClass;
+                try {
+                    Field[] fields = clazz.getDeclaredFields();
+                    for (int i = 0; i < fields.length; i++) {
+                        Field field = fields[i];
+                        int mods = field.getModifiers();
+                        if (field.getType().isPrimitive() 
+                                || (field.getName().indexOf("$") != -1)) {
+                            continue;
+                        }
+                        if (Modifier.isStatic(mods)) {
+                            try {
+                                field.setAccessible(true);
+                                if (Modifier.isFinal(mods)) {
+                                    if 
(!((field.getType().getName().startsWith("java."))
+                                            || 
(field.getType().getName().startsWith("javax.")))) {
+                                        nullInstance(field.get(null));
                                     }
-                                } catch (Throwable t) {
+                                } else {
+                                    field.set(null, null);
                                     if (log.isDebugEnabled()) {
-                                        log.debug("Could not set field " + 
field.getName() 
-                                                + " to null in class " + 
clazz.getName(), t);
+                                        log.debug("Set field " + 
field.getName() 
+                                                + " to null in class " + 
clazz.getName());
                                     }
                                 }
+                            } catch (Throwable t) {
+                                if (log.isDebugEnabled()) {
+                                    log.debug("Could not set field " + 
field.getName() 
+                                            + " to null in class " + 
clazz.getName(), t);
+                                }
                             }
                         }
-                    } catch (Throwable t) {
-                        if (log.isDebugEnabled()) {
-                            log.debug("Could not clean fields for class " + 
clazz.getName(), t);
-                        }
+                    }
+                } catch (Throwable t) {
+                    if (log.isDebugEnabled()) {
+                        log.debug("Could not clean fields for class " + 
clazz.getName(), t);
                     }
                 }
             }
         }
         
-         // Clear the IntrospectionUtils cache.
-        IntrospectionUtils.clear();
-        
-        // Clear the classloader reference in common-logging
-        org.apache.juli.logging.LogFactory.release(this);
-        
-        // Clear the classloader reference in the VM's bean introspector
-        java.beans.Introspector.flushCaches();
-
     }
 
 
-    protected void nullInstance(Object instance) {
+    private void nullInstance(Object instance) {
         if (instance == null) {
             return;
         }
@@ -1816,25 +1845,24 @@
                 if (Modifier.isStatic(mods) && Modifier.isFinal(mods)) {
                     // Doing something recursively is too risky
                     continue;
-                } else {
-                    Object value = field.get(instance);
-                    if (null != value) {
-                        Class valueClass = value.getClass();
-                        if (!loadedByThisOrChild(valueClass)) {
-                            if (log.isDebugEnabled()) {
-                                log.debug("Not setting field " + 
field.getName() +
-                                        " to null in object of class " + 
-                                        instance.getClass().getName() +
-                                        " because the referenced object was of 
type " +
-                                        valueClass.getName() + 
-                                        " which was not loaded by this 
WebappClassLoader.");
-                            }
-                        } else {
-                            field.set(instance, null);
-                            if (log.isDebugEnabled()) {
-                                log.debug("Set field " + field.getName() 
-                                        + " to null in class " + 
instance.getClass().getName());
-                            }
+                }
+                Object value = field.get(instance);
+                if (null != value) {
+                    Class<? extends Object> valueClass = value.getClass();
+                    if (!loadedByThisOrChild(valueClass)) {
+                        if (log.isDebugEnabled()) {
+                            log.debug("Not setting field " + field.getName() +
+                                    " to null in object of class " + 
+                                    instance.getClass().getName() +
+                                    " because the referenced object was of 
type " +
+                                    valueClass.getName() + 
+                                    " which was not loaded by this 
WebappClassLoader.");
+                        }
+                    } else {
+                        field.set(instance, null);
+                        if (log.isDebugEnabled()) {
+                            log.debug("Set field " + field.getName() 
+                                    + " to null in class " + 
instance.getClass().getName());
                         }
                     }
                 }
@@ -1849,6 +1877,56 @@
     }
 
 
+    private void clearReferencesThreads() {
+        // Get the current thread group 
+        ThreadGroup tg = Thread.currentThread( ).getThreadGroup( );
+        // Find the root thread group
+        while (tg.getParent() != null) {
+            tg = tg.getParent();
+        }
+        
+        int threadCountGuess = tg.activeCount() + 50;
+        Thread[] threads = new Thread[threadCountGuess];
+        int threadCountActual = tg.enumerate(threads);
+        // Make sure we don't miss any threads
+        while (threadCountActual == threadCountGuess) {
+            threadCountGuess *=2;
+            threads = new Thread[threadCountGuess];
+            // Note tg.enumerate(Thread[]) silently ignores any threads that
+            // can't fit into the array 
+            threadCountActual = tg.enumerate(threads);
+        }
+        
+        // Iterate over the set of threads
+        for (Thread thread : threads) {
+            if (thread != null) {
+                if (thread.getContextClassLoader() == this) {
+                    // Don't warn about this thread
+                    if (thread == Thread.currentThread()) {
+                        continue;
+                    }
+                    
+                    // Skip threads that have already died
+                    if (!thread.isAlive()) {
+                        continue;
+                    }
+
+                    // Don't warn about JVM controlled threads
+                    if (thread.getThreadGroup() != null &&
+                            JVM_THREAD_GROUP_NAMES.contains(
+                                    thread.getThreadGroup().getName())) {
+                        continue;
+                    }
+                   
+                    log.error(sm.getString("webappClassLoader.warnThread",
+                            thread.getName()));
+
+                }
+            }
+        }
+    }
+
+
     /**
      * Determine whether a class was loaded by this class loader or one of
      * its child class loaders.

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=893492&r1=893491&r2=893492&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Wed Dec 23 12:32:02 2009
@@ -278,6 +278,11 @@
         Ensure JDBC driver de-registration works with a security manager.
         (markt)
       </fix>
+      <add>
+        Log errors if a web application starts a thread but fails to stop the
+        thread when the web application stops or is reloaded. Failure to stop a
+        thread is very likely to result in a memory leak. (markt)
+      </add>
       <fix>
         <bug>48214</bug>: Ensure JDBC driver de-registration is not too 
zealous.
         (markt)



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

Reply via email to