Do we need to make ConcurrentHashMap volatile?

Question

We have a shared ConcurrentHashMap which is read and written by 2 threads.

class Test {
    private ConcurrentHashMap<Object, Object> map = new ConcurrentHashMap<>();

    Object read() {
        return map.get(object);
    }

    void write(Object key, Object object) {
        map.put(key, object);
    }
}

Do we need to make the map volatile so that writes of one thread are seen by the reader threads as soon as possible?

Is it possible that a put to the map in one thread is not seen or seen very late by a different thread?

Same question for HashMap.

Solution 1

If you can make it final then do that. If you cannot make it final then yes you would need to make it volatile. volatile applies to the field assignment and if it's not final then there is a possibility (at least per the JMM) that a write of the CHM field by one thread may not be visible to another thread. To reiterate, this is the ConcurrentHashMap field assignment and not using the CHM.

That being said, you should really make it final.

Do we need to make the map volatile so that writes of one thread are seen by the reader threads as soon as possible?

If the writes you speak of are done using the mutation methods of the CHM itself (like put or remove) you making the field volatile doesn't have an effect. All memory visibility guarantees are done within the CHM.

Is it possible that a put to the map in one thread is not seen or seen very late by a different thread? Same question for HashMap.

Not for the ConcurrentHashMap. If you are using a plain HashMap concurrently, don't. See: http://mailinator.blogspot.com/2009/06/beautiful-race-condition.html

Solution 2

volatile applies happens-before semantics on reads and writes to the corresponding variable.

A field may be declared volatile, in which case the Java Memory Model ensures that all threads see a consistent value for the variable (§17.4).

It has nothing to do with objects referenced by the variable's value. You're not modifying the variable, so you shouldn't* have any problems, unless(*) you're not safely publishing the Test object that is shared across threads.

As Lii suggests in the coments, assuming you don't take the right precautions, through final, volatile, or some other synchronization mechanism, the JMM allows a reference to an object to be made available before the object has been fully initialized by its constructor. As such, one of your threads could try to use the map field before it's been initialized (eg. it would see null). In that sense, the code could break.

Is it possible that a put to the map in one thread is not seen or seen very late by a different thread?

This is not possible, as the javadoc states, ConcurrentHashMap methods introduce appropriate memory barriers,

