tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slaur...@apache.org
Subject svn commit: r1042786 - in /tomcat/trunk/java/org/apache: catalina/core/ catalina/loader/ tomcat/util/threads/
Date Mon, 06 Dec 2010 20:49:15 GMT
Author: slaurent
Date: Mon Dec  6 20:49:14 2010
New Revision: 1042786

URL: http://svn.apache.org/viewvc?rev=1042786&view=rev
Log:
bug 49159: Improve ThreadLocal memory leak clean-up 
https://issues.apache.org/bugzilla/show_bug.cgi?id=49159

Various fixes after review by markt : formatting, use string manager, svn props...

Modified:
    tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties
    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/ThreadLocalLeakPreventionListener.java   (contents,
props changed)
    tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
    tomcat/trunk/java/org/apache/tomcat/util/threads/Constants.java   (contents, props changed)
    tomcat/trunk/java/org/apache/tomcat/util/threads/LocalStrings.properties   (props changed)
    tomcat/trunk/java/org/apache/tomcat/util/threads/TaskQueue.java
    tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThread.java   (contents, props changed)
    tomcat/trunk/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java

Modified: tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties?rev=1042786&r1=1042785&r2=1042786&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties Mon Dec  6 20:49:14
2010
@@ -238,6 +238,8 @@ standardWrapper.unavailable=Marking serv
 standardWrapper.unloadException=Servlet {0} threw unload() exception
 standardWrapper.unloading=Cannot allocate servlet {0} because it is being unloaded
 standardWrapper.waiting=Waiting for {0} instance(s) to be deallocated
+threadLocalLeakPreventionListener.lifecycleEvent.error=Exception processing lifecycle event
{0}
+threadLocalLeakPreventionListener.containerEvent.error=Exception processing container event
{0}
 
 defaultInstanceManager.restrictedServletsResource=Restricted servlets property file not found
 defaultInstanceManager.privilegedServlet=Servlet of class {0} is privileged and cannot be
loaded by this web application

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=1042786&r1=1042785&r2=1042786&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardContext.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardContext.java Mon Dec  6 20:49:14 2010
@@ -2489,9 +2489,10 @@ public class StandardContext extends Con
         return this.renewThreadsWhenStoppingContext;
     }
 
