You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by sl...@apache.org on 2010/12/05 23:54:06 UTC

svn commit: r1042482 - in /tomcat/trunk: ./ conf/ java/org/apache/catalina/core/ java/org/apache/catalina/loader/ java/org/apache/tomcat/util/threads/ res/confinstall/ webapps/ webapps/docs/ webapps/docs/config/

Author: slaurent
Date: Sun Dec  5 22:54:05 2010
New Revision: 1042482

URL: http://svn.apache.org/viewvc?rev=1042482&view=rev
Log:
bug 49159: Improve ThreadLocal memory leak clean-up 
https://issues.apache.org/bugzilla/show_bug.cgi?id=49159
Renewing threads of the pool when a webapp is stopped

Added:
    tomcat/trunk/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java
    tomcat/trunk/java/org/apache/tomcat/util/threads/Constants.java
    tomcat/trunk/java/org/apache/tomcat/util/threads/LocalStrings.properties
    tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThread.java
Modified:
    tomcat/trunk/   (props changed)
    tomcat/trunk/conf/   (props changed)
    tomcat/trunk/conf/server.xml
    tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
    tomcat/trunk/java/org/apache/catalina/core/StandardThreadExecutor.java
    tomcat/trunk/java/org/apache/catalina/core/mbeans-descriptors.xml
    tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties
    tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
    tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java
    tomcat/trunk/java/org/apache/tomcat/util/threads/TaskQueue.java
    tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThreadFactory.java
    tomcat/trunk/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java
    tomcat/trunk/res/confinstall/server_1.xml
    tomcat/trunk/webapps/   (props changed)
    tomcat/trunk/webapps/docs/changelog.xml
    tomcat/trunk/webapps/docs/config/context.xml
    tomcat/trunk/webapps/docs/config/executor.xml

Propchange: tomcat/trunk/
------------------------------------------------------------------------------
--- svn:ignore (original)
+++ svn:ignore Sun Dec  5 22:54:05 2010
@@ -1,3 +1,9 @@
-.*
+.settings
+.classpath
+.project
 output
 build.properties
+.checkstyle
+.pmd
+work
+logs

Propchange: tomcat/trunk/conf/
------------------------------------------------------------------------------
--- svn:ignore (added)
+++ svn:ignore Sun Dec  5 22:54:05 2010
@@ -0,0 +1 @@
+Catalina

Modified: tomcat/trunk/conf/server.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/conf/server.xml?rev=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/conf/server.xml (original)
+++ tomcat/trunk/conf/server.xml Sun Dec  5 22:54:05 2010
@@ -28,6 +28,7 @@
   <!-- Prevent memory leaks due to use of particular java/javax APIs-->
   <Listener className="org.apache.catalina.core.JreMemoryLeakPreventionListener" />
   <Listener className="org.apache.catalina.mbeans.GlobalResourcesLifecycleListener" />
+  <Listener className="org.apache.catalina.core.ThreadLocalLeakPreventionListener" />
 
   <!-- Global JNDI resources
        Documentation at /docs/jndi-resources-howto.html

Modified: tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardContext.java?rev=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardContext.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardContext.java Sun Dec  5 22:54:05 2010
@@ -789,15 +789,14 @@ public class StandardContext extends Con
      * default value of <code>false</code> will be used.
      */
     private boolean clearReferencesStopTimerThreads = false;
-    
+
     /**
-     * Should Tomcat attempt to clear any ThreadLocal objects that are instances
-     * of classes loaded by this class loader. Failure to remove any such
-     * objects will result in a memory leak on web application stop, undeploy or
-     * reload. It is disabled by default since the clearing of the ThreadLocal
-     * objects is not performed in a thread-safe manner.
+     * Should Tomcat renew the threads of the thread pool when the application
+     * is stopped to avoid memory leaks because of uncleaned ThreadLocal
+     * variables. This also requires that the threadRenewalDelay property of the
+     * StandardThreadExecutor of ThreadPoolExecutor be set to a positive value.
      */
-    private boolean clearReferencesThreadLocals = false;
+    private boolean renewThreadsWhenStoppingContext = true;
     
     /**
      * Should the effective web.xml be logged when the context starts?
@@ -2486,34 +2485,19 @@ public class StandardContext extends Con
     }
 
 
-    /**
-     * Return the clearReferencesThreadLocals flag for this Context.
-     */
-    public boolean getClearReferencesThreadLocals() {
-
-        return (this.clearReferencesThreadLocals);
-
+    public boolean getRenewThreadsWhenStoppingContext() {
+        return this.renewThreadsWhenStoppingContext;
     }
 
-
-    /**
-     * Set the clearReferencesThreadLocals feature for this Context.
-     *
-     * @param clearReferencesThreadLocals The new flag value
-     */
-    public void setClearReferencesThreadLocals(
-            boolean clearReferencesThreadLocals) {
-
-        boolean oldClearReferencesThreadLocals =
-            this.clearReferencesThreadLocals;
-        this.clearReferencesThreadLocals = clearReferencesThreadLocals;
-        support.firePropertyChange("clearReferencesStopThreads",
-                                   oldClearReferencesThreadLocals,
-                                   this.clearReferencesThreadLocals);
-
+    public void setRenewThreadsWhenStoppingContext(boolean renewThreadsWhenStoppingContext) {
+        boolean oldRenewThreadsWhenStoppingContext =
+            this.renewThreadsWhenStoppingContext;
+        this.renewThreadsWhenStoppingContext = renewThreadsWhenStoppingContext;
+        support.firePropertyChange("renewThreadsWhenStoppingContext",
+                oldRenewThreadsWhenStoppingContext,
+                this.renewThreadsWhenStoppingContext);
     }
 
-
     // -------------------------------------------------------- Context Methods
 
 

Modified: tomcat/trunk/java/org/apache/catalina/core/StandardThreadExecutor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardThreadExecutor.java?rev=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardThreadExecutor.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardThreadExecutor.java Sun Dec  5 22:54:05 2010
@@ -83,6 +83,13 @@ public class StandardThreadExecutor exte
      */
     protected int maxQueueSize = Integer.MAX_VALUE;
     
+    /**
+     * After a context is stopped, threads in the pool are renewed. To avoid
+     * renewing all threads at the same time, this delay is observed between 2
+     * threads being renewed.
+     */
+    protected long threadRenewalDelay = 1000L;
+    
     private TaskQueue taskqueue = null;
     // ---------------------------------------------- Constructors
     public StandardThreadExecutor() {
@@ -164,6 +171,12 @@ public class StandardThreadExecutor exte
             }
         } else throw new IllegalStateException("StandardThreadPool not started.");
     }
+    
+    public void contextStopping() {
+        if (executor != null) {
+            executor.contextStopping();
+        }
+    }
 
     public int getThreadPriority() {
         return threadPriority;
@@ -249,6 +262,17 @@ public class StandardThreadExecutor exte
         return maxQueueSize;
     }
     
