Page History

While client-side locking is acceptable if the thread-safe class commits to its locking strategy and clearly documents it, Goetz et al. \[[Goetz 06|AA. Java References#Goetz 06]\] caution against its misuse:

{quote}
If extending a class to add another atomic operation is fragile because it distributes the locking code for a class over multiple classes in an object hierarchy, client-side locking is even more fragile because it entails putting locking code for class C into classes that are totally unrelated to C. Exercise care when using client-side locking on classes that do not commit to their locking strategy.
{quote}

The documentation of a class that does

 support client-side locking should explicitly state its applicability. An example of when not to use client-side locking is the class {{java.util.concurrent.ConcurrentHashMap<K,V>}}, whose documentation states \[[API 06|AA. Java References#API 06]\]:

 {mc} s/not// right? ~DS {mc}

{quote}
... even though all operations are thread-safe, retrieval operations do not entail locking, and there is not any support for locking the entire table in a way that prevents all access. This class is fully interoperable with Hashtable in programs that rely on its thread safety but not on its synchronization details.

 
{quote}  

In general, use client-side locking only when the documentation of the class recommends it. For example, the documentation of the wrapper method {{synchronizedList()}} of class {{java.util.Collections}} \[[API 06|AA. Java References#API 06]\] states:

{code

:bgColor

=#FFCCCC

}
// Client
public class BookWrapper {
  private final Book book;

  BookWrapper(Book book) {
    this.book = book;
  }

  public void issue(int days) {
    book.issue();
  }

  public Calendar getDueDate() {
    return book.getDueDate();
  }

  public void renew() {
    synchronized(book) {
      if (book.getDueDate().after(Calendar.getInstance())) {
        throw new IllegalStateException("Book overdue");
      } else {
        book.issue(14); // Issue book for 14 days
      }
    }
  }
}
{code}

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 will definitely break if {{Book}} is modified to use an internal private lock, as recommended by [CON04-J. Synchronize classes that may interact with untrusted code using a private final lock object]. This is because threads that call {{getDueDate()}} of class {{BookWrapper}} 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.

h2. Compliant Solution (composition)

This compliant solution uses an internal private lock object and synchronizes all its methods using this lock.

{code

:bgColor

=#ccccff

}
public final class BookWrapper {
  private final Book book;
  private final Object lock = new Object();

  BookWrapper(Book book) {
    this.book = book;
  }

  public void issue(int days) {
    synchronized(lock) {
      book.issue();
    }
  }

  public Calendar getDueDate() {
    synchronized(lock) {
      return book.getDueDate();
    }
  }

  public void renew() {
    synchronized(lock) {
      if (book.getDueDate().after(Calendar.getInstance())) {
        throw new IllegalStateException("Book overdue");
      } else {
        book.issue(14); // Issue book for 14 days
      }
    }
  }
}
{code}

Consequently, the {{BookWrapper}} class's locking strategy is independent of the locking policy of the {{Book}} instance. This solution incurs a very small performance penalty but the resulting code is much more robust \[[Goetz 06|AA. Java References#Goetz 06]\].

h2. Risk Assessment

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

|| Rule || Severity || Likelihood || Remediation Cost || Priority || Level ||
| CON31- J | low | probable | medium | {color:green}{*}P4{*}{color} | {color:green}{*}L3{*}{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+CON38-J].

h2. 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"

----
[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!|VOID CON06-J. Do not defer a thread that is holding a

 lock]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!|11. Concurrency (CON)]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!|CON08-J. Do not call alien methods that synchronize on the same objects as any callers in the execution chain]