CODEWITHSUNDEEP
javaconcurrencynestedthread-safety
flakes
flakes

Reputation: 23674

Thread safety of concurrency safe containers where values are mutable

Say I have a map which ensures thread safety (full concurrency of retrievals) ConcurrentHashMap<String, Foo> where the values being retrieved are some mutable objects

class Foo { 
    public Object bar;
}

Foo values will only ever be constructed by a single thread, added once to the map, and then never modified. The operation might look like this:

Foo foo = new Foo();
foo.bar = "Test";
concurrentMap.add("key", foo);

In a separate thread work is performed by looking at values in the map (assuming here that the values is properly set beforehand)

System.out.println(concurrentMap.get("key").bar);

Is there any issue with accessing Foo::bar in this situation? Is there any scenario where this can fail to execute as expected?

Upvotes: 1

Views: 212

Answers (2)

Michael P
Michael P

Reputation: 2087

Read only operations are thread safe by definition. A race condition or a data race would happen only if there's at least one thread that modifies the data.

So if you put the value into the map, before you create the thread that reads the value from the map, then the operation is completely safe. And if you never ever modify bar, or its references inside foo, you should be fine as well. You don't even need a concurrent map in this case. A regular map will just do.

However you may get unexpected results if bar is modified or if the reference to bar is modified inside foo. Here is an example of something that may go wrong. Lets assume bar is a long.

class Foo { 
    public long bar;
}

And you have thread 1 doing:

Foo foo = concurrentMap.get("key");
.....
..... /// some code
System.out.println(foo.bar);

And there's another thread in the background that does this:

Foo foo = concurrentMap.get("key");
.....
long newBar = foo.bar + 1;
foo.bar = newBar;

Here you have a straight out race condition.

Now if thread 2 actually just does this instead :

 Foo foo = concurrentMap.get("key");
 .....
 long newBar = rand.nextLong();
 foo.bar = newBar;

You don't have a race condition, but you have a data race instead, because a long is 64 bit, and the compiler may execute the assignment to a long and double as two operations.

There are many more scenarios where it can go wrong, and it's really hard to reason about them, without being very careful

So at the very least, you should make bar volatile, like this:

class Foo { 
    public volatile Object bar;
}

And be very careful about how you operate on bar and what's inside, if it's an actual object of some class.

If you'd like to learn more about data races and understand race conditions a bit more in depth you should check out this excellent course https://www.udemy.com/java-multithreading-concurrency-performance-optimization/?couponCode=CONCURRENCY

It has a section about that, that explains it really well, with really good examples.

You should also check out the official Java memory model documentation https://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.4

I hope it helps

Upvotes: 1

Michal
Michal

Reputation: 1010

It should work as putting values to and getting of out the concurrent hashmap should establish happens-before relationship (see Is ConcurrentHashMap.get() guaranteed to see a previous ConcurrentHashMap.put() by different thread?). Consumer thread should see the bar value as it was before the map insertion.

Personally I'd try hard to make Foo (and bar) objects immutable (why not if they never change?).

Upvotes: 1

Related Questions