Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: added hidden text to show DateFormat problems
Wiki Markup
According to the Java Language Specification \[[JLS 05|AA. Java References#JLS 05]\], section, 8.3.1.4 {{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 fields and not to the contents of objects that are declared {{volatile}}. A thread may not observe a recent write to the object's field from another thread.

Declaring an object {{volatile}} in order to ensure visibility of the most up-to-date object state does not work without the use of explicit 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, 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 object is thread-safe by design, then the field that will hold its reference may be declared as {{volatile}}. 

h2. Noncompliant Code Example (Arrays)

This noncompliant code example shows an array object (arrays are objects in Java) that is declared {{volatile}}. It appears that when, a value is written by a thread to one of the array elements, it will be visible to other threads immediately. 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. 

{code:bgColor=#FFcccc}
class Foo {
  volatile private int[] arr = new int[20];

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

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

  // ...
}
{code}

It is possible for one thread to set a new value to {{arr\[1\]}} while another thread attempts to read the value of {{arr\[1\]}}, with the result that the reading thread receives an inconsistent value for {{arr\[1\]}}.

In particular, this fails 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 themselves.


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 immediately 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 solution, the {{AtomicIntegerArray}} guarantees a happens-before relation between a thread that calls {{aia.set()}} and a thread that subsequently calls {{aia.get()}}.


h2. Compliant Solution (synchronization)

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

{code:bgColor=#ccccff}
class Foo {
  private int[] arr = new int[20];

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

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

In this solution, a thread that calls {{setFirst()}} grabs and releases the intrinsic lock on the {{this}} object. A thread that subsequently calls {{getFirst()}} grabs the same lock. The release and subsequent grab of an intrinsic lock establishes a happens-before relation between the two threads; 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 type {{Properties}} as {{volatile}}. The field 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 properties.getProperty(s);
  }

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

This class permits a race condition. If one thread calls {{get()}} while another calls {{put()}}, the first thread may receive a stale value, or an internally inconsistent value from the {{properties}} object because the operations within {{put()}} are nonatomic. Declaring the object {{volatile}} does not prevent this data race.

Even if the client thread sees the new reference to {{properties}}, the object state that it observes may change in the meantime. {mc} The Java Memory Model does not guarantee that the {{properties}} field will have been properly initialized when it is necessary. ==Let's not talk about initialization here=={mc} Because the object is not immutable, it is unsafe for use in a multi-threaded environment.


h2. Compliant Solution (immutable)

This compliant solution renders the {{Foo}} class immutable. Consequently, once it is properly constructed, no thread can modify {{properties}} and cause a race condition.

{code:bgColor=#ccccff}
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);
  }
}
{code}

The drawback of this solution is that the {{put()}} method cannot be accommodated if the goal is to ensure immutability. 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 tries to use the cheap read-write lock trick.  The {{properties}} field is {{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 employed on primitive types that require non-atomic functions to be performed on them, such as increment. However, the trick does not work on objects, because the {{volatile}} keyword does not extend to their members. Consequently, if one thread adds a property using {{put}}, there is no write to the {{properties}} object reference itself (just a write to its members). Consequently there is no [happens-before relation|BB. Definitions#happens-before order]  between the write and a subsequent read of a property. So a subsequent thread that tries to get the property may still get a stale value.


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 solely with intrinsic locks. 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-constructed 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. 

{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 made 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

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