Details
-
Type:
Bug
-
Status:
Resolved
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 1.3
-
Fix Version/s: 3.0
-
Component/s: PicoContainer (Java)
-
Labels:None
-
Environment:Linux (Fedora Core 12)
-
Number of attachments :
Description
Deploy the attached sample application onto Apache Tomcat v6 (included are a pre-built WAR exported from eclipse, a context file for tomcat, and the eclipse project containing the source code).
Now re-deploy by touching the war file.
Observe the following line in the catalina.out log file:
14-Jul-2010 10:52:02 org.apache.catalina.loader.WebappClassLoader clearThreadLocalMap
SEVERE: A web application created a ThreadLocal with key of type [org.picocontainer.defaults.ConstructorInjectionComponentAdapter$1] (value [org.picocontainer.defaults.ConstructorInjectionComponentAdapter$1@f91da9]) and a value of type [java.lang.Boolean] (value [false]) but failed to remove it when the web application was stopped. To prevent a memory leak, the ThreadLocal has been forcibly removed.
Is there some destroy/dispose/shutdown call on picocontainer that I am omitting, or is this a bug?
The following thread discusses the problem with Tomcat and ThreadLocals and makes a few suggestions for how to avoid it:
http://echo.nextapp.com/site/node/6254
I've marked this bug as minor since leaking Booleans isn't a serious memory leak, so it's really just the log spam that's upsetting.
-
-
-
Hide
-
-
picoThreadLocalBug/built-threadlocal-demo-app.war
-
picoThreadLocalBug/eclipseProject/.../A.java
-
picoThreadLocalBug/eclipseProject/.../B.java
-
picoThreadLocalBug/.../TestStartupListener.java
-
picoThreadLocalBug/.../MANIFEST.MF
-
picoThreadLocalBug/.../web.xml
-
picoThreadLocalBug/.../picocontainer-1.3.jar
-
picoThreadLocalBug/.../B.class
-
picoThreadLocalBug/.../TestStartupListener.class
-
picoThreadLocalBug/.../A.class
-
picoThreadLocalBug/.../.project
-
picoThreadLocalBug/.../.classpath
-
picoThreadLocalBug/.../org.eclipse.wst.common.project.facet.core.xml
-
picoThreadLocalBug/.../org.eclipse.wst.jsdt.ui.superType.name
-
picoThreadLocalBug/.../.jsdtscope
-
picoThreadLocalBug/.../org.eclipse.jdt.core.prefs
-
picoThreadLocalBug/.../org.eclipse.wst.jsdt.ui.superType.container
-
picoThreadLocalBug/.../org.eclipse.wst.common.component
-
tomcat-context-file.xml
-
-
Activity
A trivial patch to reduce noise of Tomcat shutting down. Making it possible to find real leaks again 
Mark J. Sinke
added a comment - A trivial patch to reduce noise of Tomcat shutting down. Making it possible to find real leaks again A second source of log pollution is the use of the IntoThreadLocal thread-local. However, since that stores a Class object, it may be a real source of memory leaks. My suggestion is to change DefaultPicoContainer.java as follows
{{{
330c330
<
>
624,628d623
< synchronized (this) {
< if (intoThreadLocal == null)
< }
630c625,629
< return getComponent(componentKeyOrType, (Class<? extends Annotation>) null);
> try
finally
{ > intoThreadLocal.set(null); > }687,691d685
< synchronized (this) {
< if (intoThreadLocal == null)
< }
}}}
Note that I also removed the lazy initialization of the IntoThreadLocal field, since it is private and initialized on construction (maybe it should be final as well to remove thread-visibility issues).
I added a patch file as well with the same content
Mark J. Sinke
added a comment - A second source of log pollution is the use of the IntoThreadLocal thread-local. However, since that stores a Class object, it may be a real source of memory leaks. My suggestion is to change DefaultPicoContainer.java as follows
{{{
330c330
<
>
624,628d623
< synchronized (this) {
< if (intoThreadLocal == null)
{
< intoThreadLocal = new IntoThreadLocal();
< }
< }
630c625,629
< return getComponent(componentKeyOrType, (Class<? extends Annotation>) null);
> try
{
> return getComponent(componentKeyOrType, (Class<? extends Annotation>) null);
> }
finally
{
> intoThreadLocal.set(null);
> }
687,691d685
< synchronized (this) {
< if (intoThreadLocal == null)
{
< intoThreadLocal = new IntoThreadLocal();
< }
< }
}}}
Note that I also removed the lazy initialization of the IntoThreadLocal field, since it is private and initialized on construction (maybe it should be final as well to remove thread-visibility issues).
I added a patch file as well with the same content The DefaultPicoContainer patch which solves a potential real memory leak.
Mark J. Sinke
added a comment - The DefaultPicoContainer patch which solves a potential real memory leak. Commit 5735 has applied Mark's patches ... though with some rework.
Paul Hammant
added a comment - Commit 5735 has applied Mark's patches ... though with some rework. 
This is really an extremely easy one to fix. What needs to happen is that the Boolean.FALSE which is set in the thread local for instantiation or verification guards should be set to null instead. There is no change to the logic of the guard at all.
The patch to AbstractInjector.java (of the 2.11 version) is attached as well as inline below:
{{{
{ < return Boolean.FALSE; < }245,249d244
< @Override
< protected Boolean initialValue()
<
279c274
< set(Boolean.FALSE);
> set(null);
}}}