+    public long getThreadRenewalDelay() {
+        return threadRenewalDelay;
+    }
+
+    public void setThreadRenewalDelay(long threadRenewalDelay) {
+        this.threadRenewalDelay = threadRenewalDelay;
+        if (executor != null) {
+            executor.setThreadRenewalDelay(threadRenewalDelay);
+        }
+    }
+
     // Statistics from the thread pool
     @Override
     public int getActiveCount() {

Added: tomcat/trunk/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java?rev=1042482&view=auto
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java (added)
+++ tomcat/trunk/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java Sun Dec  5 22:54:05 2010
@@ -0,0 +1,209 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.catalina.core;
+
+import java.util.concurrent.Executor;
+
+import org.apache.catalina.Container;
+import org.apache.catalina.ContainerEvent;
+import org.apache.catalina.ContainerListener;
+import org.apache.catalina.Context;
+import org.apache.catalina.Engine;
+import org.apache.catalina.Host;
+import org.apache.catalina.Lifecycle;
+import org.apache.catalina.LifecycleEvent;
+import org.apache.catalina.LifecycleListener;
+import org.apache.catalina.Server;
+import org.apache.catalina.Service;
+import org.apache.catalina.connector.Connector;
+import org.apache.coyote.ProtocolHandler;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.threads.ThreadPoolExecutor;
+
+/**
+ * A {@link LifecycleListener} that triggers the renewal of threads in Executor
+ * pools when a {@link Context} is being stopped to avoid thread-local related
+ * memory leaks.<br/>
+ * Note : active threads will be renewed one by one when they come back to the
+ * pool after executing their task, see
+ * {@link org.apache.tomcat.util.threads.ThreadPoolExecutor}.afterExecute().<br/>
+ * 
+ * This listener must be declared in server.xml to be active.
+ * 
+ * @author slaurent
+ * 
+ */
+public class ThreadLocalLeakPreventionListener implements LifecycleListener,
+        ContainerListener {
+    private static final Log log = LogFactory
+            .getLog(ThreadLocalLeakPreventionListener.class);
+
+    /**
+     * Listens for {@link LifecycleEvent} for the start of the {@link Server} to
+     * initialize itself and then for after_stop events of each {@link Context}.
+     */
+    @Override
+    public void lifecycleEvent(LifecycleEvent event) {
+        try {
+            Lifecycle lifecycle = event.getLifecycle();
+            if (Lifecycle.AFTER_START_EVENT.equals(event.getType())
+                    && lifecycle instanceof Server) {
+                // when the server starts, we register ourself as listener for
+                // all context
+                // as well as container event listener so that we know when new
+                // Context are deployed
+                Server server = (Server) lifecycle;
+                registerListenersForServer(server);
+            }
+
+            if (Lifecycle.AFTER_STOP_EVENT.equals(event.getType())
+                    && lifecycle instanceof Context) {
+                stopIdleThreads((Context) lifecycle);
+            }
+        } catch (Exception e) {
+            log.error("Exception processing event " + event, e);
+        }
+    }
+
+    @Override
+    public void containerEvent(ContainerEvent event) {
+        try {
+            String type = event.getType();
+            if (Container.ADD_CHILD_EVENT.equals(type)) {
+                processContainerAddChild(event.getContainer(),
+                        (Container) event.getData());
+            } else if (Container.REMOVE_CHILD_EVENT.equals(type)) {
+                processContainerRemoveChild(event.getContainer(),
+                        (Container) event.getData());
+            }
+        } catch (Exception e) {
+            log.error("Exception processing event " + event, e);
+        }
+
+    }
+
+    private void registerListenersForServer(Server server) {
+        for (Service service : server.findServices()) {
+            Engine engine = (Engine) service.getContainer();
+            engine.addContainerListener(this);
+            registerListenersForEngine(engine);
+        }
+
+    }
+
+    private void registerListenersForEngine(Engine engine) {
+        for (Container hostContainer : engine.findChildren()) {
+            Host host = (Host) hostContainer;
+            host.addContainerListener(this);
+            registerListenersForHost(host);
+        }
+    }
+
+    private void registerListenersForHost(Host host) {
+        for (Container contextContainer : host.findChildren()) {
+            Context context = (Context) contextContainer;
+            registerContextListener(context);
+        }
+    }
+
+    private void registerContextListener(Context context) {
+        context.addLifecycleListener(this);
+    }
+
+    protected void processContainerAddChild(Container parent, Container child) {
+        if (log.isDebugEnabled())
+            log.debug("Process addChild[parent=" + parent + ",child=" + child
+                    + "]");
+
+        try {
+            if (child instanceof Context) {
+                registerContextListener((Context) child);
+            } else if (child instanceof Engine) {
+                registerListenersForEngine((Engine) child);
+            } else if (child instanceof Host) {
+                registerListenersForHost((Host) child);
+            }
+        } catch (Throwable t) {
+            log.error("processContainerAddChild: Throwable", t);
+        }
+
+    }
+
+    protected void processContainerRemoveChild(Container parent, Container child) {
+
+        if (log.isDebugEnabled())
+            log.debug("Process removeChild[parent=" + parent + ",child="
+                    + child + "]");
+
+        try {
+            if (child instanceof Context) {
+                Context context = (Context) child;
+                context.removeLifecycleListener(this);
+            } else if (child instanceof Host) {
+                Host host = (Host) child;
+                host.removeContainerListener(this);
+            } else if (child instanceof Engine) {
+                Engine engine = (Engine) child;
+                engine.removeContainerListener(this);
+            }
+        } catch (Throwable t) {
+            log.error("processContainerRemoveChild: Throwable", t);
+        }
+
+    }
+
+    /**
+     * Updates each ThreadPoolExecutor with the current time, which is the time
+     * when a context is being stopped.
+     * 
+     * @param context
+     *            the context being stopped, used to discover all the Connectors
+     *            of its parent Service.
+     */
+    private void stopIdleThreads(Context context) {
+        if (context instanceof StandardContext
+                && !((StandardContext) context)
+                        .getRenewThreadsWhenStoppingContext()) {
+            log.debug("Not renewing threads when the context is stopping, it is configured not to do it.");
+            return;
+        }
+
+        Engine engine = (Engine) context.getParent().getParent();
+        Service service = engine.getService();
+        Connector[] connectors = service.findConnectors();
+        if (connectors != null) {
+            for (Connector connector : connectors) {
+                ProtocolHandler handler = connector.getProtocolHandler();
+                Executor executor = null;
+                if (handler != null) {
+                    executor = handler.getExecutor();
+                }
+
+                if (executor instanceof ThreadPoolExecutor) {
+                    ThreadPoolExecutor threadPoolExecutor = (ThreadPoolExecutor) executor;
+                    threadPoolExecutor.contextStopping();
+                } else if (executor instanceof StandardThreadExecutor) {
+                    StandardThreadExecutor stdThreadExecutor = (StandardThreadExecutor) executor;
+                    stdThreadExecutor.contextStopping();
+                }
+
+            }
+        }
+    }
+}

Modified: tomcat/trunk/java/org/apache/catalina/core/mbeans-descriptors.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/mbeans-descriptors.xml?rev=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/mbeans-descriptors.xml (original)
+++ tomcat/trunk/java/org/apache/catalina/core/mbeans-descriptors.xml Sun Dec  5 22:54:05 2010
@@ -120,10 +120,6 @@
                description="Should Tomcat attempt to terminate threads that have been started by the web application? Advisable to be used only in a development environment."
                type="boolean"/>
                
-    <attribute name="clearReferencesThreadLocals"
-               description="Should Tomcat attempt to clear any ThreadLocal objects that are instances of classes loaded by this class loader. "
-               type="boolean"/>
-               
     <attribute name="clearReferencesStopTimerThreads"
                description="Should Tomcat attempt to terminate TimerThreads that have been started by the web application? Advisable to be used only in a development environment."
                type="boolean"/>
@@ -281,6 +277,10 @@
                description="The reloadable flag for this web application"
                type="boolean"/>
 
+    <attribute name="renewThreadsWhenStoppingContext"
+               description="Should Tomcat renew the threads of the thread pool when the application is stopped to avoid memory leaks because of uncleaned ThreadLocal variables." 
+               type="boolean"/>
+
     <attribute name="saveConfig"
                description="Should the configuration be written as needed on startup"
                is="true"
@@ -1558,6 +1558,11 @@
     <attribute name="threadPriority"
                description="The thread priority for threads in this thread pool"
                type="int"/>
+
+    <attribute name="threadRenewalDelay"
+               description="After a context is stopped, threads in the pool are renewed. To avoid renewing all threads at the same time, this delay is observed between 2 threads being renewed. Value is in ms, default value is 1000ms. If negative, threads are not renewed."
+               type="long"/>
+               
   </mbean>
 
   <mbean name="StandardWrapper"

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=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties Sun Dec  5 22:54:05 2010
@@ -44,14 +44,12 @@ webappClassLoader.clearReferencesResourc
 webappClassLoader.clearReferencesResourceBundlesFail=Failed to clear ResourceBundle references for web application [{0}]
 webappClassLoader.clearRmiInfo=Failed to find class sun.rmi.transport.Target to clear context class loader for web application [{0}]. This is expected on non-Sun JVMs.
 webappClassLoader.clearRmiFail=Failed to clear context class loader referenced from sun.rmi.transport.Target for web application [{0}]
-webappClassLoader.clearThreadLocalDebug=The web application [{0}] created a ThreadLocal with key of type [{1}] (value [{2}]). The ThreadLocal has been correctly set to null and the key will be removed by GC.
-webappClassLoader.clearThreadLocal=The web application [{0}] created a ThreadLocal with key of type [{1}] (value [{2}]) and a value of type [{3}] (value [{4}]) but failed to remove it when the web application was stopped. This is very likely to create a memory leak.
-webappClassLoader.clearThreadLocalDebugClear=To simplify the process of tracing memory leaks, the key has been forcibly removed.
-webappClassLoader.clearThreadLocalClear=To prevent a memory leak, the ThreadLocal has been forcibly removed.
-webappClassLoader.clearThreadLocalFail=Failed to clear ThreadLocal references for web application [{0}]
-webappClassLoader.clearThreadLocal.badKey=Unable to determine string representation of key of type [{0}]
-webappClassLoader.clearThreadLocal.badValue=Unable to determine string representation of value of type [{0}]
-webappClassLoader.clearThreadLocal.unknown=Unknown
+webappClassLoader.checkThreadLocalsForLeaks.badKey=Unable to determine string representation of key of type [{0}]
+webappClassLoader.checkThreadLocalsForLeaks.badValue=Unable to determine string representation of value of type [{0}]
+webappClassLoader.checkThreadLocalsForLeaks.unknown=Unknown
+webappClassLoader.checkThreadLocalsForLeaks=The web application [{0}] created a ThreadLocal with key of type [{1}] (value [{2}]) and a value of type [{3}] (value [{4}]) but failed to remove it when the web application was stopped. Threads are going to be renewed over time to try and avoid a probable memory leak. 
+webappClassLoader.checkThreadLocalsForLeaksDebug=The web application [{0}] created a ThreadLocal with key of type [{1}] (value [{2}]). The ThreadLocal has been correctly set to null and the key will be removed by GC.
+webappClassLoader.checkThreadLocalsForLeaksFail=Failed to check for ThreadLocal references for web application [{0}]
 webappClassLoader.stopThreadFail=Failed to terminate thread named [{0}] for web application [{1}]
 webappClassLoader.stopTimerThreadFail=Failed to terminate TimerThread named [{0}] for web application [{1}]
 webappClassLoader.validationErrorJarPath=Unable to validate JAR entry with name {0}

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=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java Sun Dec  5 22:54:05 2010
@@ -458,15 +458,6 @@ public class WebappClassLoader
     private boolean clearReferencesStopTimerThreads = false;
 
     /**
-     * Should Tomcat attempt to clear any ThreadLocal objects that are instances
-     * of classes loaded by this class loader. Failure to remove any such
-     * objects will result in a memory leak on web application stop, undeploy or
-     * reload. It is disabled by default since the clearing of the ThreadLocal
-     * objects is not performed in a thread-safe manner.
-     */
-    private boolean clearReferencesThreadLocals = false;
-    
-    /**
      * Should Tomcat call {@link org.apache.juli.logging.LogFactory#release()}
      * when the class loader is stopped? If not specified, the default value
      * of <code>true</code> is used. Changing the default setting is likely to
@@ -754,25 +745,6 @@ public class WebappClassLoader
      }
 
 
-     /**
-      * Return the clearReferencesThreadLocals flag for this Context.
-      */
-     public boolean getClearReferencesThreadLocals() {
-         return (this.clearReferencesThreadLocals);
-     }
-
-
-     /**
-      * Set the clearReferencesThreadLocals feature for this Context.
-      *
-      * @param clearReferencesThreadLocals The new flag value
-      */
-     public void setClearReferencesThreadLocals(
-             boolean clearReferencesThreadLocals) {
-         this.clearReferencesThreadLocals = clearReferencesThreadLocals;
-     }
-
-
     // ------------------------------------------------------- Reloader Methods
 
 
@@ -1949,8 +1921,8 @@ public class WebappClassLoader
         // Stop any threads the web application started
         clearReferencesThreads();
         
-        // Clear any ThreadLocals loaded by this class loader
-        clearReferencesThreadLocals();
+        // Check for leaks triggered by ThreadLocals loaded by this class loader
+        checkThreadLocalsForLeaks();
         
         // Clear RMI Targets loaded by this class loader
         clearReferencesRmiTargets();
@@ -2347,7 +2319,7 @@ public class WebappClassLoader
         }
     }
 
