concurrent calls of singleton class methods

Question

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?

Nostology

I am not sure what your question is, but your getSingleton() method should be synchronized – Geometric

work() should not be static, otherwise the example doesn't make sense – Unconcerned

I tried with synchronized but the two threads are still running concurrently – Nostology

removing static the two work() functions runs in sequence, one at a time. Apparently the second call is queued after the first call. How may the second call just run in place of the first? – Nostology

@Nostology synchronized won't stop two threads from running concurrently: it will only serialize their access to the getSingleton() function. Put otherwise, it means that you can be sure that no matter how many threads are currently in execution, only one of them at most will be executing getSingleton() at any point in time. – Unconcerned

Besides, the compiler should complain if you do s.work() because you're invoking a static method using an instanced class: at the very least you should be calling Singleton.work(). – Unconcerned

Answer 1

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.

Meissen

Shouldn't we remove synchronized in this case? – Alethiaaletta

@jcm oh yeah - forgot to remove it (I copy-pasted the original impl and modified it). Fixed now. Thx. – Meissen

@Meissen Is this still the fastest, safest and best way for Singleton? – Embodiment

Answer 2

As @amit stated in a comment your getSingleton() method should be synchronized. The reason for this is that it is possible for multiple threads to ask for an instance at the same time and the first thread will still be initializing the object and the reference will be null when the next thread checks. This will result in two instances being created.

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

Marking your method as synchronized will cause it to block and only allow one thread at a time to call it. This should solve your problem.

Alethiaaletta

Synchronizing every call to the getSingleton() is not efficient. Only first few calls to getSingleton() should be synchronized to ensure that the instance was initialized and published properly. – Dansby

@Dansby This question is 5 years old and was rather vague. Efficiency was never mentioned. If it is merely to lazily instantiate a singleton then the holder pattern mentioned in the top answer is the correct way to to do it bypassing synchronization entirely. – Alethiaaletta

Answer 3

Either use synchronized on the factory method

public class Singleton {
    private static Singleton istance = null;

    private final Singleton() {} // avoid overrides

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

    public void work() { // not static, otherwise there's no need for the singleton
        // ...
    }
}

or, simply, use a private final initializer (instantiation will happen at class-load time)

public class Singleton {
    private static final Singleton istance = new Singleton(); // class-load initialization

    private final Singleton() {} 

    public static Singleton getSingleton() { // no need for synchronized
        return istance;
    }

    public void work() { 
        // ...
    }
}

Unconcerned

my only question is, work() called by one thread at a time as the Sigleton class has only one object created in memory. Right? – Nightcap

No, you can call work() (or getSingleton) concurrently from multiple threads. If you do you obviously need to make sure that work() is safe to be called concurrently. – Unconcerned

Ok but as class in singleton in by nature, only one thread can have lock of the object at a time. So the method work() used by one thread at a time. Right? – Nightcap

Singleton means that only a single instance of the class exists, not that it can't be used concurrently by multiple threads. So, as I wrote above, there is nothing preventing work() from being called concurrently. They are completely orthogonal concepts. – Unconcerned

Answer 4

Resource holder given in Java Concurrency In Practice:http://www.javaconcurrencyinpractice.com/ is the best non-blocking singleton pattern available. The singleton is lazily initialized (both SingletonHolder and Singleton class is loaded at Run-Time when the getInstance() method is called the first time) and the access-or method is non-blocking.

public class SingletonFactory {

private static class SingletonHolder {
    static Singleton instance = new Singleton();
}

public static Singleton getInstance() {
    return SingletonFactory.SingletonHolder.instance;
}

static class Singleton{
}

}

Immesh

Answer 5

I came up with this code that is doing pretty much what I needed. The orignal question was "Is possible to do the following without using threads? But rather by directly manipulating the memory with the language?" If the answer is no, maybe you can help me improve the following:

public class Main {
private static Thread t;
public static void main(String[] args) {
    work();
    for (int i =0;i<100; i++);
    System.out.println("oooooooooooooooooooooooooooooooooooooooooo");
    for (int i =0;i<100; i++);
    work();
    for (int i =0;i<500; i++);
    System.out.println("oooooooooooooooooooooooooooooooooooooooooo");
}

public static void work(){
    if (t != null) t.interrupt();
    t= new Thread (new Runnable(){
            public void run(){
                // Pause for 4 seconds
                try {
                    Thread.sleep(600);
                } catch (InterruptedException e) {
                    // We've been interrupted: no more messages.
                    return;
                }
                for(int i=0; i<10000; i++){
                    System.out.println(i);
                }
            }
            });
    t.start();
}
}

This code is useful to "debounce" multiple calls to a listener, firing in a burst on user inputs. It has the disadvantages it uses a sleep function. The sleep time should be high enough to prevent events in a burst to start execution of the time consuming task (only the last event should). Unfortunately there is no guarantee that this can always happen even for a large sleep time.

Nostology

Answer 6

You can use Locks around the shared resources. Use the Reentrant class. It prevents race conditions for multiple threads.

Molest