Why is synchronizing a method not advisable?

Question

Consider the below code snippet:
public class DbSingleton {

private static volatile DbSingleton instance = null;

private DbSingleton() {
        if (instance != null) {
            throw new RuntimeException("Use getInstance()");
        }
    }

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

Why is the above preferred to declaring getInstance() as synchronized? What does it mean when a method is declared as synchronized? Will it synchronize the entire class and slow down the performance?

Lew Bloch · Answer

Double-checked locking is evil and unnecessary, as is the Singleton antipattern. Also, initializing the static variable to null is a bad idea. Just initialize the variable as a final variable, or use an enum.
private static final DbSingleton instance = new DbSingleton();
…
public static DbSingleton getInstance() {
    return instance;
}

The instance is already lazily initialized when the class is, so any additional effort to accomplish that is stupid.

Rubydesic · Answer

Acquiring a lock is expensive. In the given code, a lock is only acquired if instance == null.
Consider the following

Thread A calls getInstance()
Thread B calls getInstance()
Thread A acquires the lock
Thread B waits on the lock
Thread A checks that instance == null initializes it, and releases the lock
Thread B acquires the lock
Thread B checks that instance != null and returns instance.

This scenario will happen with either a synchronized method or double checked locking. However, after the instance has been instantiated, consider the difference.
(This is the same as public static synchronized DbSingleton getInstance())
public static DbSingleton getInstance() {
    synchronized (DbSingleton.class) {
        if (instance == null) {
            instance = new DbSingleton();
        }
        return instance;
    }
}

With this method, after instance has been initialized..

Thread A calls getInstance, acquires the lock, returns instance, and releases the lock
Thread B calls getInstance, acquires the lock, returns instance, and releases the lock
etc.

However, acquiring the lock is redundant in this case, which is the purpose of this code:
public static DbSingleton getInstance() {
    if (instance == null) {
        synchronized (DbSingleton.class) {
            if (instance == null) {
                instance = new DbSingleton();
            }
        }
    }
    return instance;
}

If instance does not equal null, a lock is never acquired and the flow looks like

Thread A calls getInstance and returns instance (no lock)
Thread B calls getInstance and returns instance (no lock)

Burak Serdar · Answer

If you declare getInstance as synchronized, you will pay for the synchronization penalty every time you call getInstance even after it is initialized. With this implementation, getInstance will never go into a synchronized block after its first call because the instance is initialized already.

Why is synchronizing a method not advisable?

3 Answers

Add your own answers!

Ask a Question