CODEWITHSUNDEEP
javamultithreadingsynchronization
Sohaib
Sohaib

Reputation: 4694

Is the following implementation thread safe?

I have a java thread safe map which stores String to Object mappings:

ConcurrentHashMap<String, MyObject> map = new ConcurrentHashMap<>();

Here MyObject is a user defined class. Now consider the following method:

public void replace(String key, MyObject o) {
    MyObject m = map.get("somekey");
    synchronized(m) {
        // Modify some internal components of m
        // and some other objects related to m
    }
}

My intention here is I want to maximise performance by not putting a lock on the whole class but only on threads that try to access the same key in the map. Basically every time an entry is replaced in the map (not added) this part of the code gets executed. Something like:

public void put(String key, MyObject o) {
    if (map.putIfAbsent(key, o) == null){ //do something
    }
    else {
        replace(key, o);
    }
}

For all practical purposes we can assume that this is the only method through which a reference to MyObject can be changed. Is this implementation thread safe?

Upvotes: 1

Views: 176

Answers (1)

Nicolas Filotto
Nicolas Filotto

Reputation: 44965

Is the following implementation thread safe?

It depends on how you access and modify your instances of MyObject in your code, if you always access and modify a given instance of MyObject within a synchronized block using this MyObject instance as object's monitor then yes it would be thread safe, otherwise it won't be.

Your put method is not thread safe as putIfAbsent and replace are not executed atomically which could lead to race condition issues, use merge(K key, V value, BiFunction<? super V,? super V,? extends V> remappingFunction) to do the exact same thing but atomically as next:

public void put(String key, MyObject o) {
    map.merge(
        key, o,
        (v1, v2) -> {
            // Modify some internal components of v1
            // and some other objects related to v1
            return v2;
        }
    );
}

Upvotes: 3

Related Questions