Java Synchronization – Java synchronized Block vs. Collections.synchronizedMap

javasynchronization

Is the following code set up to correctly synchronize the calls on synchronizedMap?

public class MyClass {
  private static Map<String, List<String>> synchronizedMap = Collections.synchronizedMap(new HashMap<String, List<String>>());

  public void doWork(String key) {
    List<String> values = null;
    while ((values = synchronizedMap.remove(key)) != null) {
      //do something with values
    }
  }

  public static void addToMap(String key, String value) {
    synchronized (synchronizedMap) {
      if (synchronizedMap.containsKey(key)) {
        synchronizedMap.get(key).add(value);
      }
      else {
        List<String> valuesList = new ArrayList<String>();
        valuesList.add(value);
        synchronizedMap.put(key, valuesList);
      }
    }
  }
}

From my understanding, I need the synchronized block in addToMap() to prevent another thread from calling remove() or containsKey() before I get through the call to put() but I do not need a synchronized block in doWork() because another thread cannot enter the synchronized block in addToMap() before remove() returns because I created the Map originally with Collections.synchronizedMap(). Is that correct? Is there a better way to do this?

Best Answer

Collections.synchronizedMap() guarantees that each atomic operation you want to run on the map will be synchronized.

Running two (or more) operations on the map however, must be synchronized in a block. So yes - you are synchronizing correctly.

Related Solutions

Java – Why Synchronize on SynchronizedMap or SynchronizedCollections

why do we need to synchronize on that synchronizemap itself?

You may need to synchronize on an already synchronized collection because you are performing two operations on the collection -- in your example, a containsKey() and then a put(). You are trying to protect against race conditions in the code that is calling the collection. In addition, in this case, the synchronized block also protects the ArrayList values so that multiple threads can add their values to these unsynchronized collections.

If you look at the code you linked to, they first check for the existence of the key and then put a value into the map if the key did not exist. You need to protect against 2 threads checking for a key's existence and then both of them putting into the map. The race is which one will put first and which one will overwrite the previous put.

The synchronized collection protects itself from multiple threads corrupting the map itself. It does not protect against logic race conditions around multiple calls to the map.

synchronized (synchronizedMap) {
    // test for a key in the map
    if (synchronizedMap.containsKey(key)) {
      synchronizedMap.get(key).add(value);
    } else {
      List<String> valuesList = new ArrayList<String>();
      valuesList.add(value);
      // store a value into the map
      synchronizedMap.put(key, valuesList);
   }
}

This is one of the reasons why the ConcurrentMap interface has the putIfAbsent(K key, V value);. That does not require two operations so you may not need to synchronize around it.

Btw, I would rewrite the above code to be:

synchronized (synchronizedMap) {
    // test for a key in the map
    List<String> valuesList = synchronizedMap.get(key);
    if (valueList == null) {
      valuesList = new ArrayList<String>();
      // store a value into the map
      synchronizedMap.put(key, valuesList);
    }
    valuesList.add(value);
}

Lastly, if most of the operations on the map need to be in a synchronized block anyway, you might as well not pay for the synchronizedMap and just use a HashMap always inside of synchronized blocks.

Java – Synchronized Block vs ConcurrentHashMap vs Collections.synchronizedMap

CHM(ConcurrentHashMap), instead of synchronizing every method on a common lock, restricting access to a single thread at a time, it uses a finer-grained locking mechanism called lock striping to allow a greater degree of shared access. Arbitrarily many reading threads can access the map concurrently, readers can access the map concurrently with writers, and a limited number of writers can modify the map concurrently. The result is far higher throughput under concurrent access, with little performance penalty for single-threaded access. ConcurrentHashMap, along with the other concurrent collections, further improve on the synchronized collection classes by providing iterators that do not throw ConcurrentModificationException, thus eliminating the need to lock the collection during iteration.

As with all improvements, there are still a few tradeoffs. The semantics of methods that operate on the entire Map, such as size and isEmpty, have been slightly weakened to reflect the concurrent nature of the collection. Since the result of size could be out of date by the time it is computed, it is really only an estimate, so size is allowed to return an approximation instead of an exact count. While at first this may seem disturbing, in reality methods like size and isEmpty are far less useful in concurrent environments because these quantities are moving targets.

Secondly, Collections.synchronizedMap

It's just simple HashMap with synchronized methods - I'd call it deprecated dute to CHM

Best Answer

Related Solutions

Java – Why Synchronize on SynchronizedMap or SynchronizedCollections

Java – Synchronized Block vs ConcurrentHashMap vs Collections.synchronizedMap

Related Question