Reputation: 725
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
.
Upvotes: 53
Views: 14306
Reputation: 280102
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
.
Upvotes: 12
Reputation: 40256
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
Upvotes: 51
Reputation: 1587
There are 2 sub-questions here: visability of reference to a map and visability of values written to the map.
Do we need to make the map...
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/
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.
Upvotes: 8
Reputation: 2925
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.
Upvotes: 5