Return-Path: Delivered-To: apmail-tomcat-dev-archive@www.apache.org Received: (qmail 63523 invoked from network); 23 Dec 2009 12:32:29 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3) by minotaur.apache.org with SMTP; 23 Dec 2009 12:32:29 -0000 Received: (qmail 59709 invoked by uid 500); 23 Dec 2009 12:32:28 -0000 Delivered-To: apmail-tomcat-dev-archive@tomcat.apache.org Received: (qmail 59618 invoked by uid 500); 23 Dec 2009 12:32:28 -0000 Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Tomcat Developers List" Delivered-To: mailing list dev@tomcat.apache.org Received: (qmail 59607 invoked by uid 99); 23 Dec 2009 12:32:28 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 23 Dec 2009 12:32:28 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 23 Dec 2009 12:32:24 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 176F823888E7; Wed, 23 Dec 2009 12:32:03 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r893492 - in /tomcat/tc6.0.x/trunk: ./ STATUS.txt java/org/apache/catalina/loader/LocalStrings.properties java/org/apache/catalina/loader/WebappClassLoader.java webapps/docs/changelog.xml Date: Wed, 23 Dec 2009 12:32:02 -0000 To: dev@tomcat.apache.org From: markt@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20091223123203.176F823888E7@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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 JVM_THREAD_GROUP_NAMES = + new ArrayList(); + 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 { @@ -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 driverNames = (List) 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 values = - ((HashMap) resourceEntries.clone()).values(); - Iterator 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 values = + ((HashMap) resourceEntries.clone()).values(); + Iterator 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 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) + + 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) + 48214: Ensure JDBC driver de-registration is not too zealous. (markt) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org For additional commands, e-mail: dev-help@tomcat.apache.org