Thread safety issues in reference assignment using method call

Question

I have the following code:

class SomeClass {
    private Map<String, String> someMap = null;

    public String getValue(String key) {
        if (someMap == null) {
            someMap = initialize(); // initialize() is some method which returns a proper map.
        }
        return someMap.get(key);
    }
}

Assuming I don't care about someMap getting initialized more than once, are there any other thread-safety issues that I need to be concerned about here.

As per what-operations-in-java-are-considered-atomic , Reference assignment is certainly atmoic. Is the assignment to someMap guaranteed to happen after the method call to initialize() (seems logical to me). Is there any possibility that a thread can see a partially constructed someMap. Does it matter if instead of a map, I have some other type of object.

score 4 · Accepted Answer · answered May 22 '14 at 17:46

4

The code is not thread-safe. If two threads invoke the method getValue on the same object, it is possible, that one thread sees a partially created someMap.

In order to avoid this problem, you have to remove the data races. The simplest solution is to declare someMap as volatile. The simple rule to remember is: If the code contains no data races, then all executions will appear to be sequentially consistent.

answered May 22 '14 at 17:46

nosid

48,932
13
112
139

I don't understand this. Wouldn't the assignment to `someMap` happen after `initialize` returns, as the last operation inside the `if` block. Given that reference assignment is atmoic, how can the map assigned to `someMap` be partially created. – Vivek May 22 '14 at 17:54
@Vivek: If we are talking only about one thread, then it's true that the assignment to `someMap` happens after `initialize` returns. However, if we talk about several threads, that's only true if the execution is _sequentially consistent_. Take a look at the Wikipedia article on [Sequential Consistency](http://en.wikipedia.org/wiki/Sequential_consistency). – nosid May 22 '14 at 17:59
Thanks, got it now. If I remember correctly, making the variable volatile forces all the local writes to be flushed to the main memory before the assignment to the variable. – Vivek May 22 '14 at 18:06
@Vivek: You are only writing once (or a few times). The overhead for write operations shouldn't matter, and the overhead for _volatile reads_ is low. – nosid May 22 '14 at 18:08

score 0 · Answer 2 · edited May 23 '17 at 10:26

0

Its not a Thread Safe even if you put volatile because if two threads are called getValue method at a same time, the following scenario can happen

1) Thread 1 & Thread 2 check if (someMap == null) , both will pass and try to reinitialise.

2) Both will reinitialize the reference. Thread 1 return the old reference which one is override by thread 2 .

3) So please refer the link Double Checked Locking in Singleton

edited May 23 '17 at 10:26

Community

1
1

answered May 27 '14 at 18:34

Siva Kumar

1,983
3
14
26

As i mentioned in the question, I don't care about someMap being initialized multiple times. So your scenario does not apply. – Vivek Jun 01 '14 at 09:43

Thread safety issues in reference assignment using method call

2 Answers2