CON30-J. Do not use method chaining implementations in a multi-threaded environment

Method chaining is a technique that defines several methods that return the this reference of the current object. It is a convenience mechanism that allows multiple method invocations on the same object to occur, in a single statement. However, unless special care is taken, the implementation may not be safe for multithreaded use.

Noncompliant Code Example

Several design patterns exist for building an object and setting its optional fields. However, not all of them provide initialization safety, in that, a thread may observe the object before its construction is over. This noncompliant code example shows the unsafe Javabeans pattern.

final class Currency {
  // Total amount requested (mandatory fields)
  private int dollars = -1; // Initialize to default value
  private int cents = -1;   // Initialize to default value
  // Change requested, denomination (optional fields)
  private int quarters = 0;
  private int dimes = 0;
  private int nickels = 0;
  private int pennies = 0;
  public Currency() {} // no argument constructor

  // Setter methods 
  public Currency setDollar(int amount) { 
    dollars = amount;
    return this;
  }

  public Currency setCents(int amount) { 
    cents = amount;
    return this; 
  }

  public Currency setQuarters(int quantity) { 
    quarters = quantity; 
    return this;
  } 
  public Currency setDimes(int quantity) { 
    dimes = quantity; 
    return this;
  }
  
  public Currency setNickels(int quantity) { 
    nickels = quantity;
    return this;
  }
  
  public Currency setPennies(int quantity) { 
    pennies = quantity;
    return this;
  }
}

// Client code:
Currency USD = new Currency();
USD.setDollar(10).setCents(50).setQuarters(42);

The Javabeans pattern uses a no-argument constructor along with a series of parallel setter methods to build an object. This pattern is not thread-safe and can lead to inconsistent object state. Moreover, it permits another thread to access the object even though it may only be partially initialized (not all required fields are initialized).

Compliant Solution

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

final class Currency {
  // Total amount requested (mandatory fields)
  private final int dollars;
  private final int cents;
  // Change requested, denomination (optional fields)
  private final int quarters;
  private final int dimes;
  private final int nickels;
  private final int pennies;
  
  // Static class member 
  private Currency(Builder builder) {
    dollars = builder.dollars;
    cents = builder.cents;
   
    quarters = builder.quarters;
    dimes = builder.dimes;
    nickels = builder.nickels;
    pennies = builder.pennies;
  }

  // Static class member 
  public static class Builder {
    private final int dollars;
    private final int cents;
    private int quarters = 0;
    private int dimes = 0;
    private int nickels = 0;
    private int pennies = 0;
	 
    public Builder(int dollars, int cents) {
      this.dollars = dollars;
      this.cents = cents;
    }
    public Builder quarters(int quantity) {
      quarters = quantity;  
      return this; 
    }
    public Builder dimes(int quantity) {
      dimes = quantity; 
      return this;	 
    }
    public Builder nickles(int quantity) {
      nickels = quantity; 
      return this;	 
    }
    public Builder pennies(int quantity) {
      pennies = quantity; 
      return this;	 
    }
    public Currency build() {
      return new Currency(this); 	 
    }
  }
}

// Client code:
Currency USD = new Currency.Builder(10,50).quarters(42).build();

The idea is to call the constructor with the required parameters and obtain a builder object. Each optional parameter can be set using setters on the builder. The object construction concludes with the invocation of the build() method. This also has the effect of making the class Currency immutable.

If input needs to be validated, make sure that the values are copied from the builder class to the containing class's fields prior to checking. 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.

If the number of fields is small, it is better to synchronize the setter methods instead of using this design pattern.

Risk Assessment

Using implementations of method chaining that are not thread-safe can lead to non-deterministic behavior.

Rule	Severity	Likelihood	Remediation Cost	Priority	Level
CON30- J	low	unlikely	high	P1	L1

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 7, Avoid finalizers

FIO36-J. Do not create multiple buffered wrappers on an InputStream 09. Input Output (FIO) 09. Input Output (FIO)

Space shortcuts

Page tree