CODEWITHSUNDEEP
javamultithreadingconcurrencyhashmap
Kevin Herron
Kevin Herron

Reputation: 7005

Correct HashMap Synchronization

Let's say I have a HashMap declared as follows:

@GuardedBy("pendingRequests")
private final Map<UInt32, PendingRequest> pendingRequests = new HashMap<UInt32, PendingRequest>();

Access to the map is multi-threaded, and all access is guarded by synchronizing on this final instance of the map, e.g.:

synchronized (pendingRequests) {
    pendingRequests.put(reqId, request);
}

Is this enough? Should the map be created using Collections.synchronizedMap()? Should I be locking on a dedicated lock object instead of the map instance? Or maybe both?

External synchronization (in addition to possibly using Collections.synchronizedMap()) is needed in a couple areas where multiple calls on the map must be atomic.

Upvotes: 0

Views: 351

Answers (2)

sjlee
sjlee

Reputation: 7886

All calls to the map need to be synchronized, and Collections.synchronizedMap() gives you that.

However, there is also an aspect of compound logic. If you need the integrity of the compound logic, synchronization of individual calls is not enough. For example, consider the following code:

Object value = yourMap.get(key); // synchronized
if (value == null) {
    // do more action
    yourMap.put(key, newValue); // synchronized
}

Although individual calls (get() and put()) are synchronized, your logic will not be safe against concurrent access.

Another interesting case is when you iterate. For an iteration to be safe, you'd need to synchronize for the entire duration of the iteration, or you will get ConcurrentModificationExceptions.

Upvotes: 0

Trevor Freeman
Trevor Freeman

Reputation: 7242

Synchronizing on the map itself is essentially what the Map returned by Collection.synchronizedMap() would do. For your situation it is a reasonable approach, and there is not much to recommend using a separate lock object other than personal preference (or if you wish to have more fine grained control and use a ReentrantReadWriteLock to allow concurrent reading of the map).

E.g.

private Map<Integer,Object> myMap;
private ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();

public void myReadMethod()
{
  rwl.readLock().lock();
  try
  {

    myMap.get(...);
    ...
  } finally
  {
    rwl.readLock().unlock();
  }
}

public void myWriteMethod()
{
  // may want / need to call rwl.readLock().unlock() here,
  // since if you are holding the readLock here already then
  // you cannot get the writeLock (so be careful on how your
  // methods lock/unlock and call each other).
  rwl.writeLock().lock();
  try
  {
    myMap.put(key1,item1);
    myMap.put(key2,item2);
  } finally
  {
    rwl.writeLock().unlock();
  }
}

Upvotes: 5

Related Questions