Bugs item #1070309, was opened at 2004-11-20 23:38 Message generated for change (Comment added) made by ddlucas You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=116035&aid=1070309&group_id=16035
Category: None Group: None Status: Open Resolution: None Priority: 5 Submitted By: Guillaume Poirier (gpoirier) Assigned to: David Lucas (ddlucas) Summary: ThreadLocal cache Initial Comment: There's a problem with the use of ThreadLocals for caching, it's never cleaned up. If the ClassLoader is thrown away (e.g. if a webapp is reloaded), then the ThreadLocal stuff hold until the Thread dies, which might take a while in a Servlet container where the Threads are pooled. In such situation, DOM4J would prevent the webapp's ClassLoader to be garbage collected, which would creates a a memory leak and eventually an OOM if the webapp is reloaded too many times. To fix the problem, the use of ThreadLocal could be avoided completly by using thread-safe singleton instead. There's three places it's used. There's the getInstance() method from DOMDocumentFactory and DocumentFactory. In that situation, it doesn't seem an appropriate use to me. First, the DOM spec doesn't mandate for that implementation to be thread safe, and anyway returning a different instance per thread is no help there, each instance can still be used in different threads. And you would usually expect a getInstance() method to always return the same object. Also, DOMDocumentFactory already seems perfectly thread safe. The last use of ThreadLocal I identified is in QName, basically for ThreadLocal caching, which doesn't really seem needed, QNameCache seems thread-safe as far as I can see, unless the content of the HashMaps are not meant to be shared by differents calls to DocumentFactory? But anyway, I don't really thing it would too hard to get rid of the ThreadLocal there. http://article.gmane.org/gmane.comp.java.springframework.devel/6216 ---------------------------------------------------------------------- >Comment By: David Lucas (ddlucas) Date: 2004-11-21 19:56 Message: Logged In: YES user_id=633013 One of the issues we ran into in the past was not thread-safety but "thread hot" protection. The problem is when two seperate threads modify data and the state changes during an operation. It is like a race condition, but the issues can be very dangerous between threads. That was one of the drivers to have a seperate cache per thread. It was not the best solution at the time, but was a solution that resolved the issue without over complicating what already existed. I will look into cleaning up the cache and removing ThreadLocal / Singleton caches where it makes sense. Thanks for bringing this issue back into the light. We are hoping to address this before the next release. ---------------------------------------------------------------------- Comment By: Guillaume Poirier (gpoirier) Date: 2004-11-21 11:20 Message: Logged In: YES user_id=942521 The thing is, that code would be thread safe even without the use of ThreadLocal. It doesn't provice any benifit, the only place that it might allow to avoid synchronization is in QNameCache for the HashMap, but they ARE synchronized anyway. And in that case, if you were to worry about the synchronized HashMap performance, then you can use Doug Lea's ConcurrentHashMap. ---------------------------------------------------------------------- Comment By: David Lucas (ddlucas) Date: 2004-11-21 03:32 Message: Logged In: YES user_id=633013 This is a limitation of using ThreadLocal for a quick cache. However, DOM4J must maintain its thread safety. There are several users that have demonstrated that need in the past and it would not be wise to remove the implied requirement since it currently is thread-safe. The problem is a contextual one. A more elaborate caching algorithm will be required to avoid the potential memory leak. Something in the neighborhood of clearing the cache of things not recently used or even clear it periodically based on time since last cache cleared. A ThreadLocal cache does provide some benefits by avoiding synchronization between multiple threads. The fix for this bug should maintain equal performance or better while maintaining thread safety. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=116035&aid=1070309&group_id=16035 ------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://productguide.itmanagersjournal.com/ _______________________________________________ dom4j-dev mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dom4j-dev