-    private void clearReferencesThreadLocals() {
+    private void checkThreadLocalsForLeaks() {
         Thread[] threads = getThreads();
 
         try {
@@ -2371,73 +2343,66 @@ public class WebappClassLoader
                 if (threads[i] != null) {
                     // Clear the first map
                     threadLocalMap = threadLocalsField.get(threads[i]);
-                    clearThreadLocalMap(threadLocalMap, tableField);
+                    checkThreadLocalMapForLeaks(threadLocalMap, tableField);
                     // Clear the second map
                     threadLocalMap =
                         inheritableThreadLocalsField.get(threads[i]);
-                    clearThreadLocalMap(threadLocalMap, tableField);
+                    checkThreadLocalMapForLeaks(threadLocalMap, tableField);
                 }
             }
         } catch (SecurityException e) {
-            log.warn(sm.getString("webappClassLoader.clearThreadLocalFail",
+            log.warn(sm.getString("webappClassLoader.checkThreadLocalsForLeaksFail",
                     contextName), e);
         } catch (NoSuchFieldException e) {
-            log.warn(sm.getString("webappClassLoader.clearThreadLocalFail",
+            log.warn(sm.getString("webappClassLoader.checkThreadLocalsForLeaksFail",
                     contextName), e);
         } catch (ClassNotFoundException e) {
-            log.warn(sm.getString("webappClassLoader.clearThreadLocalFail",
+            log.warn(sm.getString("webappClassLoader.checkThreadLocalsForLeaksFail",
                     contextName), e);
         } catch (IllegalArgumentException e) {
-            log.warn(sm.getString("webappClassLoader.clearThreadLocalFail",
+            log.warn(sm.getString("webappClassLoader.checkThreadLocalsForLeaksFail",
                     contextName), e);
         } catch (IllegalAccessException e) {
-            log.warn(sm.getString("webappClassLoader.clearThreadLocalFail",
+            log.warn(sm.getString("webappClassLoader.checkThreadLocalsForLeaksFail",
                     contextName), e);
         } catch (NoSuchMethodException e) {
-            log.warn(sm.getString("webappClassLoader.clearThreadLocalFail",
+            log.warn(sm.getString("webappClassLoader.checkThreadLocalsForLeaksFail",
                     contextName), e);
         } catch (InvocationTargetException e) {
-            log.warn(sm.getString("webappClassLoader.clearThreadLocalFail",
+            log.warn(sm.getString("webappClassLoader.checkThreadLocalsForLeaksFail",
                     contextName), e);
         }       
     }
 
 
     /*
-     * Clears the given thread local map object. Also pass in the field that
+     * Analyzes 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.
      */
-    private void clearThreadLocalMap(Object map, Field internalTableField)
+    private void checkThreadLocalMapForLeaks(Object map, Field internalTableField)
             throws NoSuchMethodException, IllegalAccessException,
             NoSuchFieldException, InvocationTargetException {
         if (map != null) {
-            Method mapRemove =
-                map.getClass().getDeclaredMethod("remove",
-                        ThreadLocal.class);
-            mapRemove.setAccessible(true);
             Object[] table = (Object[]) internalTableField.get(map);
-            int staleEntriesCount = 0;
             if (table != null) {
                 for (int j =0; j < table.length; j++) {
                     if (table[j] != null) {
-                        boolean remove = false;
+                        boolean potentialLeak = false;
                         // Check the key
                         Object key = ((Reference<?>) table[j]).get();
-                        if (this.equals(key) ||
-                                isLoadedByThisWebappClassLoader(key)) {
-                            remove = true;
+                        if (this.equals(key) || loadedByThisOrChild(key)) {
+                            potentialLeak = true;
                         }
                         // Check the value
                         Field valueField =
                             table[j].getClass().getDeclaredField("value");
                         valueField.setAccessible(true);
                         Object value = valueField.get(table[j]);
-                        if (this.equals(value) ||
-                                isLoadedByThisWebappClassLoader(value)) {
-                            remove = true;
+                        if (this.equals(value) || loadedByThisOrChild(value)) {
+                            potentialLeak = true;
                         }
-                        if (remove) {
+                        if (potentialLeak) {
                             Object[] args = new Object[5];
                             args[0] = contextName;
                             if (key != null) {
@@ -2446,10 +2411,10 @@ public class WebappClassLoader
                                     args[2] = key.toString();
                                 } catch (Exception e) {
                                     log.error(sm.getString(
-                                            "webappClassLoader.clearThreadLocal.badKey",
+                                            "webappClassLoader.checkThreadLocalsForLeaks.badKey",
                                             args[1]), e);
                                     args[2] = sm.getString(
-                                            "webappClassLoader.clearThreadLocal.unknown");
+                                            "webappClassLoader.checkThreadLocalsForLeaks.unknown");
                                 }
                             }
                             if (value != null) {
@@ -2458,48 +2423,27 @@ public class WebappClassLoader
                                     args[4] = value.toString();
                                 } catch (Exception e) {
                                     log.error(sm.getString(
-                                            "webappClassLoader.clearThreadLocal.badValue",
+                                            "webappClassLoader.checkThreadLocalsForLeaks.badValue",
                                             args[3]), e);
                                     args[4] = sm.getString(
-                                    "webappClassLoader.clearThreadLocal.unknown");
+                                    "webappClassLoader.checkThreadLocalsForLeaks.unknown");
                                 }
                             }
                             if (value == null) {
                                 if (log.isDebugEnabled()) {
                                     log.debug(sm.getString(
-                                            "webappClassLoader.clearThreadLocalDebug",
+                                            "webappClassLoader.checkThreadLocalsForLeaksDebug",
                                             args));
-                                    if (clearReferencesThreadLocals) {
-                                        log.debug(sm.getString(
-                                                "webappClassLoader.clearThreadLocalDebugClear"));
-                                    }
                                 }
                             } else {
                                 log.error(sm.getString(
-                                        "webappClassLoader.clearThreadLocal",
+                                        "webappClassLoader.checkThreadLocalsForLeaks",
                                         args));
-                                if (clearReferencesThreadLocals) {
-                                    log.info(sm.getString(
-                                            "webappClassLoader.clearThreadLocalClear"));
-                                }
-                            }
-                            if (clearReferencesThreadLocals) {
-                                if (key == null) {
-                                  staleEntriesCount++;
-                                } else {
-                                  mapRemove.invoke(map, key);
-                                }
                             }
                         }
                     }
                 }
             }
-            if (staleEntriesCount > 0) {
-                Method mapRemoveStale =
-                    map.getClass().getDeclaredMethod("expungeStaleEntries");
-                mapRemoveStale.setAccessible(true);
-                mapRemoveStale.invoke(map);
-            }
         }
     }
 
@@ -2512,15 +2456,23 @@ public class WebappClassLoader
     }
     
     /**
-     * @param o object to test
+     * @param o object to test, may be null
      * @return <code>true</code> if o has been loaded by the current classloader
      * or one of its descendants.
      */
-    private boolean isLoadedByThisWebappClassLoader(Object o) {
+    private boolean loadedByThisOrChild(Object o) {
         if (o == null) {
             return false;
         }
-        ClassLoader cl = o.getClass().getClassLoader();
+
+        Class<?> clazz;
+        if (o instanceof Class) {
+            clazz = (Class<?>) o;
+        } else {
+            clazz = o.getClass();
+        }
+
+        ClassLoader cl = clazz.getClassLoader();
         while (cl != null) {
             if(cl == this) {
                 return true;
@@ -2718,24 +2670,6 @@ public class WebappClassLoader
 
 
     /**
-     * Determine whether a class was loaded by this class loader or one of
-     * its child class loaders.
-     */
-    protected boolean loadedByThisOrChild(Class<? extends Object> clazz)
-    {
-        boolean result = false;
-        for (ClassLoader classLoader = clazz.getClassLoader();
-                null != classLoader; classLoader = classLoader.getParent()) {
-            if (classLoader.equals(this)) {
-                result = true;
-                break;
-            }
-        }
-        return result;
-    }    
-
-
-    /**
      * Used to periodically signal to the classloader to release JAR resources.
      */
     protected boolean openJARs() {

Modified: tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java?rev=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java Sun Dec  5 22:54:05 2010
@@ -591,8 +591,6 @@ public class WebappLoader extends Lifecy
                         ((StandardContext) container).getClearReferencesStopThreads());
                 classLoader.setClearReferencesStopTimerThreads(
                         ((StandardContext) container).getClearReferencesStopTimerThreads());
-                classLoader.setClearReferencesThreadLocals(
-                        ((StandardContext) container).getClearReferencesThreadLocals());
             }
 
             for (int i = 0; i < repositories.length; i++) {

Added: tomcat/trunk/java/org/apache/tomcat/util/threads/Constants.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/threads/Constants.java?rev=1042482&view=auto
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/threads/Constants.java (added)
+++ tomcat/trunk/java/org/apache/tomcat/util/threads/Constants.java Sun Dec  5 22:54:05 2010
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.threads;
+
+/**
+ * Static constants for this package.
+ */
+public final class Constants {
+
+    public static final String Package = "org.apache.tomcat.util.threads";
+
+}

Added: tomcat/trunk/java/org/apache/tomcat/util/threads/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/threads/LocalStrings.properties?rev=1042482&view=auto
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/threads/LocalStrings.properties (added)
+++ tomcat/trunk/java/org/apache/tomcat/util/threads/LocalStrings.properties Sun Dec  5 22:54:05 2010
@@ -0,0 +1,16 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+threadPoolExecutor.threadStoppedToAvoidPotentialLeak=Stopping thread {0} to avoid potential memory leaks after a context was stopped.

Modified: tomcat/trunk/java/org/apache/tomcat/util/threads/TaskQueue.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/threads/TaskQueue.java?rev=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/threads/TaskQueue.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/threads/TaskQueue.java Sun Dec  5 22:54:05 2010
@@ -34,6 +34,10 @@ public class TaskQueue extends LinkedBlo
     private static final long serialVersionUID = 1L;
 
     private ThreadPoolExecutor parent = null;
+    
+    // no need to be volatile, the one times when we change and read it occur in
+    // a single thread (the one that did stop a context and fired listeners)
+    private Integer forcedRemainingCapacity = null;
 
     public TaskQueue() {
         super();
@@ -74,4 +78,44 @@ public class TaskQueue extends LinkedBlo
         //if we reached here, we need to add it to the queue
         return super.offer(o);
     }
+
+
+    @Override
+    public Runnable poll(long timeout, TimeUnit unit) throws InterruptedException {
+        Runnable runnable = super.poll(timeout, unit);
+        if (runnable == null && parent != null) {
+            // the poll timed out, it gives an opportunity to stop the current
+            // thread if needed to avoid memory leaks.
+            parent.stopCurrentThreadIfNeeded();
+        }
+        return runnable;
+    }
+    
+
+    @Override
+    public Runnable take() throws InterruptedException {
+        if (parent != null && parent.currentThreadShouldBeStopped()) {
+            return poll(parent.getKeepAliveTime(TimeUnit.MILLISECONDS), TimeUnit.MILLISECONDS);
+            //yes, this may return null (in case of timeout) which normally does not occur with take()
+            //but the ThreadPoolExecutor implementation allows this
+        }
+        return super.take();
+    }
+
+    @Override
+    public int remainingCapacity() {
+        if(forcedRemainingCapacity != null) {
+            // ThreadPoolExecutor.setCorePoolSize checks that
+            // remainingCapacity==0 to allow to interrupt idle threads
+            // I don't see why, but this hack allows to conform to this
+            // "requirement"
+            return forcedRemainingCapacity.intValue();
+        }
+        return super.remainingCapacity();
+    }
+
+    public void setForcedRemainingCapacity(Integer forcedRemainingCapacity) {
+        this.forcedRemainingCapacity = forcedRemainingCapacity;
+    }
+
 }

Added: tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThread.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThread.java?rev=1042482&view=auto
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThread.java (added)
+++ tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThread.java Sun Dec  5 22:54:05 2010
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.threads;
+
+/**
+ * A Thread implementation that records the time at which it was created.
+ * 
+ * @author slaurent
+ * 
+ */
+public class TaskThread extends Thread {
+
+    private final long creationTime;
+
+    public TaskThread(ThreadGroup group, Runnable target, String name) {
+        super(group, target, name);
+        this.creationTime = System.currentTimeMillis();
+    }
+
+    public TaskThread(ThreadGroup group, Runnable target, String name,
+            long stackSize) {
+        super(group, target, name, stackSize);
+        this.creationTime = System.currentTimeMillis();
+    }
+
+    /**
+     * @return the time (in ms) at which this thread was created
+     */
+    public final long getCreationTime() {
+        return creationTime;
+    }
+
+}

Modified: tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThreadFactory.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThreadFactory.java?rev=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThreadFactory.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThreadFactory.java Sun Dec  5 22:54:05 2010
@@ -39,7 +39,7 @@ public class TaskThreadFactory implement
 
     @Override
     public Thread newThread(Runnable r) {
-        Thread t = new Thread(group, r, namePrefix + threadNumber.getAndIncrement());
+        TaskThread t = new TaskThread(group, r, namePrefix + threadNumber.getAndIncrement());
         t.setDaemon(daemon);
         t.setPriority(threadPriority);
         return t;

Modified: tomcat/trunk/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java?rev=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java Sun Dec  5 22:54:05 2010
@@ -16,12 +16,19 @@
  */
 package org.apache.tomcat.util.threads;
 
+import java.lang.Thread.UncaughtExceptionHandler;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.RejectedExecutionHandler;
 import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
+
 /**
  * Same as a java.util.concurrent.ThreadPoolExecutor but implements a much more efficient
  * {@link #getSubmittedCount()} method, to be used to properly handle the work queue.
@@ -31,7 +38,14 @@ import java.util.concurrent.atomic.Atomi
  *
  */
 public class ThreadPoolExecutor extends java.util.concurrent.ThreadPoolExecutor {
-    
+    /**
+     * The string manager for this package.
+     */
+    protected static final StringManager sm = StringManager
+            .getManager(Constants.Package);
+
+    private static final Log log = LogFactory.getLog(ThreadPoolExecutor.class);
+
     /**
      * The number of tasks submitted but not yet finished. This includes tasks
      * in the queue and tasks that have been handed to a worker thread but the
@@ -39,7 +53,20 @@ public class ThreadPoolExecutor extends 
      * This number is always greater or equal to {@link #getActiveCount()}.
      */
     private final AtomicInteger submittedCount = new AtomicInteger(0);
-    
+    private final AtomicLong lastContextStoppedTime = new AtomicLong(0L);
+
+    /**
+     * Most recent time in ms when a thread decided to kill itself to avoid
+     * potential memory leaks. Useful to throttle the rate of renewals of
+     * threads.
+     */
+    private final AtomicLong lastTimeThreadKilledItself = new AtomicLong(0L);
+
+    /**
+     * Delay in ms between 2 threads being renewed. If negative, do not renew threads.
+     */
+    private long threadRenewalDelay = 1000L;
+
     public ThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit, BlockingQueue<Runnable> workQueue, RejectedExecutionHandler handler) {
         super(corePoolSize, maximumPoolSize, keepAliveTime, unit, workQueue, handler);
     }
@@ -56,10 +83,63 @@ public class ThreadPoolExecutor extends 
     public ThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit, BlockingQueue<Runnable> workQueue) {
         super(corePoolSize, maximumPoolSize, keepAliveTime, unit, workQueue, new RejectHandler());
     }
+    
+    public long getThreadRenewalDelay() {
+        return threadRenewalDelay;
+    }
+
+    public void setThreadRenewalDelay(long threadRenewalDelay) {
+        this.threadRenewalDelay = threadRenewalDelay;
+    }
 
     @Override
     protected void afterExecute(Runnable r, Throwable t) {
         submittedCount.decrementAndGet();
+
+        if (t == null) {
+            stopCurrentThreadIfNeeded();
+        }
+    }
+
+    /**
+     * If the current thread was started before the last time when a context was
+     * stopped, an exception is thrown so that the current thread is stopped.
+     */
+    protected void stopCurrentThreadIfNeeded() {
+        if (currentThreadShouldBeStopped()) {
+            long lastTime = lastTimeThreadKilledItself.longValue();
+            if (lastTime + threadRenewalDelay < System.currentTimeMillis()) {
+                if (lastTimeThreadKilledItself.compareAndSet(lastTime,
+                        System.currentTimeMillis() + 1)) {
+                    // OK, it's really time to dispose of this thread
+
+                    final String msg = sm.getString(
+                                    "threadPoolExecutor.threadStoppedToAvoidPotentialLeak",
+                                    Thread.currentThread().getName());
+
+                    Thread.currentThread().setUncaughtExceptionHandler(
+                            new UncaughtExceptionHandler() {
+                                @Override
+                                public void uncaughtException(Thread t,
+                                        Throwable e) {
+                                    // yes, swallow the exception
+                                    log.debug(msg);
+                                }
+                            });
+                    throw new RuntimeException(msg);
+                }
+            }
+        }
+    }
+    
+    protected boolean currentThreadShouldBeStopped() {
+        if (threadRenewalDelay >= 0 && Thread.currentThread() instanceof TaskThread) {
+            TaskThread currentTaskThread = (TaskThread) Thread.currentThread();
+            if (currentTaskThread.getCreationTime() < this.lastContextStoppedTime.longValue()) {
+                return true;
+            }
+        }
+        return false;
     }
 
     public int getSubmittedCount() {
@@ -111,6 +191,41 @@ public class ThreadPoolExecutor extends 
             
         }
     }
+
+    public void contextStopping() {
+        this.lastContextStoppedTime.set(System.currentTimeMillis());
+
+        // save the current pool parameters to restore them later
+        int savedCorePoolSize = this.getCorePoolSize();
+        TaskQueue taskQueue = getQueue() instanceof TaskQueue ? (TaskQueue) getQueue() : null;
+        if (taskQueue != null) {
+            // note by slaurent : quite oddly threadPoolExecutor.setCorePoolSize
+            // checks that queue.remainingCapacity()==0. I did not understand
+            // why, but to get the intended effect of waking up idle threads, I
+            // temporarily fake this condition.
+            taskQueue.setForcedRemainingCapacity(0);
+        }
+
+        // setCorePoolSize(0) wakes idle threads
+        this.setCorePoolSize(0);
+
+        // wait a little so that idle threads wake and poll the queue again,
+        // this time always with a timeout (queue.poll() instead of queue.take())
+        // even if we did not wait enough, TaskQueue.take() takes care of timing out
+        // so that we are sure that all threads of the pool are renewed in a limited
+        // time, something like (threadKeepAlive + longest request time)
+        try {
+            Thread.sleep(200L);
+        } catch (InterruptedException e) {
+            //yes, ignore
+        }
+        
+        if (taskQueue != null) {
+            // ok, restore the state of the queue and pool
+            taskQueue.setForcedRemainingCapacity(null);
+        }
+        this.setCorePoolSize(savedCorePoolSize);
+    }
     
     private static class RejectHandler implements RejectedExecutionHandler {
         @Override

Modified: tomcat/trunk/res/confinstall/server_1.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/res/confinstall/server_1.xml?rev=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/res/confinstall/server_1.xml (original)
+++ tomcat/trunk/res/confinstall/server_1.xml Sun Dec  5 22:54:05 2010
@@ -28,6 +28,7 @@
   <!-- Prevent memory leaks due to use of particular java/javax APIs-->
   <Listener className="org.apache.catalina.core.JreMemoryLeakPreventionListener" />
   <Listener className="org.apache.catalina.mbeans.GlobalResourcesLifecycleListener" />
+  <Listener className="org.apache.catalina.core.ThreadLocalLeakPreventionListener" />
 
   <!-- Global JNDI resources
        Documentation at /docs/jndi-resources-howto.html

Propchange: tomcat/trunk/webapps/
------------------------------------------------------------------------------
--- svn:ignore (added)
+++ svn:ignore Sun Dec  5 22:54:05 2010
@@ -0,0 +1,2 @@
+petcare
+petcare.war

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Sun Dec  5 22:54:05 2010
@@ -61,6 +61,10 @@
         application if there's no session. Patch provided by Marc Guillemot.
         (slaurent)
       </add>
+      <add>
+        <bug>49159</bug>: Improve memory leak protection by renewing threads of
+        the pool when a web application is stopped. (slaurent)
+      </add>
       <fix>
         <bug>49650</bug>: Remove unnecessary entries package.access property
         defined in catalina.properties. Patch provided by Owen Farrell. (markt) 

Modified: tomcat/trunk/webapps/docs/config/context.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/context.xml?rev=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/config/context.xml (original)
+++ tomcat/trunk/webapps/docs/config/context.xml Sun Dec  5 22:54:05 2010
@@ -497,21 +497,22 @@
         not specified, the default value of <code>false</code> will be used.</p>
       </attribute>
 
-      <attribute name="clearReferencesThreadLocals" required="false">
-        <p>If <code>true</code>, Tomcat attempts to clear any ThreadLocal
-        objects that are instances of classes loaded by this class loader.
-        Failure to remove any such objects will result in a memory leak on web
-        application stop, undeploy or reload.  If not specified, the default
-        value of <code>false</code> will be used since the clearing of the
-        ThreadLocal objects is not performed in a thread-safe manner.</p>
-      </attribute>
-
       <attribute name="processTlds" required="false">
         <p>Whether the context should process TLDs on startup.  The default
         is true.  The false setting is intended for special cases
         that know in advance TLDs are not part of the webapp.</p>
       </attribute>
 
+      <attribute name="renewThreadsWhenStoppingContext" required="false">
+        <p>If <code>true</code>, when this context is stopped, Tomcat renews all
+        the threads from the thread pool that was used to serve this context.
+        This also requires that the 
+        <code>ThreadLocalLeakPreventionListener</code> be configured in 
+        <code>server.xml</code> and that the <code>threadRenewalDelay</code>
+        property of the <code>Executor</code> be &gt;=0. If not specified, the 
+        default value of <code>true</code> will be used.</p>
+      </attribute>
+
       <attribute name="swallowOutput" required="false">
         <p>If the value of this flag is <code>true</code>, the bytes output to
         System.out and System.err by the web application will be redirected to

Modified: tomcat/trunk/webapps/docs/config/executor.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/executor.xml?rev=1042482&r1=1042481&r2=1042482&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/config/executor.xml (original)
+++ tomcat/trunk/webapps/docs/config/executor.xml Sun Dec  5 22:54:05 2010
@@ -106,6 +106,11 @@
       <p>(boolean) Whether minSpareThreads should be started when starting the Executor or not,
           the default is <code>false</code></p>
     </attribute>
+    <attribute name="threadRenewalDelay" required="false">
+      <p>After a context is stopped, threads in the pool are renewed. To avoid renewing all threads at the same time, 
+        this delay is observed between 2 threads being renewed. Value is in ms, default value is 1000ms.
+        If negative, threads are not renewed.</p>
+    </attribute>
   </attributes>
 
 



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


Re: svn commit: r1042482 - in /tomcat/trunk: ./ conf/ java/org/apache/catalina/core/ java/org/apache/catalina/loader/ java/org/apache/tomcat/util/threads/ res/confinstall/ webapps/ webapps/docs/ webapps/docs/config/

Posted by Felix Schumacher <fe...@internetallee.de>.
Am Montag, den 06.12.2010, 22:01 +0100 schrieb Sylvain Laurent:
> > 
> >> +    public void lifecycleEvent(LifecycleEvent event) {
> >> +        try {
> >> +            Lifecycle lifecycle = event.getLifecycle();
> >> +            if (Lifecycle.AFTER_START_EVENT.equals(event.getType())
> >> +                    && lifecycle instanceof Server) {
> > With the operator on the new line it is easy to miss what is going on.
> > Generally, Tomcat style is to put the operator on the first line. e.g.
> > if (Lifecycle.AFTER_START_EVENT.equals(event.getType()) &&
> >        lifecycle instanceof Server) {
> > 
> > There are multiple instances of this throughout the commit.
> 
> Still eclipse. But I did not manage to find a setting to change this behavior :-(
It is "Line Wrapping->Expressions->Binary expressions" [x] Wrap before
operator.

hth
 Felix


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


Re: svn commit: r1042482 - in /tomcat/trunk: ./ conf/ java/org/apache/catalina/core/ java/org/apache/catalina/loader/ java/org/apache/tomcat/util/threads/ res/confinstall/ webapps/ webapps/docs/ webapps/docs/config/

Posted by Mark Thomas <ma...@apache.org>.
On 07/12/2010 17:49, Sylvain Laurent wrote:
> 
> On 7 déc. 2010, at 09:41, Rainer Jung wrote:
>>> 
>> 
>> Just in case Mark's remark a) wasn't clear: after building Tomcat
>> you can debug inside the sub directory output/build which contains
>> a full installation.
> 
> I had understood Mark, but it's not practical for debugging... I
> prefer to debug tomcat within eclipse and use the classes compiled by
> eclipse. I'll just ignore the work and log directories when
> committing changes. Though if we could ignore them that would be
> nice... 

Debugging that way isn't something I would want to do or recommend that
others do but if it works for you, lets not make your life difficult. My
previous veto was due to:
a) I thought it was bad practice (too easy to commit local changes to
config files, too easy to include local changes in a release build) and
b) because it reverted a change I needed.

I won't object to adding the ignores you need providing that build.xml
is updated to ensure that they are also ignored as part of any build. A
quick glance shows temp, work and logs should be OK but
conf/<enginename> will need a change or two to ensure it is ignored.

Mark

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


Re: svn commit: r1042482 - in /tomcat/trunk: ./ conf/ java/org/apache/catalina/core/ java/org/apache/catalina/loader/ java/org/apache/tomcat/util/threads/ res/confinstall/ webapps/ webapps/docs/ webapps/docs/config/

Posted by Sylvain Laurent <sy...@m4x.org>.
On 7 déc. 2010, at 09:41, Rainer Jung wrote:
>> 
> 
> Just in case Mark's remark a) wasn't clear: after building Tomcat you can debug inside the sub directory output/build which contains a full installation.

I had understood Mark, but it's not practical for debugging... I prefer to debug tomcat within eclipse and use the classes compiled by eclipse.
I'll just ignore the work and log directories when committing changes. Though if we could ignore them that would be nice...
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1042482 - in /tomcat/trunk: ./ conf/ java/org/apache/catalina/core/ java/org/apache/catalina/loader/ java/org/apache/tomcat/util/threads/ res/confinstall/ webapps/ webapps/docs/ webapps/docs/config/

Posted by Rainer Jung <ra...@kippdata.de>.
On 06.12.2010 22:01, Sylvain Laurent wrote:
> On 6 déc. 2010, at 01:09, Mark Thomas wrote:
>> On 05/12/2010 22:54, slaurent@apache.org wrote:

>>> Modified:
>>>     tomcat/trunk/   (props changed)
>>>     tomcat/trunk/conf/   (props changed)
>>>     tomcat/trunk/webapps/    (props changed)
>> -1 to all these changes.
>> a) This is not a Tomcat instance for running. That is created in output.
>> b) It reverts a useful change I made earlier today
> Sorry, I did not realize there could be changes on directories with SVN. For debugging I launch tomcat from the base directory and this creates directories likes log, work, etc...
> I believe you fixed this.

Just in case Mark's remark a) wasn't clear: after building Tomcat you 
can debug inside the sub directory output/build which contains a full 
installation.

Regards,

Rainer

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


Re: svn commit: r1042482 - in /tomcat/trunk: ./ conf/ java/org/apache/catalina/core/ java/org/apache/catalina/loader/ java/org/apache/tomcat/util/threads/ res/confinstall/ webapps/ webapps/docs/ webapps/docs/config/

Posted by Mark Thomas <ma...@apache.org>.
On 06/12/2010 21:01, Sylvain Laurent wrote:
> I fixed most of your points except one, see below.

>>> +    protected void processContainerRemoveChild(Container parent, Container child) {
>>> +
>>> +        if (log.isDebugEnabled())
>>> +            log.debug("Process removeChild[parent=" + parent + ",child="
>>> +                    + child + "]");
>>> +
>>> +        try {
>>> +            if (child instanceof Context) {
>>> +                Context context = (Context) child;
>>> +                context.removeLifecycleListener(this);
>>> +            } else if (child instanceof Host) {
>>> +                Host host = (Host) child;
>>> +                host.removeContainerListener(this);
>>> +            } else if (child instanceof Engine) {
>>> +                Engine engine = (Engine) child;
>>> +                engine.removeContainerListener(this);
>>> +            }
>> Why all this? Just use:
>> child.removeLifecycleListener(this);
> 
> No, it's not the same. The listener is interested in both LifeCycle events for Context, and Container events for Host and Engine. These are not the same things.

You can still drop the casts though and just call
child.removeLifecycleListener() and child.removeContainerListener()

Some of the other fixes still look a little odd. I suspect the Eclipse
formatter. I'll highlight those in a separate e-mail.

Mark

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


Re: svn commit: r1042482 - in /tomcat/trunk: ./ conf/ java/org/apache/catalina/core/ java/org/apache/catalina/loader/ java/org/apache/tomcat/util/threads/ res/confinstall/ webapps/ webapps/docs/ webapps/docs/config/

Posted by Sylvain Laurent <sy...@m4x.org>.
I fixed most of your points except one, see below.

On 6 déc. 2010, at 01:09, Mark Thomas wrote:

> On 05/12/2010 22:54, slaurent@apache.org wrote:
>> Author: slaurent
>> Date: Sun Dec  5 22:54:05 2010
>> New Revision: 1042482
>> 
>> URL: http://svn.apache.org/viewvc?rev=1042482&view=rev
> 
> General comments:
> 
> Numerous new lines with length > 80 chars.

Actually I had configured eclipse for 80 chars but the default line-wrapping settings are such that it does not wrap in several cases...

> ASF policy is to strongly discourage author tags.
fixed

> Your svn client should be configured to set svn:eol-style native on new
> text files.
OK, done for future new files.

>> Modified:
>>    tomcat/trunk/   (props changed)
>>    tomcat/trunk/conf/   (props changed)
>>    tomcat/trunk/webapps/    (props changed)
> -1 to all these changes.
> a) This is not a Tomcat instance for running. That is created in output.
> b) It reverts a useful change I made earlier today
Sorry, I did not realize there could be changes on directories with SVN. For debugging I launch tomcat from the base directory and this creates directories likes log, work, etc...
I believe you fixed this.

>> Added: tomcat/trunk/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java
> 
>> +    private static final Log log = LogFactory
>> +            .getLog(ThreadLocalLeakPreventionListener.class);
> That is an odd place for a line-break that I find hard to read. I find
> either of the following easier to read:
> 
> private static final Log log =
>        LogFactory.getLog(ThreadLocalLeakPreventionListener.class);
> 
> private static final Log log = LogFactory.getLog(
>        ThreadLocalLeakPreventionListener.class);
> 
> There are multiple instances of this throughout the commit.

Still the default eclipse formatter... We should really provide eclipse formatter settings. Formatting code by hand is really a waste of time.

> 
>> +    public void lifecycleEvent(LifecycleEvent event) {
>> +        try {
>> +            Lifecycle lifecycle = event.getLifecycle();
>> +            if (Lifecycle.AFTER_START_EVENT.equals(event.getType())
>> +                    && lifecycle instanceof Server) {
> With the operator on the new line it is easy to miss what is going on.
> Generally, Tomcat style is to put the operator on the first line. e.g.
> if (Lifecycle.AFTER_START_EVENT.equals(event.getType()) &&
>        lifecycle instanceof Server) {
> 
> There are multiple instances of this throughout the commit.

Still eclipse. But I did not manage to find a setting to change this behavior :-(

> 
>> +        } catch (Exception e) {
>> +            log.error("Exception processing event " + event, e);
>> +        }
> Error messages really should be using a StringManager.
> There are multiple instances of this throughout the commit.

done.

>> +    protected void processContainerRemoveChild(Container parent, Container child) {
>> +
>> +        if (log.isDebugEnabled())
>> +            log.debug("Process removeChild[parent=" + parent + ",child="
>> +                    + child + "]");
>> +
>> +        try {
>> +            if (child instanceof Context) {
>> +                Context context = (Context) child;
>> +                context.removeLifecycleListener(this);
>> +            } else if (child instanceof Host) {
>> +                Host host = (Host) child;
>> +                host.removeContainerListener(this);
>> +            } else if (child instanceof Engine) {
>> +                Engine engine = (Engine) child;
>> +                engine.removeContainerListener(this);
>> +            }
> Why all this? Just use:
> child.removeLifecycleListener(this);

No, it's not the same. The listener is interested in both LifeCycle events for Context, and Container events for Host and Engine. These are not the same things.

> Does the fact the Container implements Lifecycle simplify any of the
> other code here?
Can't tell.

>> Modified: tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
>> -    private void clearThreadLocalMap(Object map, Field internalTableField)
>> +    private void checkThreadLocalMapForLeaks(Object map, Field internalTableField)
>>             throws NoSuchMethodException, IllegalAccessException,
>>             NoSuchFieldException, InvocationTargetException {
> Not all of those Exceptions are required after this commit.
Fixed.



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


Re: svn commit: r1042482 - in /tomcat/trunk: ./ conf/ java/org/apache/catalina/core/ java/org/apache/catalina/loader/ java/org/apache/tomcat/util/threads/ res/confinstall/ webapps/ webapps/docs/ webapps/docs/config/

Posted by Mark Thomas <ma...@apache.org>.
On 05/12/2010 22:54, slaurent@apache.org wrote:
> Author: slaurent
> Date: Sun Dec  5 22:54:05 2010
> New Revision: 1042482
> 
> URL: http://svn.apache.org/viewvc?rev=1042482&view=rev

General comments:

Numerous new lines with length > 80 chars.

ASF policy is to strongly discourage author tags.

Your svn client should be configured to set svn:eol-style native on new
text files.

I did wonder about the fixing over time nature of this approach vs. the
immediate fixing of the previous approach and whether or not we should
give users the option of both. On reflection I think providing both will
add a fair amount of complexity and given that this is trying to fix
what is essentially an application bug then I think just having one
approach is fine and this is a better approach than the previous
implementation.

Specific comments in-line.

> Modified:
>     tomcat/trunk/   (props changed)
>     tomcat/trunk/conf/   (props changed)
>     tomcat/trunk/webapps/    (props changed)
-1 to all these changes.
a) This is not a Tomcat instance for running. That is created in output.
b) It reverts a useful change I made earlier today


> Added: tomcat/trunk/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java

> +    private static final Log log = LogFactory
> +            .getLog(ThreadLocalLeakPreventionListener.class);
That is an odd place for a line-break that I find hard to read. I find
either of the following easier to read:

private static final Log log =
        LogFactory.getLog(ThreadLocalLeakPreventionListener.class);

private static final Log log = LogFactory.getLog(
        ThreadLocalLeakPreventionListener.class);

There are multiple instances of this throughout the commit.


> +    public void lifecycleEvent(LifecycleEvent event) {
> +        try {
> +            Lifecycle lifecycle = event.getLifecycle();
> +            if (Lifecycle.AFTER_START_EVENT.equals(event.getType())
> +                    && lifecycle instanceof Server) {
With the operator on the new line it is easy to miss what is going on.
Generally, Tomcat style is to put the operator on the first line. e.g.
if (Lifecycle.AFTER_START_EVENT.equals(event.getType()) &&
        lifecycle instanceof Server) {

There are multiple instances of this throughout the commit.


> +        } catch (Exception e) {
> +            log.error("Exception processing event " + event, e);
> +        }
Error messages really should be using a StringManager.
There are multiple instances of this throughout the commit.


> +    protected void processContainerRemoveChild(Container parent, Container child) {
> +
> +        if (log.isDebugEnabled())
> +            log.debug("Process removeChild[parent=" + parent + ",child="
> +                    + child + "]");
> +
> +        try {
> +            if (child instanceof Context) {
> +                Context context = (Context) child;
> +                context.removeLifecycleListener(this);
> +            } else if (child instanceof Host) {
> +                Host host = (Host) child;
> +                host.removeContainerListener(this);
> +            } else if (child instanceof Engine) {
> +                Engine engine = (Engine) child;
> +                engine.removeContainerListener(this);
> +            }
Why all this? Just use:
child.removeLifecycleListener(this);

Does the fact the Container implements Lifecycle simplify any of the
other code here?

> Modified: tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
> -    private void clearThreadLocalMap(Object map, Field internalTableField)
> +    private void checkThreadLocalMapForLeaks(Object map, Field internalTableField)
>              throws NoSuchMethodException, IllegalAccessException,
>              NoSuchFieldException, InvocationTargetException {
Not all of those Exceptions are required after this commit.

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