How can I block ConcurrentHashMap get() operations during a put()

Question

ConcurrentHashMap<String, Config> configStore = new ConcurrentHashMap<>();
...
void updateStore() {
  Config newConfig = generateNewConfig();
  Config oldConfig = configStore.get(configName);
  if (newConfig.replaces(oldConfig)) {
   configStore.put(configName, newConfig);
  }
}

The ConcurrentHashMap can be read by multiple threads but can be updated only by a single thread. I'd like to block the get() operations when a put() operation is in progress. The rationale here being that if a put() operation is in progress, that implies the current entry in the map is stale and all get() operations should block until the put() is complete. How can I go about achieving this in Java without synchronizing the whole map?

Answer 1

How can I go about achieving this in Java without synchronizing the whole map?

There are some good answers here but there is a simpler answer to use the ConcurrentMap.replace(key, oldValue, newValue) method which is atomic.

while (true) {
    Config newConfig = generateNewConfig();
    Config oldConfig = configStore.get(configName);
    if (!newConfig.replaces(oldConfig)) {
        // nothing to do
        break;
    }
    // this is atomic and will only replace the config if the old hasn't changed
    if (configStore.replace(configName, oldConfig, newConfig)) {
        // if we replaced it then we are done
        break;
    }
    // otherwise, loop around and create a new config
}

Answer 2

The accepted answer proposed to use compute(...) instead of put().

But if you want

to block the get() operations when a put() operation is in progress

then you should also use compute(...) instead of get().

That's because for ConcurrentHashMap get() doesn't block while compute() is in progress.

---

Here is a unit test to prove it:

  @Test
  public void myTest() throws Exception {
    var map = new ConcurrentHashMap<>(Map.of("key", "v1"));
    var insideComputeLatch = new CountDownLatch(1);

    var threadGet = new Thread(() -> {
      try {
        insideComputeLatch.await();
        System.out.println("threadGet: before get()");
        var v = map.get("key");
        System.out.println("threadGet: after get() (v='" + v + "')");
      } catch (InterruptedException e) {
        throw new Error(e);
      }
    });

    var threadCompute = new Thread(() -> {
      System.out.println("threadCompute: before compute()");
      map.compute("key", (k, v) -> {
        try {
          System.out.println("threadCompute: inside compute(): start");
          insideComputeLatch.countDown();
          threadGet.join();
          System.out.println("threadCompute: inside compute(): end");
          return "v2";
        } catch (InterruptedException e) {
          throw new Error(e);
        }
      });
      System.out.println("threadCompute: after compute()");
    });

    threadGet.start();
    threadCompute.start();

    threadGet.join();
    threadCompute.join();
  }

Output:

threadCompute: before compute()
threadCompute: inside compute(): start
threadGet: before get()
threadGet: after get() (v='v1')
threadCompute: inside compute(): end
threadCompute: after compute()

Answer 3

This fundamentally doesn't work. Think about it: When the code realizes that the information is stale, some time passes and then a .put call is done. Even if the .put call somehow blocks, the timeline is as follows:

• Some event occurs in the cosmos that makes your config stale.
• Some time passes. [A]
• Your run some code that realizes that this is the case.
• Some time passes. [B]
• Your code begins the .put call.
• An extremely tiny amount of time passes. [C]
• Your code finishes the .put call.

What you're asking for is a strategy that eliminates [C] while doing absolutely nothing whatsoever to prevent reads of stale data at point [A] and [B], both of which seem considerably more problematic.

Whatever, just give me the answer

ConcurrentHashMap is just wrong if you want this, it's a thing that is designed for multiple concurrent (hence the name) accesses. What you want is a plain old HashMap, where every access to it goes through a lock. Or, you can turn the logic around: The only way to do what you want is to engage a lock for everything (both reads and writes); at which point the 'Concurrent' part of ConcurrentHashMap has become completely pointless:

private final Object lock = new Object[0];

public void updateConfig() {
    synchronized (lock) {
       // do the stuff
    }
}

public Config getConfig(String key) {
    synchronized (lock) {
        return configStore.get(key);
    }
}

NB: Use private locks; public locks are like public fields. If there is an object that code outside of your control can get a ref to, and you lock on it, you need to describe the behaviour of your code in regards to that lock, and then sign up to maintain that behaviour forever, or indicate clearly when you change the behaviour that your API just went through a breaking change, and you should thus also bump the major version number.

For the same reason public fields are almost invariably a bad idea in light of the fact that you want API control, you want the refs you lock on to be not accessible to anything except code you have under your direct control. Hence why the above code does not use the synchronized keyword on the method itself (as this is usually a ref that leaks all over the place).

Okay, maybe I want the different answer

The answer is either 'it does not matter' or 'use locks'. If [C] truly is all you care about, that time is so short, and pales in comparison to the times for [A] and [B], that if A/B are acceptable, certainly so is C. In that case: Just accept the situation.

Alternatively, you can use locks but lock even before the data ever becomes stale. This timeline guarantees that no stale data reads can ever occur:

• The cosmos cannot ever make your data stale.
• Your code, itself, is the only causal agent for stale date.
• Whenever code runs that will or may end up making data stale:
• Acquire a lock before you even start.
• Do the thing that (may) make some config stale.
• Keep holding on to the lock; fix the config.
• Release the lock.

Answer 4

It surely looks like you can defer this to compute and it will take care for that for you:

Config newConfig = generateNewConfig();
configStore.compute(
    newConfig,
    (oldConfig, value) -> {
       if (newConfig.replaces(oldConfig)) {
            return key;
       }
       return oldConfig;
    }
);

You get two guarantees from using this method:

Some attempted update operations on this map by other threads may be blocked while computation is in progress, so the computation should be short and simple

and

The entire method invocation is performed atomically

according to its documentation.