Bug 44178 - Race condition in CleanerThread.java getReferenceQueue() method
Summary: Race condition in CleanerThread.java getReferenceQueue() method
Status: NEW
Alias: None
Product: Batik - Now in Jira
Classification: Unclassified
Component: Utilities (show other bugs)
Version: 1.8
Hardware: All All
: P2 major
Target Milestone: ---
Assignee: Batik Developer's Mailing list
Depends on:
Reported: 2008-01-07 09:33 UTC by Archie Cobbs
Modified: 2011-07-04 09:48 UTC (History)
0 users


Note You need to log in before you can comment on or make changes to this bug.
Description Archie Cobbs 2008-01-07 09:33:12 UTC
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).
Comment 1 Eddie Moojen 2009-08-03 23:53:25 UTC
Synchronizing on the getReferenceQueue() method is IMHO correct.

Also changing line 104 from:
                    ref = queue.remove();

                    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.
Comment 2 Helder Magalhães 2009-08-04 03:20:33 UTC
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...)
Comment 3 Volker Gersabeck 2011-07-04 09:48:47 UTC
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?