Behavior of entrySet().removeIf in ConcurrentHashMap Behavior of entrySet().removeIf in ConcurrentHashMap multithreading multithreading

Behavior of entrySet().removeIf in ConcurrentHashMap


I also managed to reproduce such case on my machine.I think, the problem is that EntrySetView (which is returned by ConcurrentHashMap.entrySet()) inherits its removeIf implementation from Collection, and it looks like:

    default boolean removeIf(Predicate<? super E> filter) {        Objects.requireNonNull(filter);        boolean removed = false;        final Iterator<E> each = iterator();        while (each.hasNext()) {            // `test` returns `true` for some entry            if (filter.test(each.next())) {                // entry has been just changed, `test` would return `false` now               each.remove(); // ...but we still remove               removed = true;            }        }        return removed;    }

In my humble opinion, this cannot be considered as a correct implementation for ConcurrentHashMap.


After discussion with user Zielu in comments below Zielu's answer I have gone deeper into the ConcurrentHashMap code and found out that:

  • ConcurrentHashMap implementation provides remove(key, value) method which calls replaceNode(key, null, value)
  • replaceNode checks if both key and value are still present in the map before removing so using it should be fine. Documentation says that it

Replaces node value with v, conditional upon match of cv if * non-null.

  • In the case mentioned in the question ConcurrentHashMap's .entrySet() is called which returns EntrySetView class. Then removeIf method calls .iterator() which returns EntryIterator.
  • EntryIterator extends BaseIterator and inherits remove implementation that calls map.replaceNode(p.key, null, null) which disables conditional removal and just always removes the key.

The negative course of events could be still prevented if iterators always iterated over 'current' values and never returned old ones if some value is modified. I still don't know if that happens or not, but the test case mentioned below seems to verify the whole thing.

I think that have created a test case which shows that the behavior described in my question can really happen. Please correct me if I there are any mistakes in the code.

The code starts two threads. One of them (DELETING_THREAD) removes all entries mapped to 'false' boolean value. Another one (ADDING_THREAD) randomly puts (1, true) or (1,false) values into the map. If it puts true in the value it expects that the entry will still be there when checked and throws an exception if it is not. It throws an exception quickly when I run it locally.

package test;import java.util.Random;import java.util.concurrent.ConcurrentHashMap;public class MainClass {    private static final Random RANDOM = new Random();    private static final ConcurrentHashMap<Integer, Boolean> MAP = new ConcurrentHashMap<Integer, Boolean>();    private static final Integer KEY = 1;    private static final Thread DELETING_THREAD = new Thread() {        @Override        public void run() {            while (true) {                MAP.entrySet().removeIf(entry -> entry.getValue() == false);            }        }    };    private static final Thread ADDING_THREAD = new Thread() {        @Override        public void run() {            while (true) {                boolean val = RANDOM.nextBoolean();                MAP.put(KEY, val);                if (val == true && !MAP.containsKey(KEY)) {                    throw new RuntimeException("TRUE value was removed");                }            }        }    };    public static void main(String[] args) throws InterruptedException {        DELETING_THREAD.setDaemon(true);        ADDING_THREAD.start();        DELETING_THREAD.start();        ADDING_THREAD.join();    }}