In org.apache.batik.util.CleanerThread we see: public static ReferenceQueue getReferenceQueue() { if ( queue == null ) { synchronized (CleanerThread.class) { queue = new ReferenceQueue(); thread = new CleanerThread(); } } return queue; } This method is not thread safe due to a race condition. The test for "if (queue == null)" needs to be inside the synchronized block, not outside of it. As written, it's possible for the initialization code to be executed more than once. Fix: make the entire method synchronized, or add an additional test for "if (queue == null)" within the synchronized block (but see http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html for why the variable needs to remain volatile).
Synchronizing on the getReferenceQueue() method is IMHO correct. Also changing line 104 from: ref = queue.remove(); to: ref = getReferenceQueue().remove(); Solves NullPointerExceptions being thrown in eclipse & tomcat after reloading the context. Because this error is thrown in a while(true) { //clean references } loop, it overflows the log files. Please fix.
Increasing importance given the potential race condition. The fix seems fairly easy, should I try to create a patch based on the suggestions from the previous comments? (I just didn't yet because I'm not that confident on my Java foo on this particular scope...)
We ran into the NullPointer issue on our live server this weekend, where batik wrote some 100GB of exceptions into our log until the disk was full and we had a downtime of half an hour to figure this out. Running a SaaS product, this is what we try to avoid... Since it seems quite easy to fix this, is there any chance of a next release including this bug fix? Or would you recommend to build batik on my own to include the bug fix? Is there any documentation on how to do this? Thanks