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

According to the Java Language Specification [[JLS 2005]], Section 8.3.1.4, "volatile Fields"

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 non-thread-safe mutable objects, 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 2007]].

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

Noncompliant Code Example (Arrays)

This noncompliant code example shows an array object that is declared volatile.

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

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

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

  // ...
}

Values assigned to an array element by one thread, for example, by calling setFirst(), might not be visible to another thread calling getFirst() because the volatile keyword only makes the array reference visible and does not affect the actual data contained within the array.

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

Compliant Solution (AtomicIntegerArray)

To ensure that the writes to array elements are atomic and the resulting values are visible to other threads, this compliant solution uses the AtomicIntegerArray class defined in java.util.concurrent.atomic.

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] relationship between a thread that calls atomicArray.set() and a thread that subsequently calls atomicArray.get().

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 non-volatile.

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

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

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

Synchronization establishes a [happens-before] relationship between the thread that calls setFirst() and the thread that subsequently calls getFirst(), guaranteeing visibility.

Noncompliant Code Example (Mutable Object)

This noncompliant code example declares the Properties instance field 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
    if (!value.matches("[\\w]*")) {
      throw new IllegalArgumentException();
    }
    properties.setProperty(key, value);
  }

Interleaved calls to get() and put() may result in internally inconsistent values being retrieved from the Properties object because the operations within put() modify its state. Declaring the object volatile does not eliminate 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.

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

This noncompliant code example attempts to use the volatile-read, synchronized-write technique described by Goetz [[Goetz 2007]]. The properties field is declared volatile to synchronize its reads and writes. 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
    if (!value.matches("[\\w]*")) {
      throw new IllegalArgumentException();
    }
    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 object references does not extend to object members. Consequently, there is no [happens-before] relationship between the write and a subsequent read of the property.

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

Compliant Solution (Synchronized)

This compliant solution uses method synchronization to guarantee visibility.

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
    if (!value.matches("[\\w]*")) {
      throw new IllegalArgumentException();
    }
    properties.setProperty(key, value);
  }
}

The properties field does not need to be volatile because the methods are synchronized. The field is declared final so that its reference is not published when it is in a partially initialized state (see guideline TSM03-J. Do not publish partially initialized objects for more information).

Noncompliant Code Example (Mutable Sub-Object)

In this noncompliant code example, the volatile format field is used to store 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);
  }
}

Because DateFormat is not thread-safe [[API 2006]], the parse() method might return a value for Date that does not correspond to the str argument.

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 2006]]

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

This solution does not violate guideline [OBJ11-J. Defensively copy private mutable class members before returning their references] because the class no longer contains internal mutable state.

Compliant Solution (Synchronization)

This compliant solution synchronizes statements within the parse() method, making DateHandler thread-safe [[API 2006]].

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 create a separate DateFormat instance per thread.

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

Risk Assessment

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

Automated Detection

TODO

Related Vulnerabilities

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

References

[[Goetz 2007]] Pattern #2: "one-time safe publication"
[[Miller 2009]] Mutable Statics
[[API 2006]] Class java.text.DateFormat
[[JLS 2005]]

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