I would like to use ConcurrentHashMap to let one thread delete some items from the map periodically and other threads to put and get items from the map at the same time.
I'm using map.entrySet().removeIf(lambda)
in the removing thread. I'm wondering what assumptions I can make about its behavior. I can see that removeIf
method uses iterator to go through elements in the map, check the given condition and then remove them if needed using iterator.remove()
.
Documentation gives some info about ConcurrentHashMap iterators behavior:
Similarly, Iterators, Spliterators and Enumerations return elements reflecting the state of the hash table at some point at or since the creation of the iterator/enumeration. hey do not throw ConcurrentModificationException. However, iterators are designed to be used by only one thread at a time.
As the whole removeIf
call happens in one thread I can be sure that the iterator is not used by more than one thread at the time. Still I'm wondering if the course of events described below is possible:
'A'->0
map.entrySet().removeIf(entry->entry.getValue()==0)
.iteratator()
inside removeIf
call and gets the iterator reflecting the current state of the collection map.put('A', 1)
'A'->0
mapping (iterator reflects the old state) and because 0==0
is true it decides to remove A key from the map. 'A'->1
but deleting thread saw the old value of 0
and the 'A' ->1
entry is removed even though it shouldn't be. The map is empty. I can imagine that the behavior may be prevented by the implementation in many ways. For example: maybe iterators are not reflecting put/remove operations but are always reflecting value updates or maybe the remove method of the iterator checks if the whole mapping (both key and value) is still present in the map before calling remove on the key. I couldn't find info about any of those things happening and I'm wondering if there's something which makes that use case safe.
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
.