Search code examples
javamultithreadingsingletonatomicreference

Singleton with arguments using AtomicReference


I have to create a Singleton which takes input arguments . Basically I need to create a DBConnector in a library based on some configuration . Now this configuration is passed to the library by the consuming app . Based on the passed in configuration , I want to create an instance of DBConnector which then would be reused in the library . I thought of using DI to handle this , but when this library is initialized I dont know if a DB connection is actually required , and I do not want to create this DBConnector if it is not required . Once library is initialized, on a getResponse(RequestType rt) call do I come to know if a DBConnector is required (based on RequestType) and that is when I need to create an instance . Thus the below code look good for a multiThreaded environment ?

public class DBConnectorFactory
{
private static volatile DBConnector dBConnector = null;
private static AtomicReference<DBConnector> atomicReference = new AtomicReference<>();
private DBConnectorFactory()
{}

public static DBConnector getDBConnector(DBConfig dBConfig)
{
    if(dBConnector == null)
    {
        if(atomicReference.compareAndSet(null,new DBConnector(dBConfig)))
            dBConnector = atomicReference.get();
        return atomicReference.get();
    }

    else
        return dBConnector;
}

}

EDIT Wrote a multithreaded test and all the threads are getting the same instance . However just want to make sure I am not missing out on any edge cases due to Java Memory Model


Solution

  • As is, it looks logically sound to me.

    One thing I find intriguing is your use of a singleton pattern that might throw away an instance of DBConnector if you lose a race and resort to using a second AtomicReference#get(). Isn't the entire point of a singleton to ensure that only a single instance is ever created? If this was your intention, then the pattern you are using is not suited for this. You must synchronize.

    Otherwise, if you insist on using lock-free initialization and potentially have multiple instantiations, you you should just use a single AtomicReference, like so:

    private static AtomicReference<DBConnector> instance = new AtomicReference<>();
    
    public static DBConnector getDBConnector(DBConfig dBConfig) {
        // First try
        DBConnector con = instance.get();
        if (con == null) {
            con = // ...
    
            if (instance.compareAndSet(null, con)) {
                // Successful swap, return our copy
                return con;
            } else {
                // Lost the race, poll again
                return instance.get():
            }
        }
    
        // Already present value
        return con;
    }