Singleton design pattern broken?

Question

Possible Duplicate:
Efficient way to implement singleton pattern in Java

I would have thought that class below is a thread safe singleton but reading http://taskinoor.wordpress.com/2011/04/18/singleton_multithreaded/ it seems it is not.

public class ThreadSafeSingleton {

    private static ThreadSafeSingleton ref;

    private ThreadSafeSingleton(){

    }

    public ThreadSafeSingleton getSingletonObject(){

        if(ref == null){
            ref = new ThreadSafeSingleton();
        }   
        return ref;

    }
}

According to the article the only truly thread safe singleton is -

public class ThreadSafeSingleton {

    private static ThreadSafeSingleton ref = new ThreadSafeSingleton();

    private ThreadSafeSingleton(){

    }

    public ThreadSafeSingleton getSingletonObject(){
        return ref;

    }
}

Is this correct ?

Please check the "Related" list on the right hand column (which is by the way exactly the same list which appeared after you typed in the question's title). Surely this question has been asked countless times before :) — BalusC, Dec 01 '11 at 21:28
Definitely not the only threadsafe way, and not even the best threadsafe way. It is true that the top is not threadsafe while the bottom appears to be. — corsiKa, Dec 01 '11 at 21:36
My understanding is that the bottom solution is the "best" thread safe singleton implementation (although best can depend on a particular use case). Glowcoder, what way do you recommend as better? — Trevor Freeman, Dec 01 '11 at 21:50
@increment How do you define "best"? Different solutions have different trade offs. It's certainly not the best for expensive initialisation if you only need the object later on. — Voo, Dec 01 '11 at 21:52
@user470184, this is not the ONLY way to implement a thread safe singleton. That's not said in the blog. There are a number of platform specific solutions which are shortly described at the end of the blog post. In particular, if you are using J2SE 5.0 or higher then you can use `volatile` with DCLP and thus can have lazy initialization too. — taskinoor, Dec 02 '11 at 05:22

Dmitry B. · Answer 1 · 2011-12-01T21:38:01.890

4

That's not the ONLY thread-safe singleton, but that's correct. Another way is to synchronize the code that creates the singleton instance in the first example. But the problem with this is that you have to synchronize the code, which may or may not be a problem. another possible issue is that the singleton is not lazily initialized. Again, it may or may not be a problem depending on the architecture and requirements. There is another weird pattern that deals with that issue: http://en.wikipedia.org/wiki/Initialization_on_demand_holder_idiom.

edited Dec 01 '11 at 21:38

answered Dec 01 '11 at 21:28

Dmitry B.

9,107
3
43
64

AKA, the Bill Pugh solution for the Singleton pattern : http://en.wikipedia.org/wiki/Singleton_pattern#The_solution_of_Bill_Pugh – eSniff Dec 01 '11 at 21:43

score 2 · Answer 2 · answered Dec 01 '11 at 21:29

Yes, the article is correct.

In the top example, if the method gets called by two functions at the same time, they may both see ref as null because neither has actually finished creating and assigning it before the other checks it.

In the bottom example, ref is allocated once when the class is loaded, before anything can access it.

score 1 · Answer 3 · answered Dec 01 '11 at 21:29

Yes, it is correct.

In the first case, if two threads call the getSingletonObject() at the same time, there is a risk of ending up with two instances of the singleton.

In the second case, the method just returns a reference to an existing object that is created during the class loadup, which is done by the JVM in a thread-safe fashion.

The second is much better than the first one in terms of thread-safety.

score 0 · Answer 4 · answered Dec 01 '11 at 21:28

if(ref == null)
{
    ref = new ThreadSafeSingleton();
}            

return ref;

Two threads operating at roughly the same time could hit if (ref == null). Two threads could see that ref is, in fact, null. Two threads would then create a new instance. Two threads could then be given separate instances. And now you have two different instances of your "singleton" object.

There's no mechanism in place in this snippet to prevent the race condition described above.

score 0 · Answer 5 · answered Dec 01 '11 at 21:45

There are a couple of different ways to implement a threadsafe singleton object. Static initialization (the bottom method) is one of those ways.

You can also use a static inner class, dependency injection, synchronize getInstance(), or even a double-checked locking pattern in Java 5+ so long as the holder variable is declared volatile. See Java Concurrency in Practice for more information.

Of those, if I have to build an actual singleton, I prefer the first method. Otherwise I like to use scopes in Guice.

Singleton design pattern broken?

5 Answers5