Is this code a thread-safe one?

Question

I want to process a flow of client requests. Each request has its special type. First I need to initialize some data for that type, and after this I can start processing the requests. When the client type comes for the first time, I just initialize the corresponding data. After this all the following requests of that type are processed using that data.

I need to do this in a thread-safe manner.

Here is a code I have written. Is it thread-safe?

public class Test {

    private static Map<Integer, Object> clientTypesInitiated = new ConcurrentHashMap<Integer, Object>();

    /* to process client request we need to 
    create corresponding client type data.
    on the first signal we create that data, 
    on the second - we process the request*/

    void onClientRequestReceived(int clientTypeIndex) {
        if (clientTypesInitiated.put(clientTypeIndex, "") == null) {
            //new client type index arrived, this type was never processed
            //process data for that client type and put it into the map of types
            Object clientTypeData = createClientTypeData(clientTypeIndex);
            clientTypesInitiated.put(clientTypeIndex, clientTypeData);
        } else {
            //already existing index - we already have results and we can use them
            processClientUsingClientTypeData(clientTypesInitiated.get(clientTypeIndex));
        }
    }

    Object createClientTypeData(int clientIndex) {return new Object();}

    void processClientUsingClientTypeData(Object clientTypeData) {}
}

From one hand, ConcurrentHashMap cannot produce map.put(A,B) == null two times for the same A. From the other hand, the assignment and comparisson operation is not thread-safe.

So is this code is ok? If not, how can I fix it?

UPDATE: I have accepted the answer of Martin Serrano because his code is thread-safe and it is not prone to double initialization issue. But I would like to note, that I have not found any isssues with my version, posted as an answer below, and my version does not require synchronization.

Answer 1

No, I don't think it is still thread-safe.

You need to wrap put operation in a synchronized block.

As per javadoc for ConcurrentHashMap

Retrieval operations (including get) generally do not block, so may overlap with update operations (including put and remove).

Answer 2

This code is not thread safe because

//already existing index - we already have results and we can use them
processClientUsingClientTypeData(clientTypesInitiated.get(clientTypeIndex));

has a chance of getting the "" value you temporarily insert in the put check.

This code could be made threadsafe thusly:

public class Test {

    private static Map<Integer, Object> clientTypesInitiated = new ConcurrentHashMap<Integer, Object>();

    /* to process client request we need to 
       create corresponding client type data.
       on the first signal we create that data, 
       on the second - we process the request*/

void onClientRequestReceived(int clientTypeIndex) {
    Object clientTypeData = clientTypesInitiated.get(clientTypeIndex);
    if (clientTypeData == null) {
        synchronized (clientTypesInitiated) {
          clientTypeData = clientTypesInitiated.get(clientTypeIndex);
          if (clientTypeData == null) {
              //new client type index arrived, this type was never processed
              //process data for that client type and put it into the map of types
              clientTypeData = createClientTypeData(clientTypeIndex);
              clientTypesInitiated.put(clientTypeIndex, clientTypeData);
          }
        }
    }
    processClientUsingClientTypeData(clientTypeData);
}

Object createClientTypeData(int clientIndex) {return new Object();}

void processClientUsingClientTypeData(Object clientTypeData) {}

}