3

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.

xingbin
  • 27,410
  • 9
  • 53
  • 103
Batman
  • 49
  • 2
  • 1
    [Intuitively, this algorithm seems like an efficient solution to the problem. However, this technique has many subtle problems and should usually be avoided.](https://en.wikipedia.org/wiki/Double-checked_locking) It is exactly your case. Check that out. – Andrew Tobilko May 24 '18 at 13:53
  • Yes, but I am using a "temp" variable. Doesn't it solve the "partially-created-object" issue? And if using a "temp" variable doesn't solve it, then why does synchronizing the entire "getInstance()" method solve the partially-created-object issue? – Batman May 24 '18 at 13:55

2 Answers2

0

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.

Steve11235
  • 2,849
  • 1
  • 17
  • 18
  • I have checked by decompiling the class file. At least the compiler has not removed my "temp" variable. :-) – Batman May 24 '18 at 14:17
  • I hear you. However, the runtime can perform additional optimizations. What I'm hearing is that you can't predict what will happen. I still find it hard to believe that any optimization would assign a new instance before the constructor is done... – Steve11235 May 24 '18 at 14:47
  • Even if a new instance is assigned to a variable before the constructor is fully done, we can still be sure that when the execution moves to the next line, by that time, the object is fully created, right? And based on this, the "temp" variable fix should work.. right? – Batman May 24 '18 at 15:02
  • The "temp solution" is _not_ a solution. Even with the `temp` assignment, it _still_ is possible for some thread T to call `getInstance()` and see `singleton` not equal to `null`, but referencing an incompletely constructed object. The reason is, thread T did not use any form of synchronization. Without synchronization, there is no guarantee that thread T will _see_ variables updated in the same order that they actually _were_ updated by some other thread. – Solomon Slow May 24 '18 at 15:52
0

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.

Solomon Slow
  • 25,130
  • 5
  • 37
  • 57
  • thanks for the explanation. Do you think it would make any difference if all the instance variables of the singleton object are marked as 'final' and are initialized from within the constructor? Would such a code be thread-safe? Or would it still be thread unsafe? – Batman May 24 '18 at 18:44