4

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
    }
}
12
  • 2
    but you're calling en.getKey from another thread. that can't be thread-safe for sure Commented Nov 6, 2018 at 12:20
  • Do you have some other thread that may add or remove entries from the Map while you are iterating over it? Commented Nov 6, 2018 at 12:22
  • 1
    @Eran EventQueue.invokeLater does this indeed - as I said Commented Nov 6, 2018 at 12:23
  • Not entirely confident if it works, but see if you are using ConcurrentHashMap or normal HashMap and use the concurrent one if not already... Commented Nov 6, 2018 at 12:24
  • 1
    @AKSW en.getKey() doesn't mutate the Map. Commented Nov 6, 2018 at 12:24

4 Answers 4

2

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());
    }
Sign up to request clarification or add additional context in comments.

1 Comment

Usually, I make a copy of the array list with CopyOnWriteArrayList stackoverflow.com/questions/2950871/…
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.

3 Comments

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.
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."
0

Since I'm already here and there isn't a "complete" answer, there it goes:

The HashMap class documentation explicitly says that if multiple threads access a HashMap instance and at least one modifies it structurally, the HashMap instance must be synchronized externally (usually using Collections.synchronizedMap).

Note that this implementation is not synchronized. If multiple threads access a hash map concurrently, and at least one of the threads modifies the map structurally, it must be synchronized externally. (A structural modification is any operation that adds or deletes one or more mappings; merely changing the value associated with a key that an instance already contains is not a structural modification.) This is typically accomplished by synchronizing on some object that naturally encapsulates the map. If no such object exists, the map should be "wrapped" using the Collections.synchronizedMap method.

It also says that changing the value associated with a key isn't a structural modification, but doesn't say if those cases always produces a defined behavior (probably race conditions occur, IDK).

Now for the ConcurrentModificationException. It is raised by a fail-fast iterator when it can tell that a structural modification was performed it the backing Map without being made through this specific iterator remove() method. That said, iterators in single thread programs can also raise this exception when you alter the backing Map while the iterator is alive if this modification isn't performed using the iterator remove().

From the ConcurrentModificationException documentation:

Note that this exception does not always indicate that an object has been concurrently modified by a different thread. If a single thread issues a sequence of method invocations that violates the contract of an object, the object may throw this exception. For example, if a thread modifies a collection directly while it is iterating over the collection with a fail-fast iterator, the iterator will throw this exception.

In your case, if a thread in your program instantiate an iterator from a HashMap instance just for reading, and another thread (or even the same thread) also instantiate an iterator from the same Map instance and the lifetime of those iterators overlap, if one iterator calls remove(), then the other can raise ConcurrentModificationException.

Comments

-1

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();
    }
}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.