You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2010/03/29 17:01:15 UTC
svn commit: r928798 - in /tomcat/trunk: java/org/apache/catalina/core/
java/org/apache/catalina/loader/ webapps/docs/config/
Author: markt
Date: Mon Mar 29 15:01:14 2010
New Revision: 928798
URL: http://svn.apache.org/viewvc?rev=928798&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48895
Make clearing thread locals optional and disabled by default since it isn't thread-safe
Modified:
tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
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/webapps/docs/config/context.xml
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=928798&r1=928797&r2=928798&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardContext.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardContext.java Mon Mar 29 15:01:14 2010
@@ -772,7 +772,16 @@ public class StandardContext
* <code>false</code> will be used.
*/
private boolean clearReferencesStopThreads = 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 the effective web.xml be logged when the context starts?
*/
@@ -2288,6 +2297,34 @@ public class StandardContext
/**
+ * Return the clearReferencesThreadLocals flag for this Context.
+ */
+ public boolean getClearReferencesThreadLocals() {
+
+ return (this.clearReferencesThreadLocals);
+
+ }
+
+
+ /**
+ * Set the clearReferencesStopThreads feature for this Context.
+ *
+ * @param clearReferencesStopThreads The new flag value
+ */
+ public void setClearReferencesThreadLocals(
+ boolean clearReferencesThreadLocals) {
+
+ boolean oldClearReferencesThreadLocals =
+ this.clearReferencesThreadLocals;
+ this.clearReferencesThreadLocals = clearReferencesThreadLocals;
+ support.firePropertyChange("clearReferencesStopThreads",
+ oldClearReferencesThreadLocals,
+ this.clearReferencesThreadLocals);
+
+ }
+
+
+ /**
* Has this context been initialized?
*/
public boolean isInitialized() {
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=928798&r1=928797&r2=928798&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/LocalStrings.properties Mon Mar 29 15:01:14 2010
@@ -38,8 +38,10 @@ 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. However, to simplify the process of tracing memory leaks, the key has been forcibly removed.
-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. To prevent a memory leak, the ThreadLocal has been forcibly removed.
+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.stopThreadFail=Failed to terminate thread named [{0}] for web application [{1}]
webappClassLoader.stopTimerThreadFail=Failed to terminate TimerThread named [{0}] for web application [{1}]
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=928798&r1=928797&r2=928798&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java Mon Mar 29 15:01:14 2010
@@ -446,6 +446,15 @@ public class WebappClassLoader
private boolean clearReferencesStopThreads = 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
@@ -676,6 +685,27 @@ 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;
+ }
+
+
/**
* Set the clearReferencesLogFactoryRelease feature for this Context.
*
@@ -2254,16 +2284,26 @@ public class WebappClassLoader
log.debug(sm.getString(
"webappClassLoader.clearThreadLocalDebug",
args));
+ if (clearReferencesThreadLocals) {
+ log.debug(sm.getString(
+ "webappClassLoader.clearThreadLocalDebugClear"));
+ }
}
} else {
log.error(sm.getString(
"webappClassLoader.clearThreadLocal",
args));
+ if (clearReferencesThreadLocals) {
+ log.info(sm.getString(
+ "webappClassLoader.clearThreadLocalClear"));
+ }
}
- if (key == null) {
- staleEntriesCount++;
- } else {
- mapRemove.invoke(map, key);
+ if (clearReferencesThreadLocals) {
+ if (key == null) {
+ staleEntriesCount++;
+ } else {
+ mapRemove.invoke(map, key);
+ }
}
}
}
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=928798&r1=928797&r2=928798&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/WebappLoader.java Mon Mar 29 15:01:14 2010
@@ -596,6 +596,8 @@ public class WebappLoader extends Lifecy
((StandardContext) container).getClearReferencesStatic());
classLoader.setClearReferencesStopThreads(
((StandardContext) container).getClearReferencesStopThreads());
+ classLoader.setClearReferencesThreadLocals(
+ ((StandardContext) container).getClearReferencesThreadLocals());
}
for (int i = 0; i < repositories.length; i++) {
Modified: tomcat/trunk/webapps/docs/config/context.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/context.xml?rev=928798&r1=928797&r2=928798&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/config/context.xml (original)
+++ tomcat/trunk/webapps/docs/config/context.xml Mon Mar 29 15:01:14 2010
@@ -380,6 +380,15 @@
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r928798 - in /tomcat/trunk: java/org/apache/catalina/core/
java/org/apache/catalina/loader/ webapps/docs/config/
Posted by sebb <se...@gmail.com>.
On 30/03/2010, Konstantin Kolinko <kn...@gmail.com> wrote:
> 2010/3/30 sebb <se...@gmail.com>:
>
> > On 29/03/2010, markt@apache.org <ma...@apache.org> wrote:
> >> Author: markt
> >> Date: Mon Mar 29 15:01:14 2010
> >> New Revision: 928798
> >>
> >> URL: http://svn.apache.org/viewvc?rev=928798&view=rev
> >> Log:
> >> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48895
> >> Make clearing thread locals optional and disabled by default since it isn't thread-safe
> >
> > However, the new boolean flag (clearReferencesThreadLocals) that
> > controls the process is not safely published between threads. It
> > should probably be volatile.
> >
>
>
> Startup is performed by a single thread, and it will set the value of
> this field.
OK, in that case not a problem.
However, it seems to me that this assumption should be documented in
the StandardContext class.
I think I saw a recent Bugzilla which suggested using multiple threads
to speed startup, in which case the assumption might become incorrect.
> According to the memory model all the threads started after that will
> see the assigned value.
Agreed; it's only a problem if two active threads access the same
mutable variable.
> I do not remember any of the "option value" fields in Tomcat being
> marked as volatile. All of them are usual fields.
>
> Best regards,
>
> Konstantin Kolinko
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r928798 - in /tomcat/trunk: java/org/apache/catalina/core/
java/org/apache/catalina/loader/ webapps/docs/config/
Posted by Konstantin Kolinko <kn...@gmail.com>.
2010/3/30 sebb <se...@gmail.com>:
> On 29/03/2010, markt@apache.org <ma...@apache.org> wrote:
>> Author: markt
>> Date: Mon Mar 29 15:01:14 2010
>> New Revision: 928798
>>
>> URL: http://svn.apache.org/viewvc?rev=928798&view=rev
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48895
>> Make clearing thread locals optional and disabled by default since it isn't thread-safe
>
> However, the new boolean flag (clearReferencesThreadLocals) that
> controls the process is not safely published between threads. It
> should probably be volatile.
>
Startup is performed by a single thread, and it will set the value of
this field.
According to the memory model all the threads started after that will
see the assigned value.
I do not remember any of the "option value" fields in Tomcat being
marked as volatile. All of them are usual fields.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r928798 - in /tomcat/trunk: java/org/apache/catalina/core/
java/org/apache/catalina/loader/ webapps/docs/config/
Posted by sebb <se...@gmail.com>.
On 29/03/2010, markt@apache.org <ma...@apache.org> wrote:
> Author: markt
> Date: Mon Mar 29 15:01:14 2010
> New Revision: 928798
>
> URL: http://svn.apache.org/viewvc?rev=928798&view=rev
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48895
> Make clearing thread locals optional and disabled by default since it isn't thread-safe
However, the new boolean flag (clearReferencesThreadLocals) that
controls the process is not safely published between threads. It
should probably be volatile.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org