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

Question

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.

The title of your question is a bit off. You don't have to make a ConcurrentHashMap thread-safe: It already is thread-safe right out of the box. What you're trying to do here is to make your _own_ code thread-safe. — Solomon Slow, Nov 28 '16 at 04:24

Alexey Soshin · Accepted Answer · 2016-11-28T20:00:02.743

3

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.

edited Nov 28 '16 at 20:00

answered Nov 27 '16 at 17:56

Alexey Soshin

16,718
2
31
40

What I am supposed to do in the inner if block? How to handle that and what? – john Nov 27 '16 at 18:09
Inner block means that you got two of the same cql prepared simultaneously, and only one got in the map. If you don't care, just drop the block. If you do, log it, or throw an error. – Alexey Soshin Nov 27 '16 at 18:58
1

Note: `putIfAbsent` is present on `ConcurrentHashMap`, not `Map`. You'd need to change the field declaration. – Andy Turner Nov 27 '16 at 19:39
I was looking at this [answer](http://stackoverflow.com/a/8567817/1950349). Is it same thing or different? – john Nov 27 '16 at 19:42

John Vint · Answer 2 · 2016-11-28T13:19:16.627

If you're looking to do some form of memoization, then this is the best/easiest you can do in Java 7. Guava has a computing Cache implementation you can use and Java 8 has a computeIfAbsent method in the Map interface, but you're obviously out of luck here.

If you can create objects on an empty race, like Alexey's answer suggests, that would be the best solution. If you cannot, your implementation is both thread-safe and reasonable.

It is a form of double-checked locking, however, with this implementation you are ensuring a happens-before ordering by using the put and get methods of the CHM.

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

2 Answers2

Linked