java:singleton, static variable and thread safety

class MyClass
{
private static MyClass obj;

public static MyClass getInstance()
{
    if(obj==null)
    {
        obj = new MyClas开发者_开发知识库s();
    }
    return obj;
}

In the above java code sample, because obj is a static variable inside the class, will getInstance still be non-thread safe? Because static variables are shared by all threads, 2 simultaneous threads shall be using the same object. Isnt it?

Vipul Shah

Because static variables are so widely shared they are extremely un-thread safe.

Consider what happens if two threads call your getInstance at the same time. Both threads will be looking at the shared static obj and both threads will see that obj is null in the if check. Both threads will then create a new obj.

You may think: "hey, it is thread safe since obj will only ever have one value, even if it is initialized multiple times." There are several problems with that statement. In our previous example, the callers of getInstance will both get their own obj back. If both callers keep their references to obj then you will have multiple instances of your singleton being used.

Even if the callers in our previous example just did: MyClass.getInstance(); and didn't save a reference to what MyClass.getInstance(); returned, you can still end up getting different instances back from getInstance on those threads. You can even get into the condition where new instances of obj are created even when the calls to getInstance do not happen concurrently!

I know my last claim seems counter-intuitive since the last assignment to obj would seem to be the only value that could be returned from future calls to MyClass.getInstance(). You need to remember, however, that each thread in the JVM has its own local cache of main memory. If two threads call getInstance, their local caches could have different values assigned to obj and future calls to getInstance from those threads will return what is in their caches.

The simplest way to make sure that getInstance thread safe would be to make the method synchronized. This will ensure that

1. Two threads can not enter getInstance at the same time
2. Threads trying to use obj will never get a stale value of obj from their cache

Don't try to get clever and use double checked locking: http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

Good explanation can be found here:

http://en.wikipedia.org/wiki/Singleton_pattern

The wiki article highlights various thread-safe approaches along with some of their pros and cons.

in this case getInstance() is not thread-safe, even if you use static variable. only synchronization makes this thread-safe.

The following example shows a weird thread save modified single ton pattern which supports generics as well.

To have it just thread save and synchronization save just take the synchronized block and the transient and volatile keywords. Notice, that there is a double check, the synchronized block is inside an if. This brings more performance, because synchronized is expensive.

Of course for a real singleton do not use maps, I said it is a modified one.

public class Edge<T> {
    @SuppressWarnings({"unchecked"})
    private static transient volatile HashMap<Object,HashMap<Object, Edge>> instances = new HashMap<Object, HashMap<Object,Edge>>();
    /**
     * This function is used to get an Edge instance
     * @param <T> Datatype of the nodes.
     * @param node1, the source node
     * @param node2, the destination node
     * @return the edge of the two nodes.
     */
    @SuppressWarnings({"unchecked"})
    public static <T> Edge<T> getInstance(T node1, T node2){
        if(!(instances.containsKey(node1) && instances.get(node1).containsKey(node2))){
            synchronized (Edge.class) {
                if(!(instances.containsKey(node1) && instances.get(node1).containsKey(node2))){
                    Edge<T> edge = new Edge<T>(node1, node2);
                    if(!instances.containsKey(node1)){
                        instances.put(node1, new HashMap<Object, Edge>());
                    }
                    instances.get(node1).put(node2, edge);
                }
            }
        }
        return (Edge<T>)instances.get(node1).get(node2);
    }

public class Singleton{
    private static transient volatile Singleton instance;
    public static Singleton getInstance(){
        if(instance==null)synchronized(Singleton.class){
            if(instance==null){
                instance = new Singleton();
            }
        }
        return instance;
    }
    private Singleton(){
        /*....*/
    }
}

Page 182: http://books.google.com/books?id=GGpXN9SMELMC&printsec=frontcover&dq=design+patterns&hl=de&ei=EFGCTbyaIozKswbHyaiCAw&sa=X&oi=book_result&ct=result&resnum=2&ved=0CDMQ6AEwAQ#v=onepage&q&f=false

Think this can be tagged as answered now.

class MyClass
{
private static MyClass obj;

private MyClass(){
    // your initialization code
}
public static synchronized MyClass getInstance()
{
    if(obj==null)
    {
        obj = new MyClass();
    }
    return obj;
}

I'll agree with @Manoj. I believe the above will be one of the best methods to achieve singleton object. And synchronization makes the object thread safe. Even, it's static :)