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

Reply via email to