Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

Note that declaring the variables as volatile does not resolve the issue because these compound operations involve reads and writes of multiple variables.

Noncompliant Code Example (

...

Overflow Check, Atomic Integer Fields)

The issues described in the previous noncompliant code example can also occur when the fields a and b fields of type int are replaced with atomic integers, as shown in the noncompliant code example below.

Code Block
bgColor#FFcccc
final class Adder {
  private final AtomicInteger a = new AtomicInteger();
  private final AtomicInteger b = new AtomicInteger();

  public int getSum() throws ArithmeticException {
    // Check for integer overflow
    if (b.get() > 0 ? a.get() > Integer.MAX_VALUE - b.get() : a.get() < Integer.MIN_VALUE - b.get()) {
      throw new ArithmeticException("Not in range");
    }
    return a.get() + b.get(); // or, return a.getAndAdd(b.get());
  }

  public void setValues(int a, int b) {
    this.a.set(a);
    this.b.set(b);
  }
}

For example, when a thread is executing setValues(), another thread may invoke getSum() and retrieve an incorrect result. Furthermore, in the absence of synchronization, there are data races occur in the check for integer overflow. For instance, a thread can call setValues() after a second thread that is attempting to add the numbers has read a, but before it has read b. In this case, the second thread will get an improper sum.

Even worse, a thread can call setValues() after a second thread has verified that overflow will not occur, but before the second thread reads the values to be added. This That would cause the second thread to add two values without checking for overflow, yielding an incorrect sum. Even though a check for integer overflow is installed, it is ineffective because of the time-of-check-time-of-use (TOCTOU) condition between the overflow check and the addition operation.

Compliant Solution (

...

Addition,

...

Synchronized)

This compliant solution synchronizes the setValues() and getSum() methods to ensure atomicity.

...

Unlike the noncompliant code examples, if a and b currently have the value 0, and one thread calls getSum() while another calls setValues(1, 1), getSum() may return return 0, or 2, depending on which thread obtains the intrinsic lock first. The locking strategy guarantees that getSum() never returns the unacceptable value 1.

This compliant solution also ensures that there is no TOCTOU condition between checking for overflow and adding the fields.

...

If operations on shared variables are not atomic, unexpected results may can be produced. For example, there information can be inadvertent information disclosure as disclosed inadvertently because one user may be able to can receive information about other users.

...

Dynamic analysis tools with a Java concurrency focus, such as SureLogic Flashlight and Coverity Dynamic Analysis will uncover the race conditions shown in the noncompliant code examples above. To accomplish thisdo that, however, these those tools would have to must observe the noncompliant code being called by two or more threads. Such , such as in an integration or stress-test environment. These The tools use a dynamic lockset analysis to observe race conditions that occur as the program runs. This analysis intersects the set of locks that are observed to be held when each piece of shared state in the program is accessed. If the lockset for a piece of shared state is empty then a race condition , the tool may have been observed a race condition and the tool reports this that to the user.

HeurisiticsHeuristics-based static analysis tools, such as FindBugs and PMD, do not detect problems with the noncompliant code examples shown above without some "hint" that the program code is intended to be thread-safe. For example, consider the compliant code below where the use of a synchronized method is a hint to the analysis tool that the class is intended meant to be used concurrently.

Code Block
java
java
public class Foo {
  private boolean flag = true;

  public synchronized boolean toggleAndGet() {
    flag ^= true; // Same as flag = !flag;
    return flag;
  }
}

FindBugs and PMD will not report a warning about this implementation as because they do not note any problems.

SureLogic JSure, an analysis-based verification tool, will complain that the lock is unknown to the tool and ask the user to annotate what state the lock protects, i.e.. In other words, the tool wants to know the locking policy that the programmer intends for this class. To express this intent, the programmer adds two annotations:

...

The @RegionLock annotation creates a locking policy, named FlagLock, that specifies that reads and writes to the flag field flag are to be guarded by a lock on the receiver, i.e.that is, this. The second annotation, @Promise is used to place an annotation on the default constructor generated by the compiler. The @Unique("return") annotation promises that the receiver is not aliased during object construction, i.e.that is, that a race condition cannot occur during construction. (CON14-J. Do not let the "this" reference escape during object construction provides further details.) If the constructor was explicit in the code then , the annotations would be as follows:

Code Block
java
java
@RegionLock("FlagLock is this protects flag")
public class Foo {
  private boolean flag;

  @Unique("return")
  public Foo() {
    flag = true;
  }

  public synchronized boolean toggleAndGet() {
    flag ^= true; // Same as flag = !flag;
    return flag;
  }
}

The JSure verification tool provides a strong assurance that the annotated model holds for all possible executions of the program. If the below following noncompliant code is later added to the class, JSure will report the violation of the locking policy to the user.

Code Block
java
java
  public boolean getValue() {
    return flag;
  }

then JSure will report the violation of the locking policy to the user.

If the noncompliant getValue() method shown above is defined in the code for Foo, then FindBugs can also report a problem, again if the locking model is annotated. However, it uses a different annotation than JSure.:

Code Block
java
java
public class Foo {
  @GuardedBy("this")
  private boolean flag = true;

  public synchronized boolean toggleAndGet() {
    flag ^= true; // Same as flag = !flag;
    return flag;
  }

  public boolean getValue() {
    return flag;
  }
}

With the @GuardedBy annotation in place, and only with this annotation in place, FindBugs reports that the field is not guarded against concurrent access in the getValue() method.

Related Vulnerabilities

Search for The vulnerabilities resulting from the violation of this rule are listed on the CERT website.

References

Wiki Markup
\[[API 06|AA. Java References#API 06]\] Class AtomicInteger
\[[JLS 05|AA. Java References#JLS 05]\] [Chapter 17, Threads and Locks|http://java.sun.com/docs/books/jls/third_edition/html/memory.html], sectionSection 17.4.5 Happens-beforeBefore Order, sectionSection 17.4.3 Programs and Program Order, sectionSection 17.4.8 Executions and Causality Requirements
\[[Tutorials 08|AA. Java References#Tutorials 08]\] [Java Concurrency Tutorial|http://java.sun.com/docs/books/tutorial/essential/concurrency/index.html]
\[[Lea 00|AA. Java References#Lea 00]\] Sections,Section 2.2.7 The Java Memory Model, Section 2.1.1.1 Objects and locks
\[[Bloch 08|AA. Java References#Bloch 08]\] Item 66: Synchronize access to shared mutable data
\[[Goetz 06|AA. Java References#Goetz 06]\] 2.3. "Locking"
\[[MITRE 09|AA. Java References#MITRE 09]\] [CWE ID 667|http://cwe.mitre.org/data/definitions/667.html] "Insufficient Locking,", [CWE ID 413|http://cwe.mitre.org/data/definitions/413.html] "Insufficient Resource Locking,", [CWE ID 366|http://cwe.mitre.org/data/definitions/366.html] "Race Condition within a Thread,", [CWE ID 567|http://cwe.mitre.org/data/definitions/567.html] "Unsynchronized Access to Shared Data"

...