Retrievals reflect the results of the most recently completed update operations holding upon their onset. (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.

HashMap, however, is not a thread-safe type. volatile wouldn't help here either because it controls changes to the variable, not the object referenced by the variable. You'll need external synchronization to protect put and get calls to a HashMap.

Solution 3

There are 2 sub-questions here: visability of reference to a map and visability of values written to the map.

1. Visability of the reference to the map:

   Do we need to make the map...

should

safe publication

1. Provide access to the reference through a properly locked field (JLS 17.4.5)
2. Use static initializer to do the initializing stores (JLS 12.4) (not our case actually)
3. Provide access to the reference via a volatile field (JLS 17.4.5), or as the consequence of this rule, via the AtomicX classes like AtomicReference
4. Initialize the value as a final field (JLS 17.5)

So, in your case your "map" reference isn't published correctly. This may cause NullPointerException in Test.read() or/and Test.write() (it depends on which thread instantiates ConcurrentHashMap and puts it into "map" field). Correct code would be one of the following:

//1. Provide access to the reference through a properly locked field
class Test {

    ConcurrentHashMap map;

    synchronized void init(ConcurrentHashMap map) {
        this.map = map;
    }

    synchronized void read() {
        map.get(object);
    }

    synchronized void write() {
        map.put(key, object);
    }
}

// or
class Test {
    ReadWriteLock rwl = new ReentrantReadWriteLock();

    ConcurrentHashMap map;

    void init(ConcurrentHashMap map) {
        rwl.writeLock().lock();
        this.map = map;
        rwl.writeLock().release();
    }

    void read() {
        rwl.readLock().lock();
        try {
            map.get(object);
        } finally {
          rwl.readLock().release();
        }
    }

    void write() {
        rwl.writeLock().lock();
        try {
            map.put(key, object);
        } finally {
            rwl.writeLock().release();
        }
    }
}

// 3. Provide access to the reference via a volatile field
class Test {

    volatile ConcurrentHashMap map; // or AtomicReference<ConcurrentHashMap> map = new AtomicReference();

    void init(ConcurrentHashMap map) {
        this.map = map;
    }

    void read() {
        map.get(object);
    }

    void write() {
        map.put(key, object);
    }
}

// 4. Initialize the value as a final field
class Test {

    final ConcurrentHashMap map;

    Test(ConcurrentHashMap map) {
        this.map = map;
    }

    void read() {
        map.get(object);
    }

    void write() {
        map.put(key, object);
    }
}

Of course, you can use plain HashMap in case of p.1 (when you work with the properly locked field "map") instead of ConcurrentHashMap. But if you still want to use ConcurrentHashMap for better performance, the best way to publish your "map" correctly, as you see, is to make the field final.

Here is a nice article about safe publication from an Oracle guy, btw: http://shipilev.net/blog/2014/safe-public-construction/

2. Visability of values written to the map:

Is it possible that a put to the map in one thread is not seen or seen very late by a different thread?

No, if you don't get NPE (see p.1) or have published your map correctly, a reader always sees all changes produced by a writer, because a pair of ConcurrentHashMap.put/get produces appropriate memory barriers/Happens-Before edge.

Same question for HashMap

HashMap isn't thread safe at all. Methods HashMap.put/get work with internal state of the map in not thread-safe manner (non-atomic, no inter-thread visibility of changed state guaranteed), so, you may just corrupt state of the map. This means you must use an appropriate locking mechanism (synchronized sections, ReadWriteLock etc.) to work with HashMap. And, as result of locking, you achieve what you need - a reader always sees all changes produced by a writer because those locks produce memory barriers/Happens-Before edges.

Solution 4

No, you don't.

volatile means that the variable cannot be cached in a register, and so will always be "write-through" to memory. This means that one thread's change to a variable will be visible to other threads.

In this case, the variable is a reference to a Map. You use the same Map all the time, so you don't change the reference - rather you change the contents of that Map. (This is to say, the Map is mutable.) This also means that you can, and therefore should, make the reference to the Map final.

The ConcurrentHashMap differs from the HashMap in that you can typically safely read from it and write to it at the same time from different threads, without external locking. If you, however, want to be able to trust the size at any given point, do check-then-write operations or the like, you need to design that yourself.

View more solutions

Comments

[Kelvin Ng]

upvote for mentioning (final and constructor)[stackoverflow.com/questions/3974350/…

[Boann]

It would not be necessary to make the map field final or volatile (although it would still be a good idea) if something else already provides the necessary happens-before relationship between the assignment and reading of the field. For example, if the field is assigned before all threads that read it are created, or if all involved threads access some other shared volatile variable between the assignment and reading of the map field.

[jezg1993]

@Boann Though technically it is correct, I am not sure it addresses the OP's overall concern of why to define a java.util.concurrent field as volatile. The confusion with OP I believe is "If the class is thread safe why do I need to define it volatile?" You are right though, if some other operation can promise a happens-before ordering you wouldn't necessarily need it.

[Lii]

I really don't think this is correct. As John Vint writes you do need to make the field volatile or final. The ConcurrentHashMap is thread safe, but the field must be safely published to other threads. Check out the concept of unsafe publication, for example this.

[Bex]

@Lii in this case, it's a part of implicit construction of the object, so if the object is properly published, this map will be published as well. As you point out, it's not formally final, but it can't, currently change. If it could, that would be another matter. The point is that the state of the ConcurrentHashMap is thread safe, regardless of whether the reference to it is. But as a matter of fact, in the current example, the reference is indeed safe.

[Lii]

Then I think your answer needs a clear statement that the objects needs to be published in a safe way, and what that means. In its current form I think your answer is misleading people to believe that the code is safe with without any further synchronization between threads.

[Bex]

@Lii You mean beyond what is stated in the second paragraph of my answer? The question gives an example, and asks, for that example, if the member needs to be volatile. For this example, the plain answer is no, it does not. For another example, the answer may be different. Object initializations of members, such as the one given in this example, are thread safe and atomic i Java, partly because such references cannot escape during construction. Also note that the field is private. See also reasoning on "Effectively Immutable Objects" in Java Concurrency in Practice.

[Bex]

For this kind of initialization, there is no way a reference can leak. A constructor can leak a reference, but this kind of initialization has already happened by then.