9

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?

6
  • 4
    I am not sure what your question is, but your getSingleton() method should be synchronized
    – amit
    Commented Oct 13, 2012 at 23:42
  • work() should not be static, otherwise the example doesn't make sense
    – CAFxX
    Commented Oct 13, 2012 at 23:48
  • I tried with synchronized but the two threads are still running concurrently
    – Luky
    Commented Oct 13, 2012 at 23:48
  • 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?
    – Luky
    Commented Oct 13, 2012 at 23:55
  • 1
    @Luky 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.
    – CAFxX
    Commented Oct 14, 2012 at 0:11

6 Answers 6

23

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.

3
  • Shouldn't we remove synchronized in this case? Commented Oct 16, 2012 at 22:55
  • @jcm oh yeah - forgot to remove it (I copy-pasted the original impl and modified it). Fixed now. Thx.
    – Bohemian
    Commented Oct 17, 2012 at 2:15
  • @Bohemian Is this still the fastest, safest and best way for Singleton? Commented Mar 10, 2021 at 2:50
5

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.

2
  • 1
    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.
    – carbolymer
    Commented May 22, 2017 at 10:12
  • @carbolymer 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. Commented May 24, 2017 at 1:39
1

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() { 
        // ...
    }
}
4
  • my only question is, work() called by one thread at a time as the Sigleton class has only one object created in memory. Right?
    – Atul
    Commented May 13, 2020 at 17:15
  • 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.
    – CAFxX
    Commented May 14, 2020 at 23:15
  • 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?
    – Atul
    Commented May 15, 2020 at 3:47
  • 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.
    – CAFxX
    Commented May 15, 2020 at 5:41
0

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{
}

}

0

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.

0

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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.