changcheng / wro4j

Automatically exported from code.google.com/p/wro4j
0 stars 0 forks source link

Use of InheritableThreadLocal in ro.isdc.wro.config.Context questionable #304

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Use WRO4J as-is
2. Run it in Tomcat 7
3. Stop Tomcat 7 gracefully
4. Watch Tomcat 7 complain about ThreadLocal leaks
Sep 27, 2011 1:43:56 PM org.apache.catalina.loader.WebappClassLoader 
checkThreadLocalMapForLeaks
SEVERE: The web application [] created a ThreadLocal with key of type 
[java.lang.InheritableThreadLocal] (value 
[java.lang.InheritableThreadLocal@5efb55]) and a value of type 
[ro.isdc.wro.config.Context] (value [ro.isdc.wro.config.Context@1c6a970[

What is the expected output? What do you see instead?
No leaks

What version of the product are you using? On what operating system?
1.4.1

Please provide any additional information below.
Somewhere along the way down from WroFilter.filter someone is creating a child 
thread, and that thread is inheriting the CURRENT thread local, but that thread 
is _not_ releasing the CURRENT thread local.  

Solution: change the referenced class to ThreadLocal.

Original issue reported on code.google.com by gacha...@gmail.com on 27 Sep 2011 at 4:46

GoogleCodeExporter commented 9 years ago
There is a reason why InheritableThreadLocal is used instead of ThreadLocal. I 
will try to reproduce the problem to see if the leak is real and if there is 
anything can be done with it. 

Thanks for reporting this issue.

Original comment by alex.obj...@gmail.com on 30 Sep 2011 at 8:45

GoogleCodeExporter commented 9 years ago
Hi Alex. It's only a leak if the thread holding the ITL isn't shutdown during a 
model reload or Servlet Context reload; otherwise its just an annoying error 
message from Tomcat 7.

Original comment by gacha...@gmail.com on 30 Sep 2011 at 9:42

GoogleCodeExporter commented 9 years ago
I am experiencing the same things. It seems like some Threads are not getting 
stopped either.

Original comment by talibsha...@gmail.com on 6 Oct 2011 at 8:34

GoogleCodeExporter commented 9 years ago

Original comment by alex.obj...@gmail.com on 24 Oct 2011 at 9:53

GoogleCodeExporter commented 9 years ago

Original comment by alex.obj...@gmail.com on 24 Oct 2011 at 9:54

GoogleCodeExporter commented 9 years ago
The memory leak warning doesn't appear anymore. It wasn't the 
InheritableThreadLocal causing the problem, rather there was a problem with the 
scheduler who didn't manage to stop started threads. I've refactored the code 
related to scheduling and it should be fixed now in branch 1.4.x

Original comment by alex.obj...@gmail.com on 25 Oct 2011 at 8:05

GoogleCodeExporter commented 9 years ago
Still able to reproduce it in 1.4.x... If I won't find a solution soon, I'll 
fix it in 1.4.x

Original comment by alex.obj...@gmail.com on 21 Nov 2011 at 8:56

GoogleCodeExporter commented 9 years ago

Original comment by alex.obj...@gmail.com on 7 May 2012 at 7:51

GoogleCodeExporter commented 9 years ago
The Context is hold in InheritableThreadLocal because the Context
should be accessible from within new threads created by executor (ex: when 
parallelPreprocessing is set to true).

Original comment by alex.obj...@gmail.com on 7 May 2012 at 8:16

GoogleCodeExporter commented 9 years ago

Original comment by alex.obj...@gmail.com on 7 May 2012 at 8:20

GoogleCodeExporter commented 9 years ago

Original comment by alex.obj...@gmail.com on 7 May 2012 at 8:59

GoogleCodeExporter commented 9 years ago
Fixed in branch 1.4.x. 
Now using an alternative approach, where each request generates a correlationId 
which is used as a key in the map storing the Context's. 

Original comment by alex.obj...@gmail.com on 8 May 2012 at 3:20