CODEWITHSUNDEEP
javamultithreadingsynchronizationconcurrenthashmap
SyBer
SyBer

Reputation: 5597

How safe is to delete already removed ConcurrentHashMap element?

In case of following code, run by multiple threads:

private static final Map<String, keyinfo> mapKeys = new ConcurrentHashMap<String, keyinfo>();

private static void purgeOldKeys() {
    for (Map.Entry<String, keyinfo> key : mapKeys.entrySet()) {
        if(key.getValue().createTime + keyCacheTime < getCurrentDBTime())
            mapKeys.remove(key);
    }
}

Can I avoid the synchronizer?

Or because removing already removed element, is not defined according to JavaDoc, the synchronizer will be still required?

Upvotes: 2

Views: 7060

Answers (2)

Has QUIT--Anony-Mousse
Has QUIT--Anony-Mousse

Reputation: 77474

In general, when removing from a collection, it is much cleaner (and faster!) to use a full Iterator API instead of the lazy "foreach" notion.

The iterator.remove(); will not invalidate the iterator; and it knows the position it was at. Use this pattern:

for (Iterator<> iter = map.entrySet().iterator(); iter.hasNext(); ) {
    Map.Entry<> entry = iter.next();
    if (testRemoval(entry))
        iter.remove(); // <----- remove using the iterator position!
}

It's faster because it does not involve searching the object again. It's more robust, because the iterator knows the object has been removed. In many collections, the code you showed above will "fast fail" because of a concurrent modification.

Upvotes: 5

Evgeniy Dorofeev
Evgeniy Dorofeev

Reputation: 136062

1) This code can't remove anything because there's a bug in it - mapKeys.remove(key); - key in your code is in fact a Map.Entry. It should be 

for (Map.Entry<String, keyinfo> e : map.entrySet()) {
    if (e.getValue().createTime + keyCacheTime < getCurrentDBTime())
            map.remove(e.getKey());
    }
}

2) As for removing entries while iterating over ConcurrentHashMap it is safe, ConcurrentHashMap.entrySet API 

The view's iterator is a "weakly consistent" iterator that will never throw ConcurrentModificationException

and this test confirms it

    Map<String, String> map = new ConcurrentHashMap<String, String>();
    map.put("1", "2");
    map.put("2", "2");
    map.put("3", "3");
    for (String k : map.keySet()) {
        map.remove(k);
    }
    System.out.println(map);

prints

{}

Upvotes: 4

Related Questions