ConcurrentModificationException when using iterator to remove entry

Question

I have a simple piece of code that loops through a map, checks a condition for each entry, and executes a method on the entry if that condition is true. After that the entry is removed from the map. To delete an entry from the map I use an Iterator to avoid ConcurrentModificationException's.

Except my code does throw an exception, at the it.remove() line:

Caused by: java.util.ConcurrentModificationException
    at java.util.HashMap$HashIterator.remove(Unknown Source) ~[?:1.8.0_161]
    at package.Class.method(Class.java:34) ~[Class.class:?]

After a long search I can't find a way to fix this, all answers suggest using the Iterator.remove() method, but I'm already using it. The documentation for Map.entrySet() clearly specifies that it is possible to remove elements from the set using the Iterator.remove() method.

Any help would be greatly appreciated.

My code:

Iterator<Entry<K, V>> it = map.entrySet().iterator();
while (it.hasNext()) {
    Entry<K, V> en = it.next();

    if (en.getValue().shouldRun()) {
        EventQueue.invokeLater(()->updateSomeGui(en.getKey())); //the map is in no way modified in this method
        en.getValue().run();
        it.remove(); //line 34
    }
}

but you're calling `en.getKey` from another thread. that can't be thread-safe for sure — UninformedUser, Nov 06 '18 at 12:20
Do you have some other thread that may add or remove entries from the Map while you are iterating over it? — Eran, Nov 06 '18 at 12:22
Not entirely confident if it works, but see if you are using ConcurrentHashMap or normal HashMap and use the concurrent one if not already... — Rohit Dodle, Nov 06 '18 at 12:24
@Eran is `en.getKey` called instantly or later in the other thread? Maybe after the iterator has removed the element. — UninformedUser, Nov 06 '18 at 12:33
@AKSW @Eran, no there aren't any threads that *could* mutate the map. Commenting the `EventQueue` line out didn't help either. — superbadcodemonkey, Nov 06 '18 at 12:37
@AKSW `en.getKey()` may be called after the iterator has removed the element, but I'm not sure that's a problem (or at least I don't see why it should be a problem). — Eran, Nov 06 '18 at 12:37
@Rohit Dodle, changing the map to a `ConcurrentHashMap` fixed it! If you post your comment as an answer I'll accept it. I'm why this fixed it though, why doesn't the normal `HashMap` support removing while iterating? (For all purposes the code I posted is single-threaded.) — superbadcodemonkey, Nov 06 '18 at 12:40
Glad that it helped. I'll post the answer with the explanation. Please accept once you go through it. — Rohit Dodle, Nov 06 '18 at 12:44
@supershitcodemonkey `HashMap` supports removing while iterating. Perhaps you didn't post all the relevant code in your question. — Eran, Nov 06 '18 at 12:50
Right, I would also be interested in the reasoner of the exception. Indeed, `ConcurrentHashMap` is a workaround, but it makes more sense to understand why things happen. — UninformedUser, Nov 06 '18 at 16:19

aurelius · Answer 1 · 2018-11-06T13:02:30.587

If you cannot change HashMap to ConcurrentHashMap you can use another approach to your code.

You can create a list of entries containing the entries that you want to delete and then iterate over them and remove them from the original map.

e.g.

    HashMap<String, String> map = new HashMap<>();
    map.put("1", "a1");
    map.put("2", "a2");
    map.put("3", "a3");
    map.put("4", "a4");
    map.put("5", "a5");
    Iterator<Map.Entry<String, String>> iterator = map.entrySet().iterator();
    List<Map.Entry<String, String>> entries = new ArrayList<>();

    while (iterator.hasNext()) {
        Map.Entry<String, String> next = iterator.next();
        if (next.getKey().equals("2")) {
            /* instead of remove
            iterator.remove();
            */
            entries.add(next);
        }
    }

    for (Map.Entry<String, String> entry: entries) {
        map.remove(entry.getKey());
    }

Usually, I make a copy of the array list with CopyOnWriteArrayList https://stackoverflow.com/questions/2950871/how-can-copyonwritearraylist-be-thread-safe — ecle, Nov 06 '18 at 13:21

score 1 · Accepted Answer · answered Nov 06 '18 at 12:51

1

Please use ConcurrentHashMap in place of HashMap as you are acting on the object in multiple threads. HashMap class isn't thread safe and also doesn't allow such operation. Please refer below link for more information related to this.

https://www.google.co.in/amp/s/www.geeksforgeeks.org/difference-hashmap-concurrenthashmap/amp/

Let me know for more information.

answered Nov 06 '18 at 12:51

Rohit Dodle

376
4
13

But that doesn't make sense in this context, since the map literally **can't** be modified from another thread. It's theoretically possible that another thread *read* from it, but that wouldn't explain the `ConcurrentModificationException`. This code sits however in a fairly big system of stuff including some "black magic" asm stuff, so I suppose *something somehow* managed to modify it. I'll still accept this as it did fix my problem. – superbadcodemonkey Nov 06 '18 at 13:28
Oracle documentation of the exception class may be of some help here. It doesn't HAVE to be a modification exactly... even if it's just a read in another thread, it may throw this error. Quoting from the documentation "For example, it is not generally permissible for one thread to modify a Collection while another thread is iterating over it." – Rohit Dodle Nov 06 '18 at 13:36
Oracle's documentation: https://docs.oracle.com/javase/7/docs/api/java/util/ConcurrentModificationException.html – Rohit Dodle Nov 06 '18 at 13:37

score -1 · Answer 3 · answered Oct 13 '21 at 14:17

For such purposes you should use the collection views a map exposes:

keySet() lets you iterate over keys. That won't help you, as keys are usually immutable.

values() is what you need if you just want to access the map values. If they are mutable objects, you can change directly, no need to put them back into the map.

entrySet() the most powerful version, lets you change an entry's value directly.

Example: convert the values of all keys that contain an upperscore to uppercase

for(Map.Entry<String, String> entry:map.entrySet()){
    if(entry.getKey().contains("_"))
        entry.setValue(entry.getValue().toUpperCase());
}

Actually, if you just want to edit the value objects, do it using the values collection. I assume your map is of type <String, Object>:

for(Object o: map.values()){
    if(o instanceof MyBean){
        ((Mybean)o).doStuff();
    }
}

ConcurrentModificationException when using iterator to remove entry

My code:

3 Answers3