Page History

...

Note that this advice is consistent with LCK04-J. Do not synchronize on a collection view if the backing collection is accessible when the backing list is inaccessible to an untrusted client.

Noncompliant Code Example (Intrinsic Lock)

This noncompliant code example uses a thread-safe class Book that cannot be refactored. This might happen, for example, when the source code is not available for review or the class is part of a general library that cannot be extended.

final class Book {
  // May change its locking policy in the future to use private final locks
  private final String title;
  private Calendar dateIssued;
  private Calendar dateDue;

  Book(String title) {
    this.title = title; 
  }
  
  public synchronized void issue(int days) {
    dateIssued = Calendar.getInstance();
    dateDue = Calendar.getInstance();
    dateDue.add(dateIssued.DATE, days);	 
  }

  public synchronized Calendar getDueDate() {
    return dateDue;
  }
}

...

If class Book changes its synchronization policy in the future, the BookWrapper class's locking strategy might silently break. For instance, the Bookwrapper class's locking strategy breaks if Book is modified to use a private final lock object, as recommended by LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code. This is because threads that call BookWrapper.getDueDate() may perform operations on the thread-safe Book using its new locking policy. However, threads that call method renew() will always synchronize on the intrinsic lock of the Book instance. Consequently, the implementation will use two different locks.

Compliant Solution (Private Final Lock Object)

This compliant solution uses a private final lock object and synchronizes all its methods using this lock.

...

The BookWrapper class's locking strategy is now independent of the locking policy of the Book instance.

Noncompliant Code Example (Class Extension and Accessible Member Lock)

Goetz and colleagues describe the fragility of class extension for adding functionality to thread-safe classes \[[Goetz 06|AA. Java References#Goetz 06]\]:

...

// This class may change its locking policy in the future, for example, 
// if new non-atomic methods are added
class IPAddressList {
  private final List<InetAddress> ips = Collections.synchronizedList(new ArrayList<InetAddress>());
  
  public List<InetAddress> getList() {
    return ips; // No defensive copies required as package-private visibility
  }

  public void addIPAddress(InetAddress address) {
    ips.add(address);
  }
}

class PrintableIPAddressList extends IPAddressList {
  public void addAndPrintIPAddresses(InetAddress address) {
    synchronized(getList()) {
      addIPAddress(address);
      InetAddress[] ia = (InetAddress[]) getList().toArray(new InetAddress[0]);      
      // ...
    }
  }
}

If the {{IPAddressList}} class is modified to use block synchronization on a private final lock object, as recommended by [LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code|LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code], the subclass {{PrintableIPAddressList}} will silently break. Moreover, if a wrapper such as {{Collections.synchronizedList()}} is used, it is difficult for a client to determine the type of the class being wrapped to extend it \[[Goetz 06|AA. Java References#Goetz 06]\]. 

Compliant Solution (Composition)

This compliant solution wraps an object of class IPAddressList and provides synchronized accessors that can be used to manipulate the state of the object.

...

// Class IPAddressList remains unchanged
class PrintableIPAddressList {
  private final IPAddressList ips;
 
  public PrintableIPAddressList(IPAddressList list) {
    this.ips = list;
  }
  
  public synchronized void addIPAddress(InetAddress address) {
    ips.addIPAddress(address);
  }

  public synchronized void addAndPrintIPAddresses(InetAddress address) {  
    addIPAddress(address); 
    InetAddress[] ia = (InetAddress[]) ips.getList().toArray(new InetAddress[0]);     
    // ...
  }
}

In this case, composition allows the {{PrintableIPAddressList}} class to use its own intrinsic lock independent of the lock of the underlying list class.  This does not require the underlying collection to be thread-safe because the {{PrintableIPAddressList}} wrapper prevents direct access to its methods by publishing its own synchronized equivalents. This approach provides consistent locking even if the underlying class changes its locking policy in the future \[[Goetz 06|AA. Java References#Goetz 06]\].

Risk Assessment

Using client-side locking when the thread-safe class does not commit to its locking strategy can cause data inconsistencies and deadlock.

Automated Detection

TODO

Related Vulnerabilities

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

References

\[[API 06|AA. Java References#API 06]\] Class Vector, Class WeakReference, Class ConcurrentHashMap<K,V>
\[[JavaThreads 04|AA. Java References#JavaThreads 04]\] 8.2 "Synchronization and Collection Classes"
\[[Goetz 06|AA. Java References#Goetz 06]\] 4.4.1. Client-side Locking, 4.4.2. Composition and 5.2.1. ConcurrentHashMap
\[[Lee 09|AA. Java References#Lee 09]\] "Map & Compound Operation"

...