1

I have seen some similar questions,but I still have some confusions.
code is here:

private volatile static DoubleCheckSingleton instance;

private DoubleCheckSingleton() {}
public static DoubleCheckSingleton getInstance(){

    if(instance==null){ //first

        synchronized (DoubleCheckSingleton.class){

            if(instance==null){  // second
                instance=new DoubleCheckSingleton();
            }

        }

    }

    return instance;

}

In this question Why is volatile used in double checked locking, it says that without a volatile keyword, a thread may assign the instance variable before the constructor finishes, so another thread may see a half-constructed object, which can cause serious problem.

But I don't understand how volatile can solve the problem. Volatile is used to ensure visibility, so when thread A assign a half-constructed object to instance variable, the other thread can immediately see the change, which makes the situation worse.

How does volatile solve the problem, somebody please explain to me. Thanks!

RamPrakash
  • 1,687
  • 3
  • 20
  • 25
haoyu wang
  • 1,241
  • 4
  • 17
  • "A write to a `volatile` field (§8.3.1.4) _happens-before_ every subsequent read of that field" – [§17.4.5 of the _Java Language Specification_](https://docs.oracle.com/javase/specs/jls/se13/html/jls-17.html#jls-17.4.5). – Slaw Feb 03 '20 at 18:47
  • Without the use of volatile, your `//first` test may not see changes to the variable made by other threads. See [JLS §17.3](https://docs.oracle.com/javase/specs/jls/se13/html/jls-17.html#jls-17.3) for an example. – VGR Feb 03 '20 at 18:52
  • For a singleton, this pattern is unnecessarily complicated. See https://stackoverflow.com/a/3578704/3474 – erickson Feb 04 '20 at 17:58

3 Answers3

4

a thread may assign the instance variable before the constuctor finishes

That's not actually true. The assignment is right there in the code example:

instance=new DoubleCheckSingleton()

Obviously, the thread that performs that assignment can not possibly do the assignment before the constructor call has returned.

The problem is, when two different threads are running on two different processors without any synchronization, they will not necessarily agree upon the order in which assignments happen. So, even though thread A assigned the fields of the new object (inside the new DoubleCheckSingleton() call) before it assigned instance, thread B potentially could see those assignments out-of-order. Thread B could see the assignment to instance before it sees some of the other things that new DobuleCheckSingleton() did.

Declaring instance to be volatile synchronizes the threads. volatile guarantees that everything thread A did before it assigned a volatile variable will become visible to thread B when thread B fetches the value of the volatile variable.

Solomon Slow
  • 25,130
  • 5
  • 37
  • 57
  • Is it possible that the the compiler somehow reorder the program, so it first allocate the memory space and assign it to instance variable and then call constructor? – haoyu wang Feb 04 '20 at 15:37
  • 1
    @haoyuwang, If the compiler decided to in-line the constructor call, then yes, I suppose that could happen. But think of it this way: There's what the `javac` compiler can do, there's what the JIT compiler (if any) can do, and there's what the hardware can do. But whatever any of those things does to your program, the end result must behave in the way that the [Java Language Specification](https://docs.oracle.com/javase/specs/) (JLS) promises it will behave. – Solomon Slow Feb 04 '20 at 16:23
1

Double checked locking spent a good bit of time as an anti-pattern, according to Java Concurrency In Practice et. al. At the time they suggested using something like the Lazy Holder pattern instead.

https://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom

Brian Goetz wrote a good article on how DCL was broken here https://www.javaworld.com/article/2074979/double-checked-locking--clever--but-broken.html

As StevenC pointed out to me, since the update to the Java Memory Model it will work properly, although in spite of this I think the Lazy Holder is still a nice alternative. It is clean to implement, and avoids the need of the volatile. Improper implementation of DCL will cause hard to find bugs.

James Gawron
  • 871
  • 10
  • 27
  • 1
    That Goetz article is out of date (2001). In Java 5, the memory model was revised. The DCL is no longer broken, provided that you 1) user Java 5+, and 2) declare `instance` as `volatile`. – Stephen C Feb 03 '20 at 22:54
  • 1
    Fair enough ... Lazy Holder still works well ... and avoids questions like "why does this need to be volatile" ... or worse, incorrect implemenation without volatile which causes difficult to locate bugs ... – James Gawron Feb 03 '20 at 22:56
-2

To put it another way, suppose we have two perfectly-racing threads:

  • THREAD 1: "if instance != null" ... okay, it's null.
  • THREAD 2: "if instance != null" ... okay, it's null.
  • THREAD 1: creates a new "instance"
  • THREAD 2: STOMPS ON the value just created by Thread 1.

... and, quite likely, neither thread is presently aware that something is wrong. It's likely that both of them think that they have a DoubleCheckSingleton and are using them just as they should be ... neither of them realizing that there are two. (So, sit back and watch data-structures getting trashed left and right as your app nose-dives into the ground.)

Even worse, this is exactly the kind of bug that "happens once-in-a-blue-moon." Which is to say that it never happens on any developer machine, but it happens five minutes after you deploy to production ... ;-)

The volatile keyword basically says that "this storage location, itself is a shared resource." Which it is.

Now also: the best practice, I think, is to initialize all of your synchronization objects before you start to launch threads or subprocesses. All of these entities can now refer to these now already-existing objects and can use them, but none of these players ever create them.

Mike Robinson
  • 8,490
  • 5
  • 28
  • 41
  • where do you see that "if instance != null" ??? Also, where in your description is the effect of the synchronization lock in between the two "if instance == null" that there are ??? – Erwin Smout Feb 03 '20 at 20:11
  • Please read: https://www.satisfice.com/blog/archives/5164. And note that what you are suggesting (non-lazy initialization) is not a workable option in many use-cases. So calling it "best practice" is clearly wrong .... even if you are using the term appropriately. (Feel free to share that link with your colleagues. Get the message out there!) – Stephen C Feb 03 '20 at 23:16
  • I don't think that's the problem. the second ```if(instance==null)``` is used to prevent the situation you say from happening – haoyu wang Feb 04 '20 at 15:46