CON26-J. Synchronize access to shared mutable objects to ensure visibility

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 (mutable object)

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>();	
    // Fill map with useful data
  }

  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 map as volatile if it is effectively immutable.

public class Container<K,V> {
  volatile Map<K,V> map;

  public Container() {
    map = new HashMap<K,V>();	
    // Fill map with useful data
  }

  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.

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)