Collections.synchronizedMap(new LinkedHashMap()); is not making Map threadsafe

Question

I'm using following construct for creating a threadsafe Map.

Collections.synchronizedMap(new LinkedHashMap());

Though I'm getting ConcurrentModificationException error.

Answer 1

Without code it is hard to guess what is the real issue, but my guess is, you are not using returned collection to perform operations. As per javadoc

In order to guarantee serial access, it is critical that all access to the backing collection is accomplished through the returned collection. It is imperative that the user manually synchronize on the returned collection when iterating over it:

  Collection c = Collections.synchronizedCollection(myCollection);
     ...
  synchronized(c) {
      Iterator i = c.iterator(); // Must be in the synchronized block
      while (i.hasNext())
         foo(i.next());
  }

Failure to follow this advice may result in non-deterministic behavior.

Answer 2

Not to detract from any of the other answers here, but this code below shows that concurrent modification has little to do with actual multithreading. It's caused when you say, iterate over a collection but modify it while you are iterating....

  List list = new ArrayList();
  list.add("1");
  list.add("2");

  Iterator i = list.iterator();
  while (i.hasNext()) {
      Object value = i.next();  // throws java.util.ConcurrentModificationException

      list.add("another");  
  }

Answer 3

Long story short, the solution to not get a ConcurrentModificationException in your code would be using a ConcurrentHashMap instead of Collections.synchronizedMap(new LinkedHashMap());. Explanation here:

• What's the difference between ConcurrentHashMap and Collections.synchronizedMap(Map)?

As Nambari says, the problem is harder to identify without actual code. Note that this Map only protects the objects that contains. Still, you can modify a same object instance in a method:

Map<String, Object> map = new ConcurrentHashMap<String, Object();
//fill the map...
map.put("data", new Data());

//and now we have this unsynchronized method that two or more threads can access at the same time
public void modifyData(String key, int newDataIntValue) {
    //this is synchronized by the ConcurrentHashMap
    Data data = map.get(key);
    //this isn't
    //you can get problems here...
    data.setIntValue(newDataIntValue);
}

The synchronized collection won't save your code for these cases. You should synchronize this method by yourself.

Additional info: if you're trying to implement a cache library or the Flyweight design pattern, don't reinvent the wheel and use a proven and tested framework like ehcache or jboss cache.