How to correctly make a thread safe Singleton Factory in Java?

user2467545 picture user2467545 · Jan 10, 2014 · Viewed 22.3k times · Source

This is the first time I am writing a Factory class. Below is my Factory class, I am not sure whether this is the correct way of making thread safe Singleton Factory class or not. I will be returning instance of my Client using this factory?

public class ClientFactory {

    private static ClientFactory instance = null;   

    private ClientFactory() {

    }

    public static ClientFactory getInstance() {

        if (instance == null)
        {
            instance =  new ClientFactory();
        }

        return instance;
    }

    public IClient getClient() {

        return new TestClient();
    }
}

And here is my TestClient class -

public class TestClient implements IClient {


}

And this is how I am going to use my factory -

IClient client = ClientFactory.getInstance().getClient();

Answer

Jakub Kubrynski picture Jakub Kubrynski · Jan 10, 2014

In fact your factory isn't thread safe, because in race condition you can have more than one ClientFactory living in application. Lets assume two threads:

  1. ThreadA is evaluating condition 'if (instance == null)' and instance is null, so it enters statement
  2. ThreadB is evaluating condition 'if (instance == null)' and instance is null (because A didn't make to instantiate it), so it enters statement
  3. ThreadA creates new ClientFactory() and returns it
  4. ThreadB creates new ClientFactory() and returns it
  5. Now we have more than one ClientFactory in application. Of course other threads trying to retrieve instance some time later will always return single instance.

In my opinion the easiest way to write singleton in Java is to use enum. In your case it will looks:

public enum ClientFactory {
  INSTANCE;

  public Company getClient() {
    return new Company();
  }
}

And usage:

ClientFactory.INSTANCE.getClient()