CON04-J. Ensure that calls to chained methods are atomic

Method chaining is a convenience mechanism that allows multiple method invocations on the same object to occur in a single statement. A method chaining implementation consists of a series of methods that return the this reference. This allows a caller to invoke methods in a chain by performing the next method invocation on the return value of the previous method in the chain.

While the methods used in method chaining may be atomic, a chain of method calls is inherently non-atomic. Consequently methods that are involved in method chaining should not be concurrently invoked unless the caller provides sufficient locking as illustrated in VNA03-J. Do not assume that a group of calls to independently atomic methods is atomic.

Noncompliant Code Example

Method chaining is a useful design pattern for building an object and setting its optional fields. A class that supports method chaining provides several setter methods that each return the this reference. However, if accessed concurrently, a thread may observe shared fields to contain inconsistent values. This noncompliant code example shows the JavaBeans pattern which is not thread-safe.

final class USCurrency {
  // Change requested, denomination (optional fields)
  private int quarters = 0;
  private int dimes = 0;
  private int nickels = 0;
  private int pennies = 0;

  public USCurrency() {}

  // Setter methods 
  public USCurrency setQuarters(int quantity) { 
    quarters = quantity; 
    return this;
  } 
  public USCurrency setDimes(int quantity) { 
    dimes = quantity; 
    return this;
  }
  public USCurrency setNickels(int quantity) { 
    nickels = quantity;
    return this;
  }
  public USCurrency setPennies(int quantity) { 
    pennies = quantity;
    return this;
  }
}

// Client code:
private final USCurrency currency = new USCurrency(); 
// ...

new Thread(new Runnable() {
  @Override public void run() {    
    currency.setQuarters(1).setDimes(1);
  }
}).start();

new Thread(new Runnable() {
  @Override public void run() {    
    currency.setQuarters(2).setDimes(2);
  }
}).start();

The JavaBeans pattern uses a no-argument constructor and a series of parallel setter methods to build an object. This pattern is not thread-safe and can lead to inconsistent object state if the object is modified concurrently. In this noncompliant code example, the client constructs a USCurrency object and starts two threads that use method chaining to set the optional values of the USCurrency object. This might result in the currency object being left in an inconsistent state, for example, with two quarters and one dime or one quarter and two dimes.

Compliant Solution

This compliant solution uses the variant of the Builder pattern [[Gamma 95]] suggested by Bloch [[Bloch 08]] to ensure thread safety and atomicity of object creation.

final class USCurrency {
  private final int quarters;
  private final int dimes;
  private final int nickels;
  private final int pennies;

  public USCurrency(Builder builder) {
    this.quarters = builder.quarters;
    this.dimes = builder.dimes;
    this.nickels = builder.nickels;
    this.pennies = builder.pennies;
  }

  // Static class member 
  public static class Builder {
    private int quarters = 0;
    private int dimes = 0;
    private int nickels = 0;
    private int pennies = 0;

    public static Builder newInstance() {
      return new Builder();
    } 

    private Builder() {}
    
    // Setter methods 
    public Builder setQuarters(int quantity) { 
      this.quarters = quantity; 
      return this;
    } 
    public Builder setDimes(int quantity) { 
      this.dimes = quantity; 
      return this;
    }
    public Builder setNickels(int quantity) { 
      this.nickels = quantity;
      return this;
    }
    public Builder setPennies(int quantity) { 
      this.pennies = quantity;
      return this;
    }

    public USCurrency build() {
      return new USCurrency(this);
    }
  }
}

  // Client code: 
private volatile USCurrency currency;
// ...

new Thread(new Runnable() {
  @Override public void run() {    
    currency = USCurrency.Builder.newInstance().setQuarters(1).setDimes(1).build();
  }
}).start();

new Thread(new Runnable() {
  @Override public void run() {    
    currency = USCurrency.Builder.newInstance().setQuarters(2).setDimes(2).build();
  }
}).start();

The Builder.newInstance() factory method is called with the required arguments (if any) to obtain a Builder object. The optional parameters are set using the setter methods of the builder. The object construction concludes with the invocation of the build() method. This pattern makes the class USCurrency immutable, and consequently, thread-safe.

Note that the currency field cannot be declared as final because it is assigned a new immutable object. It is, however, declared volatile in compliance with VNA01-J. Ensure visibility of shared references to immutable objects.

If input needs to be validated, ensure that the values are defensively copied prior to validation (see [FIO00-J. Defensively copy mutable inputs and mutable internal components] for more information). The builder class does not violate [SCP03-J. Do not expose sensitive private members of the outer class from within a nested class] because it maintains a copy of the variables defined in the scope of the containing class. These take precedence and as a result, do not break encapsulation.

Risk Assessment

Using method chaining in multithreaded environments without performing external locking can lead to non-deterministic behavior.

Automated Detection

TODO

Related Vulnerabilities

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

References

[[API 06]]
[[Bloch 08]] Item 2: "Consider a builder when faced with many constructor parameters"

null      [!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!]