1

This is basically a continuation of this or this, or many others in that regard.

My question is probably simple, if I use ConcurrentHashMap::compute only, in both readers and writers of some value, is that enough to ensure visibility?

I do know that compute method:

The entire method invocation is performed atomically

Is that enough to guarantee visibility? Specifically, is that true specification/documentation wise with regards to happens-before? To simplify my question, here is an example:

static class State {
    private int age;

    int getAge() {
        return age;
    }

    State setAge(int age) {
        this.age = age;
        return this;
    }
}

and :

// very simplified, no checks
static class Holder {

    private static final ConcurrentHashMap<String, State> CHM = new ConcurrentHashMap<>();

    public State read(String key) {
        return CHM.compute(key, (x, y) -> y);
    }

    public State write(String key, int age) {
        return CHM.compute(key, (x, y) -> y.setAge(y.getAge() + age));
    }
}

No one has access to CHM and can only work via Holder.

To me the answer is obviously yes, this is safe and all readers will see the result of the latest write method. I just can't connect the dots with the documentation of ConcurrentHashMap, which is most probably obvious, but I seem to miss it.

Eugene
  • 117,005
  • 15
  • 201
  • 306
  • 1
    I would consider this to be implied by the thread-safety guarantee. – Louis Wasserman Jun 03 '21 at 17:02
  • 1
    You’re going back to the issue already [explained here](https://stackoverflow.com/a/67822241/2711488). The problem is, what does the caller of `read` do? It will see “the result of the latest `write`”, but which write was the latest write? Without any defined order, none may have happen yet and all of them may be running just now, concurrently to the caller of the `read` method. – Holger Jun 04 '21 at 08:56
  • Yes, it is good enough for visibility. I think it's a bit too much for visibility. For the `read()` method you can use `CHM.get()` and live happily ever after. – Tamas Rev Jun 04 '21 at 14:19
  • @TamasRev no you can't use `get`. The comments and the initial questions I linked discuss that much broader. – Eugene Jun 04 '21 at 19:08
  • @Holger do you mind posting an answer? I understood your point finally. – Eugene Jun 09 '21 at 01:50

2 Answers2

3

The compute() javadoc says what this method does:

Attempts to compute a mapping for the specified key and its current mapped value (or null if there is no current mapping).

So compute() replaces a value for the key.

To use compute() to modify the internal fields of some object (even the object is stored as a value in the map) is not what compute() was meant for.
Therefore, naturally, compute()'s specification/documentation guarantees (and even says) nothing about that.

Regarding happens-before, there are multiple mentions in the documentation:

  • ConcurrentHashMap:

    More formally, an update operation for a given key bears a happens-before relation with any (non-null) retrieval for that key reporting the updated value.

  • ConcurrentMap:

    Memory consistency effects: As with other concurrent collections, actions in a thread prior to placing an object into a ConcurrentMap as a key or value happen-before actions subsequent to the access or removal of that object from the ConcurrentMap in another thread.

  • java.util.concurrent:

    Actions in a thread prior to placing an object into any concurrent collection happen-before actions subsequent to the access or removal of that element from the collection in another thread.

The important thing is that the happen-before relation is only guaranteed between insertion/removal/retrieval of objects to/from the collection.
In your case it is the same State object (only internal its fields are updated), so IMO according to documentation ConcurrentHashMap is even allowed to decide that nothing changed and skip the remaining synchronization steps.

pluk
  • 31
  • 1
  • I don't know where you decided in the first paragraph that it is going to _replace_ the value and that no mutations are allowed, which is still replace, if you ask me. Then simply highlighting the words _happens-before_ is not what I was after, by far. I don't think this answers my question. I will may be revise my opinion after some time when I re-read it, but currently I can't accept this. – Eugene Jun 08 '21 at 18:11
0

There is happens-before between Holder.read and Holder.write methods obviously. But no any guaranties between State.getAge and Holder.write. if you want to read age you actually need to execute two operations:

(1) State state = holder.read(key);

(2) int age = state.getAge();

So, if Holder.write in another thread happens after (1) but before (2), you may see old age value in (2).

Sergey Kosarev
  • 71
  • 5