Fop should provide method to clean DecimalFormatCache (and other ThreadLocals, if any). Tomcat 7.0.11 complains about a memory leak: SEVERE: The web application [/(snip)] created a ThreadLocal with key of type [org.apache.fop.util.DecimalFormatCache.DecimalFormatThreadLocal] (value [org.apache.fop.util.DecimalFormatCache$DecimalFormatThreadLocal@fb637b]) and a value of type [java.text.DecimalFormat] (value [java.text.DecimalFormat@674dc]) 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. When FOP is being used with Tomcat, user should be able to add DecimalFormatCache.clean() in destroy method of ContextListener, and prevent memory leak.
I checked out the code from the FOP 1.0 tag and looked at the DecimalFormatCache. From what I can see this type of caching using ThreadLocal will not work well any environment where thread pools are used and where applications can be deployed and undeployed in runtime such as J2EE web applications. A workaround until a fix is out is to create a new thread for each PDF generation and the let the thread from the thread pool call the join() method on the new thread. This resolves the memory leak but on other hand makes the cache essentially pointless.
I've done some more research and from what I can see DecimalFormatCache is the only place in the FOP source where ThreadLocal is used. So there doesn't seem to be any more memory leaks in the FOP code related to ThreadLocal. Would a patch that which simply removes the DecimalFormatCache be accepted until a better solution comes around?
(In reply to comment #2) > I've done some more research and from what I can see DecimalFormatCache is the > only place in the FOP source where ThreadLocal is used. So there doesn't seem > to be any more memory leaks in the FOP code related to ThreadLocal. > > Would a patch that which simply removes the DecimalFormatCache be accepted > until a better solution comes around? No, unless it also solves the issue that prompted the use of ThreadLocal to begin with. The reason why this is so, is because DecimalFormat is not thread-safe. This could potentially lead to weird behavior in case of multiple concurrent runs in the same JVM. As for an alternative solution that would make sense: do something similar to what was done for color profiles, i.e. make sure all usage of DecimalFormat is properly synchronized.
I just changed bug name to more proper one. The source of the problem is this: 1. DecimalFormat is not thread safe 2. FOP uses DecimalFormat a lot 3. It takes time to instantiate DecimalFormat, so it is better to keep it for later reuse (= to cache it). If that is the case, I wonder, could we use some other caching mechanism instead of ThreadLocal? Maybe Apache JCS or Ehcache could work for us? They are both licensed under ASL2. If you agree, I could try to create drop-in replacement for DecimalFormatCache.
I wonder if this bug is not a duplicate of https://issues.apache.org/bugzilla/show_bug.cgi?id=43940 I'd rather not use a cache and possibly add another dependency just for that. I'd rather have a thread-safe decimal formatting method. Unfortunately, I haven't looked at the proposed solution in #43940, yet. Maybe you could take a look and give some feedback.
Ok, I will take a look if proposed pathces by Andreas and Julien for bug #43940 still work, and do they solve problems with Tomcat memory leak.
resetting P2 open bugs to P3 pending further review
please provide minimal input FO file that demonstrates problem; also, a patch that addresses the problem would be welcome
Created attachment 28561 [details] Example demonstrating the problem
I attached Maven WAR project as an example. Inside archive you will find: 1. fopleak.xml, FO file that is sufficient to trigger DecimalFormat caching 2. FopMemoryLeakServlet, with the code to transform FOP file. Steps to reproduce: 1. Package WAR file, e.g. using Apache Maven (mvn clean package) 2. Deploy resulting war file to latest stable Apache Tomcat (tested with 7.0.27) 3. Start tomcat 4. Stop tomcat 5. Check log files. It says: SEVERE: The web application [/fopleak] created a ThreadLocal with key of type [org.apache.fop.util.DecimalFormatCache.DecimalFormatThreadLocal] (value [org.apache.fop.util.DecimalFormatCache$DecimalFormatThreadLocal@1ee04fd]) and a value of type [java.text.DecimalFormat] (value [java.text.DecimalFormat@674dc]) 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.
Created attachment 28562 [details] Same example with simplified FO file.
(In reply to comment #6) > Ok, I will take a look if proposed pathces by Andreas and Julien for bug #43940 > still work, and do they solve problems with Tomcat memory leak. did you have a chance to look at the proposed patch in bug 43940? does it solve the tomcat problem? how about performance?
The content of attachment 28561 [details] has been deleted by Daniel Shahaf <danielsh@apache.org> who provided the following reason: Message-ID: <4F82CA3E.8080809@rcub.bg.ac.rs> The token used to delete this attachment was generated at 2012-04-09 20:39:01 IDT.
(In reply to comment #12) > (In reply to comment #6) > > Ok, I will take a look if proposed pathces by Andreas and Julien for bug #43940 > > still work, and do they solve problems with Tomcat memory leak. > > did you have a chance to look at the proposed patch in bug 43940? does it solve > the tomcat problem? how about performance? Ognjen, I am still awaiting your input as requested above. if I see no further input by April 30, I will close this bug due to lack of requested information. Regards, Glenn
I took a quick look at the patch in bug 43940 and from what I can see it replaces DecimalFormatCache with a new implementation that doesn't use ThreadLocal at all so this bug should be fixed if/when the patch bug 43940 is accepted and applied.
Patches for bug 43490 seems to be incomplete. I will try to add what is missing, and then test if memory leak is gone. -Ognjen
(In reply to comment #16) > Patches for bug 43490 seems to be incomplete. I will try to add what is > missing, and then test if memory leak is gone. Patch for 43940 is now complete, and it also resolves issue with memory leak. -Ognjen
Great, will there be a FOP 1.0.1 release containing this fix or do we have to wait until FOP 1.1 is released?
(In reply to comment #17) > (In reply to comment #16) > > Patches for bug 43490 seems to be incomplete. I will try to add what is > > missing, and then test if memory leak is gone. > > Patch for 43940 is now complete, and it also resolves issue with memory leak. > > -Ognjen ok, as soon as there is an ICLA on file, i'll apply it
(In reply to comment #18) > Great, will there be a FOP 1.0.1 release containing this fix or do we have > to wait until FOP 1.1 is released? no on a FOP 1.0.1; an FOP 1.1 is being prepared for release, hopefully during this quarter
See bug 43940, now resolved. Please verify and close if satisfied.
Test project attached to this issue now doesn't complain about the leak. I'm closing the bug. Thank you. -Ognjen