Stefan Larsson created HTTPCLIENT-1523:
------------------------------------------

             Summary: DateUtils ThreadLocal leak if referencing non-JDK classes
                 Key: HTTPCLIENT-1523
                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1523
             Project: HttpComponents HttpClient
          Issue Type: Bug
          Components: HttpClient
    Affects Versions: 4.3.3
         Environment: Java 7
            Reporter: Stefan Larsson


org.apache.http.client.utils.DateUtils use of a SoftReference is not enough to 
avoid a memory leak. 

Tomcat displays these kinds of warnings when undeploying a webapp:
{noformat}
Jun 26, 2014 3:04:27 PM org.apache.catalina.loader.WebappClassLoader 
checkThreadLocalMapForLeaks
SEVERE: The web application [/myapp] created a ThreadLocal with 
key of type [org.apache.http.client.utils.DateUtils$DateFormatHolder$1] (value 
[org.apache.http.client.utils.DateUtils$DateFormatHolder$1@22b76cd]) and a 
value of type [java.lang.ref.SoftReference] (value 
[java.lang.ref.SoftReference@5e63cdf6])
but failed to remove it when the web application was stopped. Threads are going 
to be renewed over time to try and avoid a probable memory leak. 
{noformat}

ThreadLocal is implemented through a reference from each Thread instance to the 
_value_ (a SoftReference, so OK after a GC takes place) but also to the 
ThreadLocal instance itself. In the case of DateUtils the JDK ThreadLocal is 
overridden in order to implement the initialValue() method, meaning that 
object's class has a reference to the classloader which loaded 
DateUtils.DateFormatHolder. When used in combination with a Tomcat webapp this 
eventually causes the permgen to fill up after a few redeployments.

(Thread.threadLocals is declared as a ThreadLocal.ThreadLocalMap, which is a 
collection of pairs of ThreadLocal instance plus its value)

Since there's already extensive code in DateUtils.DateFormatHolder.formatFor() 
to re-initialize the Map of date formats, one might as well perform the "new 
SoftReference(new HashMap()) there to avoid overriding the ThreadLocal.

References:
Second comment of https://plumbr.eu/blog/when-and-how-to-use-a-threadlocal

There will still be a tiny leak of ThreadLocal and SoftReference instances I 
believe with this change (max 1 per thread), but that should have much less 
impact than leaking a reference to a webapp's class loader.

ThreadLocals are supposed to be cleaned up either by letting the thread 
terminate (not an option if Tomcat is managing the thread) or by using 
ThreadLocal.remove() from the thread which assigned the ThreadLocal's value. I 
generally try to avoid using ThreadLocal whenever possible.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to