15

This question relates to behaviour of old Java versions and old implementations of the double checked locking algorithm

Newer implementations use volatile and rely on slightly changed volatile semantics, so they are not broken.


It's stated that fields assignment is always atomic except for fields of long or double.

But, when I read an explaination of why double-check locking is broken, it's said that the problem is in assignment operation:

// Broken multithreaded version
// "Double-Checked Locking" idiom
class Foo {
    private Helper helper = null;
    public Helper getHelper() {
        if (helper == null) {
            synchronized(this) {
                if (helper == null) {
                    helper = new Helper();
                }
            }
        }
        return helper;
    }

    // other functions and members...
}
  1. Thread A notices that the value is not initialized, so it obtains the lock and begins to initialize the value.
  2. Due to the semantics of some programming languages, the code generated by the compiler is allowed to update the shared variable to point to a partially constructed object before A has finished performing the initialization.
  3. Thread B notices that the shared variable has been initialized (or so it appears), and returns its value. Because thread B believes the value is already initialized, it does not acquire the lock. If B uses the object before all of the initialization done by A is seen by B (either because A has not finished initializing it or because some of the initialized values in the object have not yet percolated to the memory B uses (cache coherence)), the program will likely crash.
    (from http://en.wikipedia.org/wiki/Double-checked_locking).

When is it possible? Is it possible that on 64-bit JVM assignment operation isn't atomic? If no then whether "double-checked locking" is really broken?

Raedwald
  • 46,613
  • 43
  • 151
  • 237
Roman
  • 64,384
  • 92
  • 238
  • 332
  • possible duplicate of http://stackoverflow.com/questions/12448864/java-double-locking-can-someone-explain-more-simply-why-intuition-wouldnt-wor/12449110?noredirect=1#comment26446551_12449110 – Alexei Kaigorodov Aug 06 '13 at 04:41

7 Answers7

16

The problem is not atomicity, it's ordering. The JVM is allowed to reorder instructions in order to improve performance, as long as happens-before is not violated. Therefore, the runtime could theoretically schedule the instruction that updates helper before all instructions from the constructor of class Helper have executed.

minopret
  • 4,726
  • 21
  • 34
meriton
  • 68,356
  • 14
  • 108
  • 175
7

The assignment of the reference is atomic, but the construction is not! So as stated in the explanation, supposing thread B wants to use the singleton before Thread A has fully constructed it, it cannot create a new instance because the reference is not null, so it just returns the partially constructed object.

If you do not ensure that publishing the shared reference happens before another thread loads that shared reference, then the write of the reference to the new object can be reordered with the writes to its fields. In that case, another thread could see an up-to-date value for the object reference but out of date values for some or all of the object's state - a partially constructed object. -- Brian Goetz: Java Concurrency in Practice

Since the initial check for null is not synchronized there is no publication and this reordering is possible.

Robert
  • 8,406
  • 9
  • 38
  • 57
  • is it stated anywhere that java does the assignment first and only then calculates the right part? (It's kind of the `Future` pattern, but I've never seen official statements about its usage by java compiler). – Roman Feb 07 '11 at 21:31
  • @Roman: It's not a simple matter of "Java assigns first and only then calculates the right part". That would be an incorrect statement I think. Java allows the low level code to reorder statements in a way that can cause unexpected results when a shared field that isn't `volatile` or `final` is accessed by another thread. – ColinD Feb 07 '11 at 21:40
2

Several assignments may be needed to construct the instance of Helper inside the constructor, and the semantics allows that they are reordered with respect to the assignment helper = new Helper().

So the field helper may be assigned a reference to an object where not all assignments have taken place, so that it is incompletely initialized.

starblue
  • 55,348
  • 14
  • 97
  • 151
  • what do you mean "several assignments"? I see only 1: `helper = new Helper()`. If it can be constructed "in place" then could you give a link which proves that java realy does so weird things? Thx. – Roman Feb 07 '11 at 21:27
  • 1
    @Roman: As mentioned in the quotes in your question, the code generated by the compiler is allowed to assign `Helper` to an instance of `Helper` that _has not completed construction_. – ColinD Feb 07 '11 at 21:33
2

Double checked locking in java has a variety of problems:

http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

Stephen Denne
  • 36,219
  • 10
  • 45
  • 60
  • 5
    For Java 5+ it works as expected with no problems if you declare the field being initialized as `volatile` (as mentioned in that paper, of course). – ColinD Feb 07 '11 at 21:31
1

Read this article: http://www.javaworld.com/jw-02-2001/jw-0209-double.html Even if you did not understand all details (like me) just believe that this nice trick does not work.

AlexR
  • 114,158
  • 16
  • 130
  • 208
0

I'm sorry this might be a bit irrelevant to the question, I'm just curious. In this case wouldn't it better to acquire the lock before the assignment and/or returning the value? Like:

private Lock mLock = new ReentrantLock();
private Helper mHelper = null;

private Helper getHelper() {
    mLock.lock();
    try {
        if (mHelper == null) {
            mHelper = new Helper();
        }
        return mHelper;
    }
    finally {
        mLock.unlock();
    }
}

Or is there any advantage of using the double-checked locking?

Surya Wijaya Madjid
  • 1,823
  • 1
  • 19
  • 25
-1
/*Then the following should work.
  Remember: getHelper() is usually called many times, it is BAD 
  to call synchronized() every time for such a trivial thing!
*/
class Foo {

private Helper helper = null;
private Boolean isHelperInstantiated;
public Helper getHelper() {
    if (!isHelperInstantiated) {
        synchronized(this) {
            if (helper == null) {
                helper = new Helper();
                isHelperInstantiated = true;
            }
        }
    }
    return helper;
}

// other functions and members...
}    
jack
  • 731
  • 7
  • 8