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: [email protected]
For additional commands, e-mail: [email protected]
