Java ConcurrentHashMap and synchronization

Question

Let's say that I have a ConcurrentHashMap of clients registered to a server (inside a class Server):

Map<ClientID, String> registeredClients = new ConcurrentHashMap<ClientID, String>();

In my registration method (inside the Server class) I have:

 public void synchronized register(ClientID client, String clientName) { if(!registeredClients.containsKey(client)) registeredClients.put(client, clientName); }

And in any other methods of the server, I always check at the beginning that the client is registered (to be sure that he is eligible to use the method):

if(!registeredClients.containsKey(client)) throw new UnknownClientException();

I have one server thread per client.

Have I to synchronize that check with something like this?

synchronized(registeredClients) { if(!registeredClients.containsKey(client)) throw new UnknownClientException(); }

I think I should because theoretically the client could register after having passed the if guard and before throwing the exception (making the exception actually wrong).

I am not quite how much the ConcurrentHashMap helps programmers with synchronization issues.

There's an easy solution for your register method, but not for the rest. — Stack Exchange Broke The Law
– Stack Exchange Broke The Law, Commented Jun 11, 2015 at 9:12
putIfAbsent() ? I was thinking about that, but I think that behind the scenes it does something similar. I am more concerned with the 'check' problem. I think I have to synchronize multiple 'thread-safe operations'. — Kami
– Kami, Commented Jun 11, 2015 at 9:13
What happens if another thread unregisters the client right after the synchronized block exits? — Stack Exchange Broke The Law
– Stack Exchange Broke The Law, Commented Jun 11, 2015 at 9:20
What synchronization issues in particular are you trying to solve? — Stack Exchange Broke The Law
– Stack Exchange Broke The Law, Commented Jun 11, 2015 at 9:22
You can use remove to check if the key exists, and then just put again if it does. Don't know if you should though.. — Ian2thedv
– Ian2thedv, Commented Jun 11, 2015 at 9:23

Svetlin Zarev · Accepted Answer · 2015-06-11 10:24:54Z

No you don't have to synchonize the containsKey api if the Class is ConcurrentHashMap because as stated in the documentation its operations are thread safe (You can read all the documentation in order to understand how concurrent access is managed).

I suggest you two modifications:

Change

Map<ClientID, String> registeredClients = new ConcurrentHashMap<ClientID, String>();

to

ConcurrentMap<ClientID, String> registeredClients = new ConcurrentHashMap<ClientID, String>();

because you want explicitly manage concurrent access in you code

After doing 1. you can use registeredClients.putifAbsent(client, clientName);

instead of

 if(!registeredClients.containsKey(client)) registeredClients.put(client, clientName);

and avoid the synchronized keyword in the method signature

Abacus · Accepted Answer · 2015-06-11 09:34:49Z

0

If you are using Java 8, you can check with the following code:

registeredClients.computeIfAbsent(client, c -> {throw new UnknownClientException();});

The check if the key exists and the execution of the lambda are atomic. This requires UnknownClientException to be an unchecked exception.

For Java 7 I have no solution that is this simple.

answered Jun 11, 2015 at 9:34

Abacus

19.6k7 gold badges54 silver badges71 bronze badges

3 Comments

Stack Exchange Broke The Law Over a year ago

Note: this is no better than String name = registeredClients.get(client); if(name == null) throw new UnknownClientException(); (if you're not storing any null names in the table), or if(!registeredClients.containsKey(client)) throw new UnknownClientException(); (if you don't need to actually get the client name).

Abacus Over a year ago

wrong. you are using two statements to retrieve the name and to check it and so still have the race condition Kami wants to avoid. The code I wrote uses the computeIfAbsent method of the ConcurrentHashMap - that's what we are talking about here - and the Javadoc for that method states: The entire method invocation is performed atomically, so the function is applied at most once per key.

Stack Exchange Broke The Law Over a year ago

Yes. But doing it atomically gains you nothing, because the client could still be registered right after your atomic call finishes (and throws an exception). End result is the same: the client is registered, but you threw an exception.

Collectives™ on Stack Overflow

Java ConcurrentHashMap and synchronization

2 Answers 2

Comments

3 Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

3 Comments

Related