CON11-J. Do not assume that declaring an object reference volatile guarantees visibility of its members

Section 8.3.1.4, "volatile Fields" of the Java Language Specification [[JLS 05]] states the following:

A field may be declared volatile, in which case the Java memory model (§17) ensures that all threads see a consistent value for the variable.

Notably, this applies only to primitive fields and immutable member objects. The visibility guarantee does not extend to mutable objects that are not thread-safe, even if their references are declared volatile. A thread may not observe a recent write from another thread to a member field of such an object. Declaring an object volatile to ensure visibility of its state does not work without the use of synchronization, unless the object is [immutable]. If the object is mutable and not thread-safe, other threads might see a partially-constructed object or an object in a (temporarily) inconsistent state [[Goetz 07]].

Technically, the object does not have to be strictly immutable to be used safely. If it can be proved that the member object is thread-safe by design, the field that will hold its reference may be declared as volatile. However, this approach to using volatile burdens maintainability and should be avoided.

Noncompliant Code Example (Arrays)

This noncompliant code example shows an array object (arrays are objects in Java) that is declared volatile.

final class Foo {
  volatile private int[] arr = new int[20];

  public int getFirst() {
    return arr[0];
  }

  public void setFirst(int n) {
    arr[0] = n;
  }

  // ...
}

The assumption that a value written to an array element by one thread is visible to other threads is incorrect, because the volatile keyword only makes the array reference visible and does not affect the actual data contained within the array. For example, when a thread assigns a new value to arr[1], another thread that is attempting to read the value of arr[1] might observe a stale value.

This happens because there is no [happens-before] relation between the thread that calls setFirst() and the thread that calls getFirst(). A happens-before relation exists between a thread that writes to a volatile variable and a thread that subsequently reads it. This code is neither writing to nor reading from a volatile variable.

Compliant Solution (AtomicIntegerArray)

This compliant solution uses the java.util.concurrent.atomic.AtomicIntegerArray class. Its set(index, value) method ensures that the write is atomic and the resulting value is visible to other threads. The other threads can retrieve a value from a specific index using the get(index) method.

final class Foo {
  private final AtomicIntegerArray atomicArray = new AtomicIntegerArray(20);

  public int getFirst() {
    return atomicArray.get(0);
  }

  public void setFirst(int n) {
    atomicArray.set(0, 10);
  }

  // ...
}

AtomicIntegerArray guarantees a [happens-before] relation between a thread that calls atomicArray.set() and a thread that subsequently calls atomicArray.get(). However, if a thread calls getFirst() first, it sees the default value of the atomic integer (0).

Compliant Solution (Synchronization)

To ensure visibility, accessor methods may synchronize access, while performing operations on non-volatile elements of an array that is declared volatile. Note that the code is thread-safe, even though the array reference is not volatile.

final class Foo {
  private int[] arr = new int[20];

  public synchronized int getFirst() {
    return arr[1];
  }

  public synchronized void setFirst(int n) {
    arr[1] = n;
  }
}

Synchronization establishes a [happens-before] relation between the thread that calls setFirst() and the thread that subsequently calls getFirst(). Consequently, the array element set by setFirst() is guaranteed to be visible to getFirst().

Noncompliant Code Example (Mutable Object)

This noncompliant code example declares an instance field of the Properties type as volatile. The instance of the Properties object can be mutated using the put() method, and that makes the properties field mutable.

final class Foo {
  private volatile Properties properties;

  public Foo() {
    properties = new Properties();
    // Load some useful values into properties
  }

  public String get(String s) {
    return properties.getProperty(s);
  }

  public void put(String key, String value) {
    // Validate the values before inserting
    properties.setProperty(key, value);
  }
}

If one thread calls get() while another calls put(), the first thread may receive a stale or internally inconsistent value from the Properties object, because the operations within put() modify the state of the Properties object instance. Declaring the object volatile does not prevent this data race.

There is no time of check, time of use (TOCTOU) vulnerability in put(), despite the presence of the validation logic, because the validation is performed on the immutable value argument and not the shared Properties instance.

Compliant Solution (Immutable)

This compliant solution renders the properties field immutable by not providing a put() method. Consequently, once it is properly constructed, no thread can modify the state of the Properties object instance and cause a data race.

final class Foo {
  private final Properties properties;

  public Foo() {
    properties = new Properties();
    // Load some useful values into properties
  }

  public String get(String s) {
    return properties.getProperty(s);
  }
}

