ovdsrn
ovdsrn

Reputation: 793

Synchronization on ConcurrentHashMap

In my application I am using a ConcurrentHashMap and I need this type of "custom put-if-absent" method to be executed atomically.

public boolean putIfSameMappingNotExistingAlready(String key, String newValue) {
    String value;
    synchronized (concurrentHashMap) {
    if (value = concurrentHashMap.putIfAbsent(key, newValue)) == null) {
          // There was no mapping for the key
      return true;
      } else { if (value.equals(newValue)) {
                // The mapping <key, newValue> already exists in the map
                return false;
            } else {
                concurrentHashMap.put(key, newValue);
                return true;
            }
        }
     }
    } 

I read (in the concurrent package documentation) that

A concurrent collection is thread-safe, but not governed by a single exclusion lock.

So you can not get an exclusive lock on a ConcurrentHashMap.

My questions are:

  1. Is the code above thread-safe? To me it looks like it is guaranteed that the code in the synchronized block can be executed only by a single thread at the same time, but I want to confirm it.

  2. Wouldn't it be "cleaner" to use Collections.synchronizedMap() instead of ConcurrentHashMap in this case?

Thanks a lot!

Upvotes: 0

Views: 1668

Answers (2)

isnot2bad
isnot2bad

Reputation: 24464

The following code uses a compare-and-set loop (as suggested by SlakS) to implement thread safety (Note the infinite loop):

/**
 * Updates or adds the mapping for the given key.
 * Returns true, if the operation was successful and false,
 * if key is already mapped to newValue.
 */
public boolean updateOrAddMapping(String key, String newValue) {
    while (true) {
        // try to insert if absent
        String oldValue = concurrentHashMap.putIfAbsent(key, newValue);
        if (oldValue == null) return true;

        // test, if the values are equal
        if (oldValue.equals(newValue)) return false;

        // not equal, so we have to replace the mapping.
        // try to replace oldValue by newValue
        if (concurrentHashMap.replace(key, oldValue, newValue)) return true;

        // someone changed the mapping in the meantime!
        // loop and try again from start.
    }
}

Upvotes: 4

Tim B
Tim B

Reputation: 41210

By synchronizing on the entire collection like that you are essentially replacing the fine-grained synchronization within the concurrent collection with your own blunt-force approach.

If you aren't using the concurrency protections elsewhere then you could just use a standard HashMap for this and wrap it in your own synchronization. Using a synchronizedMap may work, but wouldn't cover multi-step operations such as above where you put, check, put.

Upvotes: 0

Related Questions