Page History

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

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

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

  // ...
}

final  {{volatile}} Fields:
{quote}
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.
{quote}

Notably, this applies only to primitive fields and immutable member objects. The visibility guarantee does not extend to mutable, thread-unsafe objects even if their references are declared {{volatile}}. A thread may not observe a recent write to a member object's field from another thread.

Declaring an object {{volatile}} to ensure visibility of the most up-to-date object state does not work without the use of synchronization, unless the object is [immutable|BB. Definitions#immutable].

In the absence of synchronization, the effect of declaring a field {{volatile}} is that when one thread sets the field to a new value, other threads can see the new object reference immediately. If the referenced object is immutable, this has the effect that other threads also see a consistent view of the state of the object. However, if the object is mutable and thread-unsafe, other threads may see a partially-constructed object, or an object in a (temporarily) inconsistent state \[[Goetz 07|AA. Java References#Goetz 07]\]. Declaring the object {{volatile}} does not prevent this issue. 

Technically the object does not have to be strictly immutable. 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 burdens maintainability and should be avoided as far as possible.

h2. Noncompliant Code Example (Arrays)

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

{code:bgColor=#FFcccc}
class Foo {
  volatileprivate privatefinal int[]AtomicIntegerArray arratomicArray = 
    new int[AtomicIntegerArray(20]);

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

  public void setFirst(int n) {
    arr[0] = natomicArray.set(0, 10);
  }

  // ...
}
{code}

It appears that when a value is written by a thread to one of the array elements, it becomes instantly visible to other threads. This is misleading because the {{volatile}} keyword just makes the array reference visible to all threads 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\]}}, may observe an inconsistent value.

This happens because there is no [happens-before|BB. Definitions#happens-before order] relation between the thread that calls {{setFirst()}} and the thread that calls {{getFirst()}}.  Normally a happens-before relation exists between a thread that writes to a volatile variable and a thread that subsequently reads it. But this code is neither writing to nor reading from a volatile variable. The array's 'volatility' applies only to the array reference, not to the array elements.


h2. Compliant Solution ({{AtomicIntegerArray}})

This compliant solution suggests using the {{java.util.concurrent.atomic.AtomicIntegerArray}} concurrency utility. Using its {{set(index, value)}} method ensures that the write is atomic and the resulting value is made visible to other threads. The other threads can retrieve a value from a specific index by using the {{get(index)}} method.

{code:bgColor=#ccccff}
class Foo {
  AtomicIntegerArray aia = new AtomicIntegerArray(5);

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

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

  // ...
}
{code}

In this compliant solution, the {{AtomicIntegerArray}} guarantees a happens-before relation between a thread that calls {{aia.set()}} and a thread that subsequently calls {{aia.get()}}. However, if a thread calls {{getFirst()}} first, it sees the default value of the atomic integer (0).


h2. Compliant Solution (synchronization)

To ensure visibility, accessor methods may synchronize access while performing operations on nonvolatile elements of an array which is declared {{volatile}}. Note that the array need not be {{volatile}} for the code to be thread-safe.

{code:bgColor=#ccccff}

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;
  }
}

final class Foo {
  private volatile Map<String, String> map;
 
  public Foo() {
    map = new HashMap<String, String>();
    // Load some useful values into map
  }
 
  public String get(String s) {
    return map.get(s);
  }
 
  public void put(String key, String value) {
    // Validate the values before inserting
    if (!value.matches("[\\w]*")) {
      throw new IllegalArgumentException();
    }
    map.put(key, value);
  }
}

final class Foo {
  private volatile Map<String, String> map;
  public Foo() {
    map = new HashMap<String, String>();
    // Load some useful values into map
  }
  public String get(String s) {
    return map.get(s);
  }
  public synchronized void put(String key, String value) {
    // Validate the values before inserting
    if (!value.matches("[\\w]*")) {
      throw new IllegalArgumentException();
    }
    map.put(key, value);
  }
}

final class Foo {
  private int[] arr = new int[20];
final Map<String, String> map;
  public synchronized int getFirstFoo() {
     return arr[1]map = new HashMap<String, String>();
  }
  // Load some useful values into map
  }
  public synchronized voidString setFirstget(intString ns) {
    arr[1] = nreturn map.get(s);
  }
{code}

Synchronization establishes apublic happens-beforesynchronized relationvoid between the thread that calls {{setFirst()}} and the thread that subsequently calls {{getFirst()}}. Consequently, the array's element set by {{setFirst()}} is guaranteed to be visible to {{getFirst()}}.


h2. Noncompliant Code Example (Mutable object)

This noncompliant code example declares an instance field of put(String key, String value) {
    // Validate the values before inserting
    if (!value.matches("[\\w]*")) {
      throw new IllegalArgumentException();
    }
    map.put(key, value);
  }
}

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

  public static java.util.Date parse(String str) 
      throws ParseException type {{Properties}} as {{volatile}}. The referenced object can be mutated using the {{put()}} method. This makes objects of class {{Foo}} mutable.

{code:bgColor=#FFcccc}
class Foo {
  private volatile Properties properties;

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

  public String get(String s) {
    return propertiesformat.getPropertyparse(sstr);
  }

  public void put(String key, String value) {
    // Perform validation of value before inserting
    properties.setProperty(key, value)}

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

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

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

final class DateHandler {
  private static final ThreadLocal<DateFormat> format = 
    new ThreadLocal<DateFormat>() {
    @Override protected DateFormat initialValue;
  }
}
{code}

If one thread calls {{get()}} while another calls {{put()}}, the first thread may receive a stale, or an 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.

{mc} I don't think anything about partially constructed objects is required here {mc}


h2. Compliant Solution (immutable)

This compliant solution renders the {{Foo}} class immutable. Consequently, once it is properly constructed, no thread can modify the state of the {{Properties}} object instance and cause a data race.

{code:bgColor=#ccccff}
class Foo {
  private final Properties properties;

  public Foo() {
    properties = newreturn Properties(DateFormat.getDateInstance(DateFormat.MEDIUM);
    // Load some useful values into properties}
  };

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

The shortcoming of making {{Foo}} immutable is that the {{put()}} method can no longer be accommodated. The {{Foo}} class is [immutable|BB. Definitions#immutable] because all its fields are {{final}} and the {{properties}} field is being safely published.


h2. Noncompliant Code Example (cheap read-write lock)

This noncompliant code example attempts to use the cheap read-write lock trick \[[Goetz 07|AA. Java References#Goetz 07]\].  The {{properties}} field is declared {{volatile}} in order to synchronize reads and writes. The non-atomic {{put()}} method is synchronized as well.

{code:bgColor=#ffcccc}
public 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) {
    // Perform validation of value
    properties.setProperty(key, value);
  }
}
{code}

The cheap read-write lock trick is often used with primitive types that require nonatomic operations to be performed on them, such as increment. However, the trick does not work with objects, because the visibility of {{volatile}} does not extend to their members. Consequently, if one thread adds a property using {{put}}, it may not be visible to other threads. There is no [happens-before relation|BB. Definitions#happens-before order]  between the write and a subsequent read of the property.


h2. Compliant Solution (synchronized)

This compliant solution uses method synchronization to ensure thread safety. 

{code:bgColor=#ccccff}
public 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) {
    // Perform validation of value
    properties.setProperty(key, value);
  }
}
{code}

Note that the {{properties}} field is not declared {{volatile}} because this solution achieves thread-safety using syncronization. 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]). 

h2. Noncompliant Code Example (mutable sub-object)

This noncompliant code example declares the field {{FORMAT}} as {{volatile}}. However, the field stores a reference to a mutable object, {{DateFormat}}. 

{code:bgColor=#FFcccc}
class DateHandler {
  private static volatile DateFormat FORMAT =
    DateFormat.getDateInstance(DateFormat.MEDIUM);

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

In the presence of multiple threads, this results in subtle thread safety issues because {{DateFormat}} is not thread-safe \[[API 06|AA. Java References#API 06]\]. For instance, a thread may observe correctly formatted output for a completely different date when it invokes {{parse()}} with a known date. 

{mc}
// Calls DateHandler, demo code
public class DateCaller implements Runnable {
  public void run(){
    try {
      System.out.println(DateHandler.parse("Jan 1, 2010"));	
    } catch (ParseException e) {
  }	

  public static void main(String[] args) {
    for(int i=0;i<10;i++)
      new Thread(new DateCaller()).start();
  }
}
{mc} 


h2. 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|AA. Java References#API 06]\]

{code:bgColor=#ccccff}
class DateHandler {
  public static Date parse(String str) throws ParseException {
    DateFormat format = DateFormat.getDateInstance(DateFormat.MEDIUM);
    return format.parse(str);
  }
}	
{code}

This 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 a local field, {{format}}. 


h2. Compliant Solution (synchronization)

This compliant solution synchronizes the {{parse()}} method and consequently, the class {{DateHandler}} is thread-safe \[[API 06|AA. Java References#API 06]\]. There is no requirement for declaring the {{FORMAT}} field as {{volatile}}. 

{code:bgColor=#ccccff}
class DateHandler {
  private static DateFormat FORMAT =
    DateFormat.getDateInstance(DateFormat.MEDIUM);

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

h2. Compliant Solution ({{ThreadLocal}} storage)

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

{code:bgColor=#ccccff}
class DateHandler {
  private static final ThreadLocal<DateFormat> df = new ThreadLocal<DateFormat>() {
    protected DateFormat initialValue() {
      return DateFormat.getDateInstance(DateFormat.MEDIUM);
    }
  };
  // ...
}
{code}

h2. Risk Assessment

Assuming that declaring a field {{volatile}} guarantees visibility of the members of the referenced object may lead threads to observe stale values of the members.

|| Rule || Severity || Likelihood || Remediation Cost || Priority || Level ||
| CON11-J | medium | probable | medium | {color:#cc9900}{*}P8{*}{color} | {color:#cc9900}{*}L2{*}{color} |

h3. Automated Detection

TODO

h3. Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the [CERT website|https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+CON11-J].

h2. References

\[[Goetz 07|AA. Java References#Goetz 07]\] Pattern #2: "one-time safe publication"
\[[Miller 09|AA. Java References#Miller 09]\] Mutable Statics
\[[API 06|AA. Java References#API 06]\] Class {{java.text.DateFormat}}
\[[JLS 05|AA. Java References#JLS 05]\]

----
[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!|FIO36-J. Do not create multiple buffered wrappers on an InputStream]&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!|09. Input Output (FIO)]&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!|09. Input Output (FIO)]
// ...
}