ConcurrentModificationException with ConcurrentHashMap

Question

I understand the concept behind it but thought using ConcurrentHashMap instead of HashMap will fix it. Because ConcurrentHashMap protects from concurrent reading and modification by different threads.

But I still see the exception.

Here's the code snippet-

SampleFile.java

prepareInfo(RequestHelper.getSender(request), someVar, concurrentMap);
....
...
    private void prepareInfo(final Sender sender, final SomeVar someVar, final ConcurrentHashMap<String,
            Object> concurrentMap){
        final Info info = RequestHelper.getInfo(someVar);
        someVar.setInfo(info);
        if(sender != null){
            prepareProfileInfo(sender.getUserDetails(), info, concurrentMap);
            mapDetailsWithMap(sender.getDetails(), concurrentMap);
            if(sender.getSenderId() != null){
                concurrentMap.put("sender_id", sender.getSenderId());
            }
            concurrentMap.putAll(sender.getAdditionalProperties());
        }
    }

The error stacktrace is -

    at java.util.HashMap$HashIterator.nextNode(HashMap.java:1445)
    at java.util.HashMap$EntryIterator.next(HashMap.java:1479)
    at java.util.HashMap$EntryIterator.next(HashMap.java:1477)
    at java.util.concurrent.ConcurrentHashMap.putAll(ConcurrentHashMap.java:1083)
    at SampleFile.prepareAccountInfo(SampleFile.java:114)

Couple of questions I am not clear about -

Why is the exception still happening?
Is there a way we can test the fix via Unit Test?

Joni · Answer 1 · 2020-08-13T18:22:30.197

6

This line retrieves a HashMap with sender.getAdditionalProperties() and then iterates on the HashMap, adding each item to concurrentMap:

        concurrentMap.putAll(sender.getAdditionalProperties());

If the HashMap within sender is modified while the iteration runs, you will get a ConcurrentModificationException. The exception means "the structure of the map was modified while I was iterating on it, so I don't know what to do now".

To allow concurrent modification and iteration on the map inside the sender object, that map should be a ConcurrentHashMap.

To test the fix, you can make a test that does the following:

Map<String,Object> map =  sender.getAdditionalProperties()
map.put("foo", "bar");
Iterator<Map.Entry<String, Object>> iterator = map.entrySet().iterator();
// uh-oh - adding an item invalidates HashMap iterator
map.put("bar", "baz");
// Throws exception for HashMap
iterator.next();

edited Aug 13 '20 at 18:22

answered Aug 13 '20 at 17:54

Joni

108,737
14
143
193

So should I iterate over the map returned by `sender.getAdditionalProperties()`? Also, ConcurrentHashMap isn't needed then - would work with a HashMap too only if I iterate over it? – R11G Aug 13 '20 at 17:59
If it's a valid use case to modify the sender.additionalProperties map while you also iterate on it -- possibly in a separate thread -- then sender.additionalProperties should be a `ConcurrentHashMap` – Joni Aug 13 '20 at 18:26
1

When `concurrentMap.putAll(sender.getAdditionalProperties());` fails with a `ConcurrentModificationException`, the modification must happen in a different thread as `putAll` will not modify the incoming map while iterating. In that case, using a thread safe map in `Sender` would only be a hotfix. It doesn’t change the fundamental problem that someone is modifying the map you’re just trying to copy, so the content of the resulting copy is entirely nondeterministic. – Holger Aug 14 '20 at 09:31
@Holger Wondering if it's because concurrentMap is getting modified a step earlier or the problem lies in the map returned by `sender.getAdditionalProperties` ? I'm not able to figure out how the sender.getAdditionalProperties is getting modified. – R11G Aug 14 '20 at 17:38
1

If `sender.additionalProperties` is not supposed to be modified after it's constructed, you can turn it into an *umodifiable map* with `additionalProperties = Collections.unmodifiableMap(additionalProperties)` if it still gets modified you'll find out quickly – Joni Aug 14 '20 at 18:00

score 4 · Answer 2 · answered Aug 13 '20 at 19:04

You've made an understandable and very common mistake.

You saw ConcurrentModificationException, and you thought: Huh. Apparently, threads.

But that's wrong. The name is... unfortunate, perhaps.

ConcurrentModificationException has nothing whatsoever to do with threading.

CoModEx simply means that this sequence of events has occured:

You have made an iterator out of a collection, e.g. by calling .iterator(), or by writing for (var x : someCollection) which makes one.
At some point in time after you made that iterator, you change the underlying collection in some way. You cleared it, added something to it, updated a value, or removed something. any change at all.
You do anything with the iterator made in step 1, such as calling .hasNext() or next() on it, or your for (var x : someCollection) gets to the closing brace / you run a continue.

Then, boom. CoModEx.

Here's a trivial way to do it. Note how this app is entirely singlethreaded:

class ThisAsplodes {
    public static void main(String[] args) {
        List<String> list = new ArrayList<String>();
        list.add("Okay");
        for (String elem : list) {
            list.add("Bye");
        }
    }
}

compile it, run it, and, voila. ConcurrentModificationException. The list is modified ("Bye" is added), and then iteration occurs on an iterator made prior to the change (the closing brace).

It gets even better: If you try to perform this sequence of events with a plain jane hashmap and multiple threads (one thread makes the iterator, another modifies it), then CoModEx could happen, but more usually things just break in weird ways. a key you just added appears not to be in the map, for example. With ConcurrentHashMap, you get the CoModEx reliably.

It is not obvious from your code where you're doing this, but at least now you know where to look.

ConcurrentModificationException with ConcurrentHashMap

2 Answers2