Synchronize write to two collections

Question

I need to put some value to maps if it is not there yet. The key->value (if set) should always be in two collections (that is put should happen in two maps atomically). I have tried to implement this as follows:

private final ConcurrentMap<String, Object> map1 = new ConcurrentHashMap<String, Object>();
private final ConcurrentMap<String, Object> map2 = new ConcurrentHashMap<String, Object>();

public Object putIfAbsent(String key) {
    Object retval = map1.get(key);
    if (retval == null) {
        synchronized (map1) {
            retval = map1.get(key);
            if (retval == null) {
                Object value = new Object(); //or get it somewhere
                synchronized (map2) {
                    map1.put(key, value);
                    map2.put(key, new Object());
                }
                retval = value;
            }
        }
    }
    return retval;
}

public void doSomething(String key) {
    Object obj1 = map1.get(key);
    Object obj2 = map2.get(key);
    //do smth
}

Will that work fine in all cases? Thanks

Answer 1

A few problems:

• Don't use "double-checked locking". A quick Google search will reveal tons of articles which explain the problems with this technique. Just check inside the synchronized block.
• You don't need to synchronize on both map1 and map2. Just use one or the other.
• Synchronize within doSomething. Make sure you synchronize on the same object which is used for synchronization in putIfAbsent.

Answer 2

You should never use synchronized with ConcurrentHashMap (it's pretty much defeating the purpose). The best way to make atomic additions to CHH is by using the built-in replace method. For example:

do {

    oldValue1 = map1.get(key1);
    oldValue2 = map2.get(key2);
    newValue1 = // some logic to determine a new value for key1/value1
    newValue2 = // some more logic to determine a new value for key2/value2

} while (!map1.replace(key1, oldValue1, newValue1) && !map2.replace(key2, oldValue2, newValue2));

I don't know how to specifically adapt this to your example, but this should give you somewhere to start. Basically what happens is you get the key from the map, do some logic, and if the key is still the same as it was before the logic, it will replace the entry and return true, then the loop will break. Otherwise it will just repeat the loop until it can do the update atomically.

Answer 3

Ok, I finally came to this solution:

private Map<String, Object> map1 = new HashMap<String, Object>();
private Map<String, Object> map2 = new HashMap<String, Object>();

private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();

public void putIfAbsent(String key, Object o) {
    rwl.readLock().lock();
    try {
        if (map1.get(key) == null) {
            rwl.readLock().unlock();
            rwl.writeLock().lock();
            try {
                if (map1.get(key) == null) {
                    map1.put(key, getValue());
                    map2.put(key, getValue());
                }
            }finally {
                rwl.readLock().lock();
                rwl.writeLock().unlock();
            }
        }
    } finally {
        readLock.unlock();
    }
}

public void readMap(String key) {
   rwl.readLock().lock();
    try {
       Object obj1 = map1.get(key);
       Object obj2 = map2.get(key);
    } finally {
        rwl.readLock().unlock();
    }

}

Answer 4

In order to do what you want, I'd use atomic references:

class PairHolder {
   public final ConcurrentMap map1;
   public final ConcurrentMap map2;
   public PairHolder(...) // set values here.
}

private AtomicReference<PairHolder> mapHolder = ... // initialize it somehow

do {
  PairHolder holder = mapHolder.get();
  ConcurrentMap map1 = holder.map1.clone()
  ConcurrentMap map2 = holder.map2.clone()
  newMap1.putIfAbsent(...);
  newMap2.putIfAbsent(...);
} while (!mapHolder.compareAndSet(holder, new PairHolder(newMap1,newMap2))

this way you alway will be sure, that mapHolder contains the reference to PairHolder, which in turn have two maps updated 100% atomically. At least CAS should guarantee this, however on multi-processor systems it might be false.