The Foo class is also [immutable], because all its fields are final and the properties field is being published safely. The disadvantage of making Foo immutable is that the put() method can no longer be accommodated. When that is unacceptable, the get() and put() methods must use some form of synchronization to prevent data races.

Noncompliant Code Example (Volatile-Read, Synchronized-Write)

This noncompliant code example attempts to use the volatile-read, synchronized-write technique described by Goetz [[Goetz 07]]. The properties field is declared volatile to synchronize reads and writes of the field. The put() method is also synchronized to ensure that its statements are executed atomically.

final class Foo {
  private volatile Properties properties;

  public Foo() {
    properties = new Properties();
    // Load some useful values into properties
  }

  public String get(String s) {
    return properties.getProperty(s);
  }

  public synchronized void put(String key, String value) {
    // Validate the values before inserting
    // ...
    properties.setProperty(key, value);
  }
}

The volatile-read, synchronized-write technique uses synchronization to preserve atomicity of compound operations such as increment and provides faster access times for atomic reads. However, it does not work with mutable objects, because the visibility of volatile does not extend to their members. Consequently, if one thread adds a property using put, other threads may not observe this change. There is no [happens-before relation] between the write and a subsequent read of the property.

This technique is also discussed in [CON01-J. Ensure that compound operations on shared variables are atomic].

Compliant Solution (Synchronized)

This compliant solution uses method synchronization to ensure thread-safety.

final class Foo {
  private final Properties properties;

  public Foo() {
    properties = new Properties();
    // Load some useful values into properties
  }

  public synchronized String get(String s) {
    return properties.getProperty(s);
  }

  public synchronized void put(String key, String value) {
    // Validate the values before inserting
    properties.setProperty(key, value);
  }
}

Note that the properties field is not declared volatile, because this solution achieves thread-safety using synchronization. The field is declared final so that its reference is not published when it is in a partially initialized state (see [CON26-J. Do not publish partially initialized objects] for more information).

Noncompliant Code Example (Mutable Sub-Object)

This noncompliant code example declares the FORMAT field volatile. However, the field stores a reference to a mutable object, java.text.DateFormat.

final class DateHandler {
  private static volatile DateFormat FORMAT =
    DateFormat.getDateInstance(DateFormat.MEDIUM);

  public static Date parse(String str) throws ParseException {
    return FORMAT.parse(str);
  }
}

In the presence of multiple threads, this results in subtle thread-safety issues, because DateFormat is not thread-safe [[API 06]]. For example, a thread might observe correctly formatted output for an arbitrary date when it invokes parse() with a known date.

Compliant Solution (Instance Per Call/Defensive Copying)

This compliant solution creates and returns a new DateFormat instance for every invocation of the parse() method. [[API 06]]

final class DateHandler {
  public static Date parse(String str) throws ParseException {
    DateFormat format = DateFormat.getDateInstance(DateFormat.MEDIUM);
    return format.parse(str);
  }
}	

This solution does not violate [OBJ11-J. Defensively copy private mutable class members before returning their references], because the class no longer contains internal mutable state, but rather only a local field, format.

Compliant Solution (Synchronization)

This compliant solution synchronizes statements within the parse() method, and consequently, the DateHandler class is thread-safe [[API 06]]. There is no requirement for declaring the FORMAT field volatile.

final class DateHandler {
  private static DateFormat FORMAT =
    DateFormat.getDateInstance(DateFormat.MEDIUM);

  public static Date parse(String str) throws ParseException {
    synchronized (FORMAT) {
      return FORMAT.parse(str);
    }
  }
}

Compliant Solution (ThreadLocal Storage)

This compliant solution uses a ThreadLocal object to store one DateFormat instance per thread.

final class DateHandler {
  private static final ThreadLocal<DateFormat> df = new ThreadLocal<DateFormat>() {
    @Override protected DateFormat initialValue() {
      return DateFormat.getDateInstance(DateFormat.MEDIUM);
    }
  };
  // ...
}

Risk Assessment

Assuming that declaring a field volatile guarantees visibility of the members of the referenced object can cause threads to observe stale values of the members.

Automated Detection

TODO

Related Vulnerabilities

Any vulnerabilities resulting from the violation of this rule are listed on the CERT website.

References

[[Goetz 07]] Pattern #2: "one-time safe publication"
[[Miller 09]] Mutable Statics
[[API 06]] Class java.text.DateFormat
[[JLS 05]]

[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!]