IMHO, clearing threadlocals is a bad idea, it has even been disabled by default because of concurrency issues. Furthermore, the solution you propose would be limited to some type of classes (Map in your case).
https://issues.apache.org/bugzilla/show_bug.cgi?id=49159 is still pending but the solution proposed would cure this issue in a cleaner way. Sylvain On 29 juil. 2010, at 17:00, Arjen Knibbe wrote: > > > markt-2 wrote: >> >> Please could you create a Bugzilla entry (against Tomact 7) for each >> issue you identified so that this valuable work doesn't get forgotten >> about? >> >> 95% of the work should just be a copy and paste. >> > I created three Bugzilla entries: > > 49667 for the JdbcLeakPreventer issue. > 49668 for the ThreadLocal issue. > 49669 for the PolicyFile issue. > > The problem with the LoaderHandler is a question of patience: after an hour > or two, it will go away. > > I searched for an "upgrading to Tomcat 7" manual in the Tomcat documentation > to check if the new JreMemoryLeakPreventionListener is mentioned, but I > could not find any guidelines for upgrading... > > > markt-2 wrote: >> >> PS How do you fancy trying to fix some of these issues? >> > I didn't try to write a subclass of WebappClassLoader. In stead, I used the > destroy() method of the HttpServlet to clean things up. > > Clearing the PolicyFile reference is trivial: > /** > * Clear references from Policy. > */ > private void clearReferencesPolicy() > { > System.out.println("About to clear Policy references"); > ClassLoader myLoader = this.getClass().getClassLoader(); > > try > { > Class<?> policyClass = > Class.forName("javax.security.auth.Policy"); > Field contextClassLoaderField = > policyClass.getDeclaredField("contextClassLoader"); > contextClassLoaderField.setAccessible(true); > > Object contextClassLoaderObj = > contextClassLoaderField.get(null); > if (contextClassLoaderObj == myLoader) > { > System.out.println("Policy: setting contextClassLoader to > null"); > contextClassLoaderField.set(null, null); > } > } > catch (Throwable e) > { > System.err.println("Clearing Policy references"); > e.printStackTrace(); > } > } > > Clearing of the JDBC Drivers can be done by running the same loop twice: > > /** > * Clear the JDBC drivers for the first time. > * This will load JDBC drivers that were loaded by other applications > * but not by this one, and can be found this class's loader. > */ > public void clearJdbcDrivers() > { > System.out.println("About to deregister JDBC drivers"); > > try > { > // This will list all drivers visible to this class loader > Enumeration<Driver> drivers = DriverManager.getDrivers(); > while (drivers.hasMoreElements()) > { > Driver driver = drivers.nextElement(); > // Only unload the drivers this web app loaded > if (driver.getClass().getClassLoader() != > this.getClass().getClassLoader()) { > continue; > } > System.out.println("Deregistering driver " + driver); > DriverManager.deregisterDriver(driver); > } > } > catch (Exception e) > { > System.err.println("Deregistering JDBC drivers"); > e.printStackTrace(); > } > } > This is done first in the destroy method of the servlet, and again in the > JdbcLeakPreventer. That gives nasty warnings about JDBC Drivers being > forcibly unregistered. I suggest the loop should not run before, but just > after the loop in the JdbcLeakPreventer, without outputting messages. > > Now for the real fancy thing: deeply clearing of the ThreadLocal references. > It starts with a modified copy of the methods in WebappClassLoader for > clearing ThreadLocal references: > /** > * Clear references to the ClassLoader in nested Maps and Collections > * of ThreadLocal objects. > */ > @SuppressWarnings("unchecked") > private void clearReferencesThreadLocalsDeep() > { > System.out.println("About to clear deep references from > ThreadLocals"); > > Thread[] threads = getThreads(); > > try > { > // Make the fields in the Thread class that store ThreadLocals > // accessible > Field threadLocalsField = > Thread.class.getDeclaredField("threadLocals"); > threadLocalsField.setAccessible(true); > Field inheritableThreadLocalsField = > Thread.class.getDeclaredField("inheritableThreadLocals"); > inheritableThreadLocalsField.setAccessible(true); > // Make the underlying array of ThreadLoad.ThreadLocalMap.Entry > objects > // accessible > Class<?> tlmClass = > Class.forName("java.lang.ThreadLocal$ThreadLocalMap"); > Field tableField = tlmClass.getDeclaredField("table"); > tableField.setAccessible(true); > > for (int i = 0; i < threads.length; i++) > { > Object threadLocalMap; > if (threads[i] != null) > { > // Clear the first map > threadLocalMap = threadLocalsField.get(threads[i]); > clearThreadLocalMapDeep("threadLocals", threadLocalMap, > tableField); > // Clear the second map > threadLocalMap = > inheritableThreadLocalsField.get(threads[i]); > clearThreadLocalMapDeep("inheritableThreadLocals", > threadLocalMap, tableField); > } > } > } > catch (Throwable e) > { > System.err.println("Clearing Deep ThreadLocal references"); > e.printStackTrace(); > } > } > > /* > * Clears the given thread local map object. Also pass in the field that > * points to the internal table to save re-calculating it on every > * call to this method. > */ > @SuppressWarnings("unchecked") > private void clearThreadLocalMapDeep(String mapName, Object map, Field > internalTableField) > throws Exception > { > if (map != null) > { > Object[] table = (Object[]) internalTableField.get(map); > if (table != null) { > for (int j =0; j < table.length; j++) { > if (table[j] != null) { > // Check the key > Object key = ((Reference<?>) table[j]).get(); > if (clearRecursive("ThreadLocal " + mapName + " deep > key", key)) > { > System.out.println > ( "Clearing of ThreadLocal table " > + mapName > + " key " > + key > + " left to Tomcat WebappClassLoader" > ); > } > > // Check the value > Field valueField = > table[j].getClass().getDeclaredField("value"); > valueField.setAccessible(true); > Object value = valueField.get(table[j]); > if (clearRecursive("ThreadLocal " + mapName + " deep > value", value)) > { > System.out.println > ( "Clearing of ThreadLocal table " > + mapName > + " value " > + value > + " left to Tomcat WebappClassLoader" > ); > } > } > } > } > } > } > > The new thing in here is the clearRecursive(String, Object) method that is > supposed to clear deep references to the WebappClassLoader. > For my particular case, it would be sufficient to go down the keys of > HashMaps, but I thought I make it more general. Beside Maps, consider > Iterables also, and trace both the keys and the values in Maps. And nested > issues could (at least in theory) contain a loop, causing an infinite > recursion. To prevent that, I keep the inspected objects in a class member: > > /** > * Save inspected objects to prevent double work > * and infinite recursions. > */ > private LinkedList seen = new LinkedList(); > > The methods for deep clearing are: > /** > * Inspect an Object, return whether to remove the reference to it. > * The reference has to be removed in the following situations: > * <ul> > * <li>It refers to the ClassLoader of this class.</li> > * <li>It refers to an Object of a Class that has been loaded > * by the ClassLoader of this class.</li> > * <li>It refers to a Class that has been loaded by the ClassLoader of > this class.</li> > * <li>The object is assignable to Map > * and a call to {...@link #clearMapRecursive(String, Map)} returns > true.</li> > * <li>The object is assignable to Collection > * and a call to {...@link #clearIterableRecursive(String, > Collection)} returns true.</li> > * </ul> > * @param info for logging purposes. > * @param object to be inspected. > * @return whether to remove the reference to this object. > */ > @SuppressWarnings("unchecked") > private boolean clearRecursive(String info, Object object) > { > if (object == null) > { > return false; > } > > ClassLoader myLoader = getClass().getClassLoader(); > if ( object == myLoader > || object.getClass().getClassLoader() == myLoader > || (object instanceof Class && ((Class<?>) > object).getClassLoader() == myLoader) > ) > { > return true; > } > > // Prevent double work and infinite recursions. > // Using containsKey can throw ClassCastException > // So, use the surest way > for (Object item: seen) > { > if (item == seen) > { > return false; > } > } > seen.add(object); > > if (object instanceof Map) > { > return clearMapRecursive(info, (Map<Object, Object>) object); > } > else if (object instanceof Iterable) > { > return clearIterableRecursive(info, (Iterable) object); > } > > return false; > } > > /** > * Clear a collection recursively. > * @param info for logging purposes. > * @param iterable the Collection to be cleared. > * @return false. > */ > private boolean clearIterableRecursive(String info, Iterable iterable) > { > Vector goners = new Vector(); > Iterator iterator = iterable.iterator(); > > // Special treatment for Collections > // in case the Iterator cannot remove > // but the Collection itself can. > if (iterable instanceof Collection) > { > Collection collection = (Collection) iterable; > > for (Object key: collection) > { > if (clearRecursive(info, key)) > { > goners.add(key); > } > } > for (Object goner: goners.toArray()) > { > System.out.println(info + ": removed key referring > ClassLoader from Collection"); > collection.remove(goner); > } > return false; > } > > while (iterator.hasNext()) > { > if (clearRecursive(info, iterator.next())) > { > iterator.remove(); > } > } > > return false; > } > > /** > * Clear a map recursively. > * @param info for logging purposes. > * @param map the Map to be cleared. > * @return false. > */ > private boolean clearMapRecursive(final String info, final Map<Object, > Object> map) > { > Vector goners = new Vector(); > Object value; > for (Object key: map.keySet()) > { > if (clearRecursive(info, key)) > { > goners.add(key); > } > else > { > value = map.get(key); > if (clearRecursive(info, value)) > { > try > { > map.put(key, null); > System.out.println(info + ": removed value referring > ClassLoader from Map"); > } > catch (NullPointerException n) > { > goners.add(key); > } > } > } > } > for (Object goner: goners.toArray()) > { > System.out.println(info + ": removed key referring ClassLoader > from Map"); > map.remove(goner); > } > > return false; > } > > Hope you 'll enjoy > > Arjen > -- > View this message in context: > http://old.nabble.com/More-sources-of-Tomcat-memory-leaks-tp29264214p29297364.html > Sent from the Tomcat - Dev mailing list archive at Nabble.com. > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
