Search code examples
javamultithreadingthread-safetyguavaatomic

How to make concurrent hash map thread safe with get and put as an atomic operation?


Is my below method thread safe? This method is in Singleton class.

  private static final Map<String, PreparedStatement> holder = new ConcurrentHashMap<>();

  public BoundStatement getStatement(String cql) {
    Session session = TestUtils.getInstance().getSession();
    PreparedStatement ps = holder.get(cql);
    if(ps == null) { // If "ps" is already present in cache, then we don't have to synchronize and make threads wait.
        synchronized {
          ps = holder.get(cql);
          if (ps == null) {
            ps = session.prepare(cql);
            holder.put(cql, ps);
          }
        }
    }
    return ps.bind();
  }

I am working with Cassandra and using datastax java driver so I am reusing prepared statements and that's why I am caching it here. Prepared Statement and BoundStatement.

Is there any better way of making my getStatement method thread safe (if it is thread safe) instead of using synchronized block like that? Any other data structure which might be thread safe for these kind of operations? I am working with Java 7.


Solution

  • Since .putIfAbsent is in Java7, you can use it:

     private static final ConcurrentHashMap<String, PreparedStatement> holder = new ConcurrentHashMap<>();
    
      public BoundStatement getStatement(String cql) {
        Session session = TestUtils.getInstance().getSession();
        PreparedStatement ps = holder.get(cql);
        if(ps == null) { // If "ps" is already present in cache, then we don't have to synchronize and make threads wait.
    
             if (holder.putIfAbsent(cql, session.prepare(cql)) != null) {
                // Someone else got there before, handle
             }
        }
        return ps.bind();
      }
    

    Note that putIfAbsent still uses same synchronization internally.