ConcurrentHashmap - reading and deleting

Question

In an interview, I was asked to check if the following code works?

ConcurrentHashMap<Integer, Integer> chm = new ConcurrentHashMap<Integer, Integer>(); if(chm.get(key) != null) { chm.get(key).doSomething(); chm.remove(key); } 

According to Javadocs, "get" returns the value of the last completed update operation. Therefore, if stream 1 is already called chm.remove (key), and if stream 2 enters the if statement and is about to call the get method, we can get an exception. Is it correct?

Next question: how can I make this thread safe?

Answer 1

You're right. If this Map can be changed in several threads, it is possible that the first call to chm.get(key) will return a non-zero value, and the second call will return null (due to the removal of the key from another thread), and thus chm.get(key).doSomething() will throw a NullPointerException .

You can make this code stream safe by using a local variable to save the result of chm.get(key) :

 ConcurrentHashMap<Integer, Integer> chm = new ConcurrentHashMap<Integer, Integer>(); Integer value = chm.get(key); if(value != null) { value.doSomething(); // PS Integer class doesn't have a doSomething() method // but I guess this is just an example of calling some arbitrary // instance method chm.remove(key); } 

BTW, even if this Map not ConcurentHashMap , and only one thread had access to it, I would still use a local variable, since it is more efficient than calling the get() method twice.

EDIT:

As noted below, this fix will not prevent doSomething() called multiple times for the same key / value by different threads. Regardless of whether this desired behavior is incomprehensible.

If you want to prevent the possibility of calling doSomething() multiple threads for the same key / value, you can use chm.remove(key) to remove the key and get the value in the same step.

However, there is a risk that doSomething() will not be executed for some key / value at all, because if the first call to doSomething() exception, the other call to doSomething() will not be another thread, since the key / value pair will no longer be in Map . On the other hand, if you remove a key / value pair from the Map only after doSomething() is successful, you guarantee that doSomething() is successful at least once for all the key / value pairs that have been re-mapped from Map .

Answer 2

Map.remove(key) returns the value if it was deleted. This is a very good trick in many situations, including yours:

 Object value = chm.remove(key) if(value != null) { value.doSomething(); } 

You cannot work safely with get and then delete, because if two threads call your method at the same time, there is always the risk that they will call doSomething two or more times before the key is deleted.

This is not possible if you delete it first. The code above is Threadsafe, also simpler.