Created attachment 25020 [details] Exposes an exit method in CleanerThread org.apache.batik.util.CleanerThread does not self-terminate, nor is there a way to dispose of it programatically. When using Batik within a webapp, this prevents the webapp's classloader from being garbage collected, eventually causing PermGen: Out of Memory errors after several webapp redeploys. CleanerThread should expose some method that allows for it to be terminated. Patch attached which follows the same solution used in geotools package [https://fisheye.codehaus.org/browse/GEOT-2742]
Thanks for taking the time to investigate/share the patch, Joel! :-) Without testing it, I think it would be interesting to mention that there was some discussion about this in the mailing list prior to this bug report being created. I've added a link to in as the issue's URL. ;-) I'm also adding metadata to reflect that a patch was made available. Should/can this be related to bug 44178? At least, one may be interested in reviewing both issues together... :-) A (somehow) quick review of the patch, mostly with cosmetic nits, follows. > + public static final CleanerThread THREAD = new CleanerThread(); Double space before "THREAD". > + queue = new ReferenceQueue<Object>(); We're not using generics yet, as Batik still targets Java 1.4 compatibility AFAIK (see bug 46434). Could this be reworked? > + ReferenceQueue<Object> rq; Tab character here. Indentation is made using 4 spaces within the framework. > + } catch (InterruptedException e) { > + > + } I'd say that either insert a comment, stating why the exception is ignored (if it matters), or get rid of the extraneous line.
Created attachment 25025 [details] Patch updated to conform to style guidelines
I would claim that the proposed patch also solves the race condition described in 44178
(In reply to comment #2) > Created an attachment (id=25025) [details] > Patch updated to conform to style guidelines I've taken a peek and it looks good, thanks. :-) (In reply to comment #3) > I would claim that the proposed patch also solves the race condition described > in 44178 Great! ;-) This appears to be quite safe and potentially fixes a couple issues. Still, I'd ask for a third opinion/deeper review by a more elder committer (Thomas? Cameron? Dieter?)
Hi everybody, Any chance that the proposed (or a similar) solution would become part of an official release any time soon? We use Batik quite a bit in a web app to export SVG and derived formats and are currently trying to get rid of a lot of memory leaks, so that we don't have to restart Tomcat regularly... Cheers,