Thread Safe singleton class

Question

I wrote a below Singleton class. I am not sure whether this is thread safe singleton class or not?

public class CassandraAstyanaxConnection {      private static CassandraAstyanaxConnection _instance;     private AstyanaxContext<Keyspace> context;     private Keyspace keyspace;     private ColumnFamily<String, String> emp_cf;        public static synchronized CassandraAstyanaxConnection getInstance() {         if (_instance == null) {             _instance = new CassandraAstyanaxConnection();         }         return _instance;     }      /**      * Creating Cassandra connection using Astyanax client      *      */     private CassandraAstyanaxConnection() {          context = new AstyanaxContext.Builder()         .forCluster(ModelConstants.CLUSTER)         .forKeyspace(ModelConstants.KEYSPACE)         .withAstyanaxConfiguration(new AstyanaxConfigurationImpl()                   .setDiscoveryType(NodeDiscoveryType.RING_DESCRIBE)         )         .withConnectionPoolConfiguration(new ConnectionPoolConfigurationImpl("MyConnectionPool")             .setPort(9160)             .setMaxConnsPerHost(1)             .setSeeds("127.0.0.1:9160")         )         .withAstyanaxConfiguration(new AstyanaxConfigurationImpl()                   .setCqlVersion("3.0.0")             .setTargetCassandraVersion("1.2"))         .withConnectionPoolMonitor(new CountingConnectionPoolMonitor())         .buildKeyspace(ThriftFamilyFactory.getInstance());          context.start();         keyspace = context.getEntity();          emp_cf = ColumnFamily.newColumnFamily(             ModelConstants.COLUMN_FAMILY,              StringSerializer.get(),              StringSerializer.get());     }      /**      * returns the keyspace      *       * @return      */     public Keyspace getKeyspace() {         return keyspace;     }      public ColumnFamily<String, String> getEmp_cf() {         return emp_cf;     } } 

Can anyone help me with this? Any thoughts on my above Singleton class will be of great help.

Updated Code:-

I am trying to incorporate Bohemian suggestion in my code. Here is the updated code, I got-

public class CassandraAstyanaxConnection {     private static class ConnectionHolder {         static final CassandraAstyanaxConnection connection = new CassandraAstyanaxConnection();     }     public static CassandraAstyanaxConnection getInstance() {         return ConnectionHolder.connection;     }     /**      * Creating Cassandra connection using Astyanax client      *      */     private CassandraAstyanaxConnection() {         context = new AstyanaxContext.Builder()         .forCluster(ModelConstants.CLUSTER)         .forKeyspace(ModelConstants.KEYSPACE)         .withAstyanaxConfiguration(new AstyanaxConfigurationImpl()               .setDiscoveryType(NodeDiscoveryType.RING_DESCRIBE)                 )                 .withConnectionPoolConfiguration(new ConnectionPoolConfigurationImpl("MyConnectionPool")                 .setPort(9160)                 .setMaxConnsPerHost(1)                 .setSeeds("127.0.0.1:9160")                         )                         .withAstyanaxConfiguration(new AstyanaxConfigurationImpl()                               .setCqlVersion("3.0.0")                         .setTargetCassandraVersion("1.2"))                         .withConnectionPoolMonitor(new CountingConnectionPoolMonitor())                         .buildKeyspace(ThriftFamilyFactory.getInstance());         context.start();         keyspace = context.getEntity();         emp_cf = ColumnFamily.newColumnFamily(                 ModelConstants.COLUMN_FAMILY,                  StringSerializer.get(),                  StringSerializer.get());     }     /**      * returns the keyspace      *       * @return      */     public Keyspace getKeyspace() {         return keyspace;     }     public ColumnFamily<String, String> getEmp_cf() {         return emp_cf;     } } 

Can anyone take a look and let me know if this time I got it right or not?

Thanks for the help.

Answer 1

You are implementing the lazy initialization pattern - where the instance is created when first used.

But there is a simple trick that allows you to code a threadsafe implementation that doesn't require synchronization! It is known as the Initialization-on-demand holder idiom, and it looks like this:

public class CassandraAstyanaxConnection {      private CassandraAstyanaxConnection(){ }              private static class Holder {        private static final CassandraAstyanaxConnection INSTANCE = new CassandraAstyanaxConnection();     }      public static CassandraAstyanaxConnection getInstance() {         return Holder.INSTANCE;     }     // rest of class omitted } 

This code initializes the instance on the first calling of getInstance(), and importantly doesn't need synchronization because of the contract of the class loader:

• the class loader loads classes when they are first accessed (in this case Holder's only access is within the getInstance() method)
• when a class is loaded, and before anyone can use it, all static initializers are guaranteed to be executed (that's when Holder's static block fires)
• the class loader has its own synchronization built right in that make the above two points guaranteed to be threadsafe

It's a neat little trick that I use whenever I need lazy initialization. You also get the bonus of a final instance, even though it's created lazily. Also note how clean and simple the code is.

Edit: You should set all constructors as private or protected. Setting and empty private constructor will do the work

Answer 2

all above methods are eagerly initializing object. how about this. This will help you to initialize ur class lazily. You may have heavy object and you don't want to initialize on startup.

public class MySinglton {     private MySinglton (){}    private static volatile MySinglton s;    public static MySinglton getInstance(){     if (s != null ) return s;      synchronized(MySinglton.class){       if (s == null ) {        s = new MySinglton();      }   }    return s;  }  }