Search code examples
javamultithreadingsingletonatomicnonblocking

Singleton using Atomic Non Blocking Method vs Synchornized


I have tried to utilized Non Blocking Atomic Boolean API to generate singleton object instead of synchronized.

I have 2 implementations

  1. Via Double Locking and Synchronized Keyword
  2. Via Atomic Non Blocking Call

I believe we can have robust and better implementation via Atomic than synchronized. Please suggest if i am incorrect. Also performs some basic test for performance which favors Atomic implementation over synchronized.

Supports

// Lazy Initialization
// Thread-safe
// Performance improvement for corresponding calls, once the object is created

public class Singleton {
    private static Singleton instance;

    static final AtomicBoolean isInitialized = new AtomicBoolean(false);

    private Singleton() {
        // private constructor //Thread.sleep(10)

    }

    // 1st Implementation
    // Via Double Locking and Synchronized
    public static Singleton getInstance() {
        if (instance == null) {
            // synchronized block to remove overhead
            synchronized (Singleton.class) {
                if (instance == null) {
                    // if instance is null, initialize
                    instance = new Singleton();
                }
            }
        }
        return instance;
    }

    // 2nd Implementation + Not ThreadSafe with Null Objects.
    // Via Atomic Non Blocking Method and avoiding Synchronized
    public static Singleton getInstanceViaAtomic() {
        if (instance != null)
            return instance;

        if (isInitialized.compareAndSet(false, true)) {
            return instance = new Singleton();
        }
        return instance;
    }

}

Update @Kayaman correctly identified the above impl was not threadsafe. I fixed in the below one.

// 2nd Implementation + Thread Safe
// Via Atomic Non Blocking Method and avoiding Synchronized
public static Singleton getInstanceViaAtomic() throws InterruptedException {
    while(instance == null) {
        if (isInitialized.compareAndSet(false, true)) {
            instance = new Singleton();
        }
    }
    return instance;

}

Solution

  • The 2nd implementation isn't thread-safe. An easy way to show it is to put a Thread.sleep(10); (or more) in the constructor of Singleton.

    This causes the first thread to set isInitialized to true (while instance is still null), then call new Singleton();.

    Another thread will see instance as null, it won't enter the if block because the boolean is now true, then it will return instance which is null.

    So a race condition ending with a NullPointerException.

    If we were to force this solution into a working one, it would have to use a spinlock and could be something like this (this is morning code, so let me know if there's something weird):

    public static Singleton getInstance() {
    
        // Fast-path when singleton already constructed
        if(instance != null)
            return instance;
    
        // Spinlock lets the first thread through, others will spin
        while(instance == null && !isInitialized.compareAndSet(false, true))
            ;
    
        // First thread creates the singleton, spun threads will ignore
        if(instance == null)
            instance = new Singleton();
    
        return instance;
    }
    

    also instance needs to be volatile to ensure visibility.

    The first lock to enter will clear the spinlock, because even though instance is null, the !compareAndSet will return false (as it succeeds the first time).

    After this any thread coming in will stay in the spinlock because instance == null and !compareAndSet is true. When the instance constructing is finished, the spinlock will always fall through because of the first condition.

    This is basically DCL with a spinlock instead of synchronized, and there's no scenario in which I think the above code would be useful.