Declaring an object volatile
to ensure visibility of the most up-to-date object state does not work without the use of explicit synchronization in cases where the object is not thread-safe.
In the absence of synchronization, the effect of declaring an object volatile
is that multiple threads which see the new object reference never see a partially initialized object. However, this holds only when the object is "effectively immutable" [[Goetz 07]], that is, its state cannot be directly or indirectly changed after the object is initialized or published.
Noncompliant Code Example
This noncompliant code example declares an instance field of type Map
as volatile
. The field can be mutated using a setter method putData()
.
public class Container<K,V> { volatile Map<K,V> map; public Container() { map = new HashMap<K,V>(); } public V get(Object k) { return map.get(k); } public void putData(K key, V value) { // Perform validation of value map.put(key, value); } } public class Client { public void doSomething() { Container con = new Container(); // This needs to see the fully constructed object not just the reference if (con.map != null) { // ... } } }
Even if the client thread sees the new reference, the object state that it observes may change in the meantime. Because the object is not effectively immutable, it is unsafe for use in a multi-threaded environment.
Compliant Solution (final)
This compliant solution declares the Map
instance as final
because the semantics of final
dictate that the object will be immediately visible after initialization.
public class Container<K,V> { final Map<K,V> map; public Container() { map = new HashMap<K,V>(); } public V get(Object k) { return map.get(k); } }
The obvious drawback of this solution is that the setter method cannot be accommodated if the goal is to ensure immutability. The Container
and Map
class are both effectively immutable in this case because the Map
instance can only be manipulated via one of the setters whilst there is none here. The words effectively immutable are used instead of immutable because the Map
instance is technically mutable but restricted here so that it has all the immutability properties. The Map
class does not have any static
methods that can be used to change its state, so the effective immutability property is enforced.
Compliant Solution (volatile)
It follows from the previous compliant solution that it is safe to declare the object volatile
because it is effectively immutable.
public class Container<K,V> { volatile Map<K,V> map; public Container() { map = new HashMap<K,V>(); } public V get(Object k) { return map.get(k); } }
Compliant Solution (synchronization)
This compliant solution uses explicit synchronization to ensure thread safety. It declares the object volatile
to guard retrievals that use the getter method. A synchronized setter method is used to set the value of the Map
object.
public class Container<K,V> { volatile Map<K,V> map; public Container() { map = new HashMap<K,V>(); } public V get(Object k) { return map.get(k); } public synchronized void putData(K key, V value) { // Perform validation of value map.put(key, value); } }
This compliant solution has the advantage that it can accommodate the setter method. Declaring the object as volatile
for safe publication using getter methods is cheaper in terms of performance, than declaring the getters as synchronized
. However, it is mandatory to synchronize the setter methods.
Risk Assessment
Failing to synchronize access to shared mutable data can cause different threads to observe different states of the object.
Rule |
Severity |
Likelihood |
Remediation Cost |
Priority |
Level |
---|---|---|---|---|---|
CON26- J |
medium |
probable |
medium |
P8 |
L2 |
Automated Detection
TODO
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
References
[[Goetz 07]] Pattern #2: "one-time safe publication"
FIO36-J. Do not create multiple buffered wrappers on an InputStream 09. Input Output (FIO) 09. Input Output (FIO)