views:

34

answers:

1

This is what my code currently looks like:

private boolean[] isInitialized = new boolean[COUNT];

private void ensureInitialized(int i) {
    if (! isInitialized[i]) {
        initialize(i);
        isInitialized[i] = true;
    }
}

Now I want to have it thread-safe. I know that double-checked-locking in Java is "teh 3vilness!!1", but since ensureInitialized may be called very often, I don't want it to be synchronized. So I am thinking of doing this:

private boolean[] isInitialized = new boolean[COUNT];

private void ensureInitialized(int i) {
    if (! isInitialized[i]) {
        synchronized (this) {
            if (! isInitialized[i]) {
                initialize(i);
                isInitialized[i] = true;
            }
        }
    }
}

Now what do I have to do to make this actually thread safe?
Some subquestions:

  • Making isInitialized volatile is not necessary, since the variable is not changed, right?
  • The array elements are changed, how can I make those volatile?
  • Are there generally better ways to do this?

(Also notice that this is an instance method, so static initializer will no work)

+1  A: 

Please note that java implementation of double-checking is called "broken pattern" because it was proven as fail (for example see http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html). To work around just use atomic operation. Follow is sample how to build thread safe singleton:

static AtomicReferenceArray<Boolean> instance = 
   new AtomicReferenceArray<Boolean>(COUNT);

private void ensure(int i)
{
    if(!instance.get(i) )
    {
        synchronized(this){
        if( !instance.get(i) ){
           initialize(i);
           instance.set(i, true);
        }
    }
}
Dewfy
Is there any evidence that java's synchronization is unnecessarily slow? In other words, yes, it is slow when there is contention, but when there is no contention it's fast.
GregS
+1, but an explanation is due. The double-checking lock is _not_ broken since Java 1.5 - using the `volatile` keyword makes it work fine. However, as the OP rightly assumed, the reads and writes to elements of the array are not volatile. What `AtomicXArray` classes do is do a volatile read for each element (See the source - `unsafe.getObjectVolatile()`).
Bozho
-1, Double check locking is not broken. The rest of the answer is technically correct. Maybe less preaching and more technical solutions?
Tim Bender
@Tim Bender - just go to link and/or read comment of @Bozho. Or at least try to google around phrase "java double checking" "broken pattern"
Dewfy
@GregS - it is rather funny, why we should use synchronization at all if no "contention"? When you sure that singleton used only by single thread lazy-load pattern is enough.
Dewfy
@Bozho - by the way assume following code **volatile boolean[] inst** this mean that volatile affects assigning of variable instance, but doesn't talks that access to inst[0] is also used volatile rules
Dewfy