CODEWITHSUNDEEP
javasynchronizationdouble-checked-locking
Michele Mariotti
Michele Mariotti

Reputation: 7469

Map synchronization with double check locking

I've read about double-checked locking and its drawbacks, but I'm asking if using a synchronizedMap can be considered safe.

Here is my code:

public class EntityUtils
{
    private static final Map<String, Map<String, String>> searchMap = Collections.synchronizedMap(new HashMap<String, Map<String, String>>());

    private static Map<String, String> getSearchablePathMap(String key)
    {
        Map<String, String> pathMap = searchMap.get(key);
        if(pathMap != null) return pathMap;

        synchronized(searchMap)
        {
            // double check locking (safe for synchronizedMap?)
            pathMap = searchMap.get(key);
            if(pathMap != null) return pathMap;

            pathMap = new HashMap<>();
            pathMap.put(..., ...);
            ...
            // heavy map population operations
            ...

            pathMap = Collections.unmodifiableMap(pathMap);

            searchMap.put(key, pathMap);
        }

        return pathMap;
    }
}

Suggestions or improvements are welcome.

Upvotes: 1

Views: 456

Answers (2)

richj
richj

Reputation: 7529

java.util.concurrent.ConcurrentHashMap has a putIfAbsent method that might help.

There is a slight obstacle in your scenario in that if the key is absent then your code both creates and populates a new map. There is an overhead to using putIfAbsent on the "not absent" execution path if the value to be set in the absent case has a significant construction cost, but if the construction cost is cheap or can be amortised over many calls then this might be useful.

Using a method from the standard library is very likely to be safer than rolling your own unless you have particular expertise in Java concurrency.

Upvotes: 2

axtavt
axtavt

Reputation: 242786

It's safe, but it doesn't have any advantages in this case.

Double-checked locking is used to avoid costly synchronization on the most frequent code path, but in your case get() is synchronized as well, therefore you actually have two synchronized blocks instead of one.

Upvotes: 4

Related Questions