Can race condition occur in below code

Could a race codition occur when invoking method addId from mul开发者_开发百科tiple threads ?

private static Map<String , Long> table;

static {
    table = new ConcurrentHashMap<String  , Long>();
}

public static void addId(String key, Long value){

    if(table.containsKey(key)){
        table.remove(key);
    }

    table.put(key, value);
}

Nothing prevents another thread from putting some value between the containsKey/remove and the put, so checking if the table already contains a key before putting isn't really "safe" (or rather, doesn't really "make sense").

Why don't you just do

public static void addId(String key, Long value){
    table.put(key, value);
}

put will override any previous entry anyway.

If you don't want multiple threads to execute the method concurrently, declare the method as synchronized.

Yes, a race condition could occur.

   if(table.containsKey(key)){
        table.remove(key);
    }

    table.put(key, value);

Another thread could modify the table between your containsKey, the remove and the put. There's no need to call remove() before doing the put(), though - remove the containsKey / remove and it will be thread safe.

Yes.

(It seems that my answer has to be 30 characters or more)

The addId call can gave a race condition: one thread can try to put a key and another can remove the a key at the same time (if it's the same key, then it could be a problem). Within that method there are a couple more cases in which you can get a race condition (with various consequences).

This may further be complicated if you have other methods that do other things with the table, i.e. read from it, and it will make your race condition much worse than "just" an overwritten value.

Ultimately, what are you asking? Are you wondering how to avoid the race condition? What does the rest of your map class do?

If you wanted to guarantee something is done with the old value when it is replaced, then the race could result in not handling all the overwritten keys.

if(table.containsKey(key)){
    Long oldValue = table.remove(key);
}
table.put(key, value);
if (oldValue != null)
    importantOperation(oldValue);

Edit:

It looks like put also returns the old value if there was one, so the check is still not necessary (as others have specified). The check would be necessary in the following awkward situation, which would have racing involved:

if(table.containsKey(key)){
    Long oldValue = table.remove(key);
    value = secretlyAdjustInput(oldValue, value)
}
table.put(key, value);