Search code examples
javamultithreadingconcurrencysingletonsingleton-methods

concurrent calls of singleton class methods


I have a singleton class:

public class Singleton {
    private static Singleton istance = null;

    private Singleton() {}

    public synchronized static Singleton getSingleton() {
        if (istance == null)
            istance = new Singleton();
        return istance;
    }

    public void work(){
            for(int i=0; i<10000; i++){
                Log.d("-----------", ""+i);
            }
    }
}

And multiple Threads are calling the work() function:

public class Main {

public static void main(String[] args) {

    new Thread (new Runnable(){
        public void run(){
            Singleton s = Singleton.getSingleton();
            s.work();}
    }).start();

    System.out.println("main thread");

    new Thread(new Runnable() { 
         public void run() {
             Singleton s = Singleton.getSingleton();
                s.work();
         }
    }).start();
}
}

I noticed the two Threads are running concurrently, as if two work functions were instantiated at the same time.

I want the last thread to be run in place of the previous thread, rather then concurrently. Is it possible in java to make the second call override the memory space of the first call?


Solution

  • Your getSingleton() method is attempting to lazily initializing the SINGLETON instance, but it has the following problems:

    • Access to the variable is not synchronized
    • The variable is not volatile
    • You are not using double checked locking

    so a race condition AMY cause two instances to be created.

    The best and simplest was to safely lazily initialize a singleton without synchronization is as follows:

    private static class Holder {
        static Singleton instance = new Singleton();
    }
    
    public static Singleton getSingleton() { // Note: "synchronized" not needed
        return Holder.instance;
    }
    

    This is thread safe because the contract of the java class loader is that all classes have their static initialization complete before they may be used. Also, the class loader does not load a class until it is referenced. If two thread call getSingleton() simultaneously, the Holder class will still only get loaded once, and thus new Singleton() will only be executed once.

    This is still lazy because the Holder class is only referenced from getSingleton() method, so the Holder class will only be loaded when the first call to getSingleton() is made.

    Synchronization is not needed because this code relies on the class loader's internal synchronization, which is bullet proof.


    This code pattern is the only way to fly with singletons. It is:

    • The fastest (no synchronization)
    • The safest (relies on industrial strength class loader safety)
    • The cleanest (least code - double checked locking is ugly and a lot of lines for what it does)


    The other similar code pattern (equally safe and fast) is to use an enum with a single instance, but I find this to be clumsy and the intention is less clear.