Synchronized access on a singleton

Question

I have a question about the synchronization on a singleton. Assume I have a singleton pattern like this:

public class Singleton {

    private final Object obj;

    private static Singleton instance;

    private Singleton(Object obj) {
        this.obj = obj;
    }

    public static synchronized Singleton getInstance(Object obj) {
        if (instance == null) {
            instance = new Singleton(obj);
        }
        return instance;
    }

    public synchronized void methodA() {
        // ...
    }

    public synchronized void methodB() {
        // ...
    }
}

And I want to synchronize the access on it. I have two assumptions, that I need to verify.

1. Assumption: Every access on this Singleton is thread safe and synchronized, due to all methods being synchronized including the initializer.

2. Assumption: When I want to make sure that a thread that wants to call methodA() and then immediately methodB() without another thread calling methods of the singleton, is it correct to synchronize on the instance like this?

Singleton singleton = Singleton.getInstance(obj);

synchronized (singleton) {
    singleton.methodA();
    singleton.methodB();
}

Explanation:

Is the 1. assumption correct because the synchronized on a not static method synchronizes on the object itself and since it is always the same object, the access is synchronized? And the call to getInstance(obj) synchronizes on the class itself?

Is the 2. assumption correct because with the getInstance(obj) every thread gets the same object and thus the synchronization is correct since another thread will wait till the synchronized block (...methodA(); methodB();) is exited?

Answer 1

Your assumptions are correct, assuming I understand them correctly. I just want to point out that in most cases you can use a much simpler singleton pattern:

public class Singleton {

    private static Singleton instance = new Singleton();

    private Singleton() {
    }

    public static Singleton getInstance() {
        return instance;
    }
}

The field will be initialized on the first static reference (e.g. getInstance()) in a thread-safe way without the need for explicit synchronization.

Also, instance should be final.

Answer 2

Your first assumption is correct, however, it's a good practice to synchronize over a dedicated lock object instead of using synchronized methods. Doing so keeps the concurrency issues out of the API and allows you to replace the lock against a more efficient implementation from the java.util.concurrent package later. You also prevent deadlocks because other objects cannot acquire the same lock object.

public class Example {

  // leaving out the singleton aspect here

  // consider java.util.concurrent.locks instead
  private Object lock = new Object();

  public synchronized void methodA() {
    synchronized(lock) {
      // ...
    }
  }

  public synchronized void methodB() {
    synchronized(lock) {
      // ...
    }
  }
}

Clients can still synchronize on the instance:

synchronized(instance) {
  instance.methodA();
  instance.methodB();
}

For an example, have a looks at the implementation of SynchronizedCollection in the java.util.Collections class, which also use an internal lock object.

One concern about synchronizing on a singleton: If you have a lot of concurrent threads in your application (for example, in a web application), this central lock can quickly become a performance bottleneck as all threads have to wait for each other.

Answer 3

Both are true.

The best way to test it (it's always good to test a solution, rather than depend on assumptions) would be to wait in between calling methods for some time, and call methodA from different thread, then check the execution order. For example:

public synchronized void methodA() {
    println("methodA");
}

public synchronized void methodB() {
    println("methodB");
}

Then, in a test case or in the main method:

new Thread(){
    public void run(){
           Singleton singleton = Singleton.getInstance();

           synchronized (singleton) {
           singleton.methodA();
           singleton.wait(5000L);//this gives us 5 seconds to call it from different thread
           singleton.methodB();
       } 
    }
}.start();

new Thread(){
    public void run(){
           Singleton.getInstance().methodA();
       } 
    }
}.start();

Answer 4

As you know synchronized keyword is used to acquire the lock on the object. If you want to call your willing method after execution of one method. Then we can call the method from synchronized method like below code but Thread will not release until it comes out of the first synchronized block.

   public synchronized void method1A() {
       // ......
    method1B();
   }

   public synchronized void method1B() {
       // ...
   }

Here Thread will acquire lock entered into synchronized method1A at last it is going to call another synchronized method1B. Thread is going to release the lock after completion of method1A execution.

Answer 5

You both assumptions are true. However for creating Singleton I will suggest below approach which is 100% thread safe and no synchronization issue because it leverages Java's class loading mechanism.

Like this class Provider will be loaded only once and hence only one instance of Network class will exist, and no two threads can create 2 instances in same JVM because class will not be loaded twice.

public class Network {
    private Network(){

    }

    private static class Provider {
        static final Network INSTANCE = new Network();
    }

    public static Network getInstance() {
        return Provider.INSTANCE;
    }
    //More code...
}

Answer 6

Thread Safe singleton sample;

public class Singleton {

        private static Singleton instance;

        private Singleton(){

        }

        public static Singleton getInstance(){
            synchronized (Singleton.class){
                if(instance == null){
                    instance = new Singleton();
                }
            }
            return instance;
        }
    }