10

Is using the remove() method okay? I've read an article that synchronization hasn't been added to the remove method. How do I properly remove a specific item from a ConcurrentHashMap?

Example Code:

    ConcurrentHashMap<String,Integer> storage = new ConcurrentHashMap<String,Integer>();
    storage.put("First", 1);
    storage.put("Second", 2);
    storage.put("Third",3);


    //Is this the proper way of removing a specific item from a tread-safe collection?
    storage.remove("First");

    for (Entry<String, Integer> entry : storage.entrySet()) {
        String key = entry.getKey();
        Object value = entry.getValue();
        // ...
        System.out.println(key + " " + value);
    }
  • "I've read an article that synchronization hasn't been added to the remove method" Was that article written about the times of Java 5,6,7, or 8? Maybe it was written with an older Java version in mind. – peter.petrov Jan 09 '15 at 19:48
  • Hopfully this link works http://javarevisited.blogspot.ca/2013/02/concurrenthashmap-in-java-example-tutorial-working.html. He says "Since update operations like put(), remove(), putAll() or clear() is not synchronized Read more: http://javarevisited.blogspot.com/2013/02/concurrenthashmap-in-java-example-tutorial-working.html#ixzz3OM79B2ol " –  Jan 09 '15 at 19:50
  • @peter.petrov - Calling the remove method on a ConcurrentHashMap like I did in my code example is okay, correct? thread-safety shouldn't be an issue? –  Jan 09 '15 at 19:54
  • Not fully sure. What this article means, I guess, is the following: in `remove` not the whole table is locked but only a given segment of it (as visible in the code posted by manouti). At least that's how I am reading the article and its meaning. So it all depends on your concurrency scenario. Not sure, need to research this myself from scratch if I am to answer it with certainty. – peter.petrov Jan 09 '15 at 19:57
  • @peter.petrov - Hm, How then would I remove a specific item from a concurrecthashmap? –  Jan 09 '15 at 19:58
  • What you do is fine, I think. Read the source code. This Segment nested class in ConcurrentHashMap extends ReentrantLock, so it is a lock itself. Again, I doubt you will have such a concurrency scenario which would break the ConcurrentHashMap remove logic. But you should know better. :) – peter.petrov Jan 09 '15 at 20:01
  • @peter.petrov - manouti confirmed it's fine. –  Jan 09 '15 at 20:02
  • Yeah, I would 99% guess so too. – peter.petrov Jan 09 '15 at 20:02
  • There is no synchronization in a `ConcurrentHashMap`. It hasn't been 'added' to the `remove()` method or to any other method. There are *locks*, but no *synchronization*. – user207421 Jun 09 '17 at 08:11

3 Answers3

4

An Iterator should do the job:

Iterator<Map.Entry<String, Integer>> iterator = storage.entrySet().iterator();
while(iterator.hasNext())
{
    Map.Entry<String, Integer> entry = iterator.next();
    if(entry.getKey().equals("First"))
    {
       iterator.remove();
    }
 }

Reference: https://dzone.com/articles/removing-entries-hashmap

Loukan ElKadi
  • 2,687
  • 1
  • 16
  • 20
2

The remove method does synchronize on a lock. Indeed checking the code of ConcurrentHashMap#remove(), there is a call to a lock method that acquires the lock:

public V remove(Object key) {
    int hash = hash(key.hashCode());
    return segmentFor(hash).remove(key, hash, null);
}

where ConcurrentHashMap.Segment#remove(key, hash, null) is defined as:

V remove(Object key, int hash, Object value) {
     lock();
     try {
        ...

Note the Javadoc description:

Retrieval operations (including get) generally do not block, so may overlap with update operations (including put and remove). Retrievals reflect the results of the most recently completed update operations holding upon their onset. For aggregate operations such as putAll and clear, concurrent retrievals may reflect insertion or removal of only some entries. Similarly, Iterators and Enumerations return elements reflecting the state of the hash table at some point at or since the creation of the iterator/enumeration. They do not throw ConcurrentModificationException. However, iterators are designed to be used by only one thread at a time.

M A
  • 71,713
  • 13
  • 134
  • 174
  • The remove method I used in my code example is okay then, right? I don't have to worry about thread-safety? –  Jan 09 '15 at 19:57
  • 1
    Yes it is fine. It is thread-safe in that no locking is needed to use it and no `ConcurrentModificationException` would occur. – M A Jan 09 '15 at 20:00
  • @manouti- Thanks you rock, I've linked the article I got my information from. I think that author is incorrect then. –  Jan 09 '15 at 20:01
  • 1
    Again: the author just says that remove is not synchronized on the whole table object. That's how I am reading it at least. – peter.petrov Jan 09 '15 at 20:03
  • @peter.petrov - So the author of the Blogspot post is incorrect? –  Jan 09 '15 at 20:04
  • No, I think he is correct but you shouldn't worry that much :) – peter.petrov Jan 09 '15 at 20:04
  • @CharlesWhitfield I updated the answer with a Javadoc description. I guess that's what the author meant when he said "concurrent retrieval may not reflect most recent change on Map" – M A Jan 09 '15 at 20:04
  • 1
    @peter.petrov - Just to confirm, and I'll let go of this situation, the way I called it on my example code above is fine, I don't have to worry about thread-safety? –  Jan 09 '15 at 20:05
  • Yes, your code is fine, and you don't have to worry. @manouti Good javadoc quote, I also think that's exactly what the article author meant. +1 from me. – peter.petrov Jan 09 '15 at 20:06
  • Scroll down to the author's summary, #5: All operations of ConcurrentHashMap are thread-safe. – spudone Jan 09 '15 at 20:08
  • 1
    @spudone Yes, there's no contradiction in that. They are thread-safe, it's the level of consistency here which is being discussed (see the javadoc mahouti posted). So e.g. I guess in some convoluted scenario a `get` operation might see that something is in the table which is already `removed` from it :) That's how I am reading all this. Think about an analogue with RDBMS and isolation levels. Hope you see what I mean. – peter.petrov Jan 09 '15 at 20:11
  • 'Synchronize on a lock' is a contradiction in terms. There is no synchronization in a `ConcurrentHashMap`, but there are locks, which are *locked* and *unlocked.* – user207421 Jun 09 '17 at 08:12
1

You can use directly removeIf on the entrySet :

map.entrySet().removeIf( entry -> .. some condicion on entry ) 

Pay attention there is a bug in Java 8 that is only fixed from Java 9 ( here ).

ic3
  • 7,917
  • 14
  • 67
  • 115