Following is my singleton class where I am using double-checked-locking without using volatile keyword and without synchronizing the entire getInstance() method:

public class MySingleton {

    private static MySingleton mySingleton;

    public static MySingleton getInstance() {
        if(mySingleton == null) {
            synchronized(MySingleton.class) {
                if(mySingleton == null) {
                    MySingleton temp = new MySingleton();
                    mySingleton = temp;
                }
            }
        }

        return mySingleton;
    }
}

According to me, this is thread-safe. If anyone thinks, this is not thread-safe, can someone please elaborate on why he/she thinks this is not thread-safe? Thanks.

2

There are 2 answers

1
Solomon Slow On

Yes, but I am using a "temp" variable. Doesn't it solve the "partially-created-object" issue?

No. It does not.

Suppose some thread A calls getInstance(), and ends up creating a new instance and assigning the mySingleton variable. Then thread T comes along, calls getInstance() and sees that mySingleton is not null.

At this point, thread T has not used any synchronization. Without synchronization, the Java Language Specification (JLS) does not require that thread T see the assignments made by thread A in the same order that thread A made them.

Let's suppose that the singleton object has some member variables. Thread A obviously must have initialized those variables before it stored the reference into mySingleton. But the JLS allows thread T to see mySingleton != null and yet still see the member variables in their uninitialized state. On some multi-core platforms, it can actually happen that way.

Assigning the object reference to a local temp variable first doesn't change anything. In fact, as Steve11235 pointed out, the temp variable might not even actually exist in the byte codes or in the native instructions because either the Java compiler or the hot-spot compiler could completely optimize it away.

4
Steve11235 On

I wasn't aware of this issue until I read all the comments. The problem is that the various optimization processes (compiler, Hot Spot, whatever) rewrite the code. Your "temp" solution could easily be removed. I find it hard to believe that a constructor could return a partial object, but if knowledgeable contributors are saying so, I'd trust their opinion.