-    public void setRenewThreadsWhenStoppingContext(boolean renewThreadsWhenStoppingContext)
{
+    public void setRenewThreadsWhenStoppingContext(
+            boolean renewThreadsWhenStoppingContext) {
         boolean oldRenewThreadsWhenStoppingContext =
-            this.renewThreadsWhenStoppingContext;
+                this.renewThreadsWhenStoppingContext;
         this.renewThreadsWhenStoppingContext = renewThreadsWhenStoppingContext;
         support.firePropertyChange("renewThreadsWhenStoppingContext",
                 oldRenewThreadsWhenStoppingContext,

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=1042786&r1=1042785&r2=1042786&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardThreadExecutor.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardThreadExecutor.java Mon Dec  6 20:49:14
2010
@@ -88,7 +88,8 @@ public class StandardThreadExecutor exte
      * renewing all threads at the same time, this delay is observed between 2
      * threads being renewed.
      */
-    protected long threadRenewalDelay = 1000L;
+    protected long threadRenewalDelay = 
+        org.apache.tomcat.util.threads.Constants.DEFAULT_THREAD_RENEWAL_DELAY;
     
     private TaskQueue taskqueue = null;
     // ---------------------------------------------- Constructors

Modified: 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=1042786&r1=1042785&r2=1042786&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java Mon
Dec  6 20:49:14 2010
@@ -34,25 +34,34 @@ import org.apache.catalina.connector.Con
 import org.apache.coyote.ProtocolHandler;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
 import org.apache.tomcat.util.threads.ThreadPoolExecutor;
 
 /**
+ * <p>
  * 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/>
+ * memory leaks.
+ * </p>
+ * <p>
  * 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/>
+ * {@link org.apache.tomcat.util.threads.ThreadPoolExecutor}.afterExecute().
+ * </p>
  * 
  * 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);
+    ContainerListener {
+    private static final Log log =
+        LogFactory.getLog(ThreadLocalLeakPreventionListener.class);
+
+    /**
+     * The string manager for this package.
+     */
+    protected static final StringManager sm =
+        StringManager.getManager(Constants.Package);
 
     /**
      * Listens for {@link LifecycleEvent} for the start of the {@link Server} to
@@ -63,7 +72,7 @@ public class ThreadLocalLeakPreventionLi
         try {
             Lifecycle lifecycle = event.getLifecycle();
             if (Lifecycle.AFTER_START_EVENT.equals(event.getType())
-                    && lifecycle instanceof Server) {
+                && 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
@@ -73,11 +82,15 @@ public class ThreadLocalLeakPreventionLi
             }
 
             if (Lifecycle.AFTER_STOP_EVENT.equals(event.getType())
-                    && lifecycle instanceof Context) {
+                && lifecycle instanceof Context) {
                 stopIdleThreads((Context) lifecycle);
             }
         } catch (Exception e) {
-            log.error("Exception processing event " + event, e);
+            String msg =
+                sm.getString(
+                    "threadLocalLeakPreventionListener.lifecycleEvent.error",
+                    event);
+            log.error(msg, e);
         }
     }
 
@@ -87,13 +100,17 @@ public class ThreadLocalLeakPreventionLi
             String type = event.getType();
             if (Container.ADD_CHILD_EVENT.equals(type)) {
                 processContainerAddChild(event.getContainer(),
-                        (Container) event.getData());
+                    (Container) event.getData());
             } else if (Container.REMOVE_CHILD_EVENT.equals(type)) {
                 processContainerRemoveChild(event.getContainer(),
-                        (Container) event.getData());
+                    (Container) event.getData());
             }
         } catch (Exception e) {
-            log.error("Exception processing event " + event, e);
+            String msg =
+                sm.getString(
+                    "threadLocalLeakPreventionListener.containerEvent.error",
+                    event);
+            log.error(msg, e);
         }
 
     }
@@ -129,43 +146,35 @@ public class ThreadLocalLeakPreventionLi
     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);
+        if (child instanceof Context) {
+            registerContextListener((Context) child);
+        } else if (child instanceof Engine) {
+            registerListenersForEngine((Engine) child);
+        } else if (child instanceof Host) {
+            registerListenersForHost((Host) child);
         }
 
     }
 
-    protected void processContainerRemoveChild(Container parent, Container child) {
+    protected void processContainerRemoveChild(Container parent, 
+        Container child) {
 
         if (log.isDebugEnabled())
             log.debug("Process removeChild[parent=" + parent + ",child="
-                    + 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);
+        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);
         }
-
     }
 
     /**
@@ -177,9 +186,8 @@ public class ThreadLocalLeakPreventionLi
      *            of its parent Service.
      */
     private void stopIdleThreads(Context context) {
-        if (context instanceof StandardContext
-                && !((StandardContext) context)
-                        .getRenewThreadsWhenStoppingContext()) {
+        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;
         }
@@ -196,10 +204,12 @@ public class ThreadLocalLeakPreventionLi
                 }
 
                 if (executor instanceof ThreadPoolExecutor) {
-                    ThreadPoolExecutor threadPoolExecutor = (ThreadPoolExecutor) executor;
+                    ThreadPoolExecutor threadPoolExecutor =
+                        (ThreadPoolExecutor) executor;
                     threadPoolExecutor.contextStopping();
                 } else if (executor instanceof StandardThreadExecutor) {
-                    StandardThreadExecutor stdThreadExecutor = (StandardThreadExecutor) executor;
+                    StandardThreadExecutor stdThreadExecutor =
+                        (StandardThreadExecutor) executor;
                     stdThreadExecutor.contextStopping();
                 }
 

Propchange: tomcat/trunk/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: tomcat/trunk/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision

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=1042786&r1=1042785&r2=1042786&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java Mon Dec  6 20:49:14
2010
@@ -2365,24 +2365,18 @@ public class WebappClassLoader
         } catch (IllegalAccessException e) {
             log.warn(sm.getString("webappClassLoader.checkThreadLocalsForLeaksFail",
                     contextName), e);
-        } catch (NoSuchMethodException e) {
-            log.warn(sm.getString("webappClassLoader.checkThreadLocalsForLeaksFail",
-                    contextName), e);
-        } catch (InvocationTargetException e) {
-            log.warn(sm.getString("webappClassLoader.checkThreadLocalsForLeaksFail",
-                    contextName), e);
         }       
     }
 
 
-    /*
+    /**
      * 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 checkThreadLocalMapForLeaks(Object map, Field internalTableField)
-            throws NoSuchMethodException, IllegalAccessException,
-            NoSuchFieldException, InvocationTargetException {
+    private void checkThreadLocalMapForLeaks(Object map,
+            Field internalTableField) throws IllegalAccessException,
+            NoSuchFieldException {
         if (map != null) {
             Object[] table = (Object[]) internalTableField.get(map);
             if (table != null) {

Modified: 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=1042786&r1=1042785&r2=1042786&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/threads/Constants.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/threads/Constants.java Mon Dec  6 20:49:14 2010
@@ -23,4 +23,5 @@ public final class Constants {
 
     public static final String Package = "org.apache.tomcat.util.threads";
 
+    public static final long DEFAULT_THREAD_RENEWAL_DELAY = 1000L;
 }

Propchange: tomcat/trunk/java/org/apache/tomcat/util/threads/Constants.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: tomcat/trunk/java/org/apache/tomcat/util/threads/Constants.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision

Propchange: tomcat/trunk/java/org/apache/tomcat/util/threads/LocalStrings.properties
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: tomcat/trunk/java/org/apache/tomcat/util/threads/LocalStrings.properties
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision

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=1042786&r1=1042785&r2=1042786&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/threads/TaskQueue.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/threads/TaskQueue.java Mon Dec  6 20:49:14 2010
@@ -81,7 +81,8 @@ public class TaskQueue extends LinkedBlo
 
 
     @Override
-    public Runnable poll(long timeout, TimeUnit unit) throws InterruptedException {
+    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
@@ -90,21 +91,22 @@ public class TaskQueue extends LinkedBlo
         }
         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 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) {
+        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

Modified: 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=1042786&r1=1042785&r2=1042786&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThread.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThread.java Mon Dec  6 20:49:14 2010
@@ -19,8 +19,6 @@ 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 {
 

Propchange: tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThread.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: tomcat/trunk/java/org/apache/tomcat/util/threads/TaskThread.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision

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=1042786&r1=1042785&r2=1042786&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java Mon Dec  6 20:49:14
2010
@@ -65,7 +65,7 @@ public class ThreadPoolExecutor extends 
     /**
      * Delay in ms between 2 threads being renewed. If negative, do not renew threads.
      */
-    private long threadRenewalDelay = 1000L;
+    private long threadRenewalDelay = Constants.DEFAULT_THREAD_RENEWAL_DELAY;
 
     public ThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime,
TimeUnit unit, BlockingQueue<Runnable> workQueue, RejectedExecutionHandler handler)
{
         super(corePoolSize, maximumPoolSize, keepAliveTime, unit, workQueue, handler);
@@ -133,9 +133,11 @@ public class ThreadPoolExecutor extends 
     }
     
     protected boolean currentThreadShouldBeStopped() {
-        if (threadRenewalDelay >= 0 && Thread.currentThread() instanceof TaskThread)
{
+        if (threadRenewalDelay >= 0
+            && Thread.currentThread() instanceof TaskThread) {
             TaskThread currentTaskThread = (TaskThread) Thread.currentThread();
-            if (currentTaskThread.getCreationTime() < this.lastContextStoppedTime.longValue())
{
+            if (currentTaskThread.getCreationTime() < 
+                    this.lastContextStoppedTime.longValue()) {
                 return true;
             }
         }
@@ -160,7 +162,8 @@ public class ThreadPoolExecutor extends 
      * thread, at the discretion of the <tt>Executor</tt> implementation.
      * If no threads are available, it will be added to the work queue.
      * If the work queue is full, the system will wait for the specified 
-     * time and it throw a RejectedExecutionException if the queue is still full after that.
+     * time and it throw a RejectedExecutionException if the queue is still 
+     * full after that.
      *
      * @param command the runnable task
      * @throws RejectedExecutionException if this task cannot be
@@ -197,7 +200,8 @@ public class ThreadPoolExecutor extends 
 
         // save the current pool parameters to restore them later
         int savedCorePoolSize = this.getCorePoolSize();
-        TaskQueue taskQueue = getQueue() instanceof TaskQueue ? (TaskQueue) getQueue() :
null;
+        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
@@ -210,16 +214,18 @@ public class ThreadPoolExecutor extends 
         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)
+        // 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
+            // yes, ignore
         }
-        
+
         if (taskQueue != null) {
             // ok, restore the state of the queue and pool
             taskQueue.setForcedRemainingCapacity(null);



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


Mime
View raw message