Java byte array multi-thread

Question

If I have multiple threads accessing the getter and setter, is this code going to run into any race condition? I do not mind the getter gets the old data during the set operation, but as long as it does not cause an exception or got null.

ConcurrentHashMap<String, Object> hashMap =
    new ConcurrentHashMap<String, Object> ();

void setByteArray(String string, byte[] byteArray) {
    hashMap.put(string, byteArray.clone());
}

byte[] getByteArray(String string) {
    return ((byte[]) hashMap.get(string)).clone();
}

Answer 1

This is almost thread-safe (if there is such a thing). The only thing missing is to declare the hashMap field final. This guarantees safe publication of the map.

Other than that, I don't see any problems (regarding thread-safety). ConcurrentHashMap is thread safe, so storing and retrieving the byte arrays should be as well.

Also, since you always copy the byte arrays, they will never be shared among threads, except for the ones that are stored in the Map. The ConcurrentHashMap will safely publish those to all threads and since they are never modified (meaning they're effectively immutable), thread safety is guaranteed.

Finally, based on the comments, here is an improved version regarding some other aspects:

private final ConcurrentHashMap<String, Object> hashMap =
    new ConcurrentHashMap<String, Object> ();

void setByteArray(String string, byte[] byteArray) {
    hashMap.put(string, byteArray.clone());
}

byte[] getByteArray(String string) {
    Object result = hashMap.get(string);
    if(result == null)
        return null;
    else
        return ((byte[]) result).clone();
}

The first thing is the private modifier for hashMap, so subclasses can not store any other objects, for example shared byte arrays.

The second thing is the null check in the getter. You might want to replace return null; by throw new IllegalArgumentException(); or something else, based on your requirements.