Avoiding TreeMap ConcurrentModificationException?

Question

I am calling function that returns TreeMap instance, and in the calling code I wanted to modify the TreeMap. However, I am getting a ConcurrentModificationException.

Here is my code:

public Map<String, String> function1() {
    Map<String, String> key_values = Collections.synchronizedMap(new TreeMap<String, String>());
    // all key_values.put() goes here

    return key_values;
}

And my calling code is:

Map<String, String> key_values =Collections.synchronizedMap(Classname.function1());
//here key_values.put() giving ConcurrentModificationException

Propertius

May I ask the point of creating a Synchronized Map from within function1? It is not being used by anyone but your "calling code"... – Golconda

how you modifying the map? – Wrathful

Answer 1

If you use a ConcurrentSkipListMap is can be faster and doesn't have this issue.

public NavigableMap<String, String> function1() {
    NavigableMap<String, String> key_values = new ConcurrentSkipListMap<String, String>();
    // all key_values.put() goes here

    return key_values;
}

If you don't need the keys to be sorted you can use a ConcurrentHashMap.

Aerostat

It may be useful to understand what a skip list is: en.wikipedia.org/wiki/Skip_list – Solvent

Be aware of the following note in the javadoc if you are using one of the compute... or merge methods: The function is NOT guaranteed to be applied once atomically. link. This can be a confusing note; but as far as I can tell it means that the logic must be idempotent (because it may be repeated). ConcurrentHashMap provides a stronger guarantee: that your logic will be computed exactly once. – Asgard

@Asgard CHM can live lock if you modify the map inside the compute function. Other Maps can have both values (the key appears more than once) In short, don't do it. – Aerostat

Answer 2

Note that Collections.synchronizedMap will never protect you from concurrent modification if you're using an iterator. In addition, unless you're accessing your Map from more than one thread, creating the synchronized map is useless. Locally-scoped collections and variables that are not being handed to other threads do not need to be synchronized.

My guess is that in the code you left out, you're iterating over one of Map.entrySet, Map.keySet, or Map.values, and calling put during that iteration (within the for loop). With the code you've shown, this is the only way this could happen.

Donahoe

Ya you r write sir,that was my mistake..Thanks – Propertius

Answer 3

You appear to be getting a synchronized map of a synchronized map. If I replace the call to function1() with a it's contents (simplified) we have:

Map<String, String> key_values =Collections.synchronizedMap(Collections.synchronizedMap( new TreeMap<String, String>()));

I think your calling line should be changed to:

Map<String, String> key_values = Classname.function1();

Ova

@Donahoe I think this detail is important. Because, with this double wrap he will not be able to synchronize on the actual TreeMap object which he would need if he tries to Iterate and update as well. – Kirtley

Answer 4

You are looking for a synchronized MAP, so I assume you are dealing with a multithreaded app. In that case if you want to use iterator, you must synchronized block for the MAP.

/*This reference will give error if you update the map after synchronizing values.*/
    Map<String, String> values =Collections.synchronizedMap(function1());

/*This reference will not give error if you update the map after synchronizing values  */
        Map<String, String> values = Collections.synchronizedMap(new TreeMap<String, String>());


     synchronized (values) 
                {           
                    Iterator it = values.entrySet().iterator();
                    while(it.hasNext())
                    {
                        it.next() ; 
    // You can update the map here.     
                    }
                }

Update :

Actually in your case, considering the err that you are twice wrapping the MAP, modifying it in the while loop even with a synchronized block will give a CM Exception as you would not be able to synchronize on the original MAP object that is getting udpated.

Kirtley

Answer 5

This is what I was doing that raised ConcurrentModificationException:

TreeMap<Integer, Integer> map = new TreeMap<>();

for (Integer i : map.keySet()) {
    map.remove(i)
}

This is what I did for the same functionality to fix the exception:

TreeMap<Integer, Integer> map = new TreeMap<>();

// I would not recommend this in production as it will create multiple
// copies of map
for (Integer i : new HashMap<>(integers).keySet()) {
    map.remove(i)
}

Smoothtongued