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.

Answer 1

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.

Answer 2

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