Versions Compared

Key

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

Wiki Markup
According to Goetz and colleagues \[[Goetz 062006|AA. Java References#Goetz 06]\]:

...

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

...

Wiki Markup
The documentation of a class that supports client-side locking should explicitly state its applicability. For example, the class {{java.util.concurrent.ConcurrentHashMap<K,V>}} should not be used for client-side locking, because its documentation states \[[API 062006|AA. Java References#API 06]\]:

...

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

In order to guarantee serial access, it is critical that all access to the backing list is accomplished through the returned list. It is imperative that the user manually synchronize on the returned list when iterating over it. Failure to follow this advice may result in non-deterministic behavior.

Note When the backing list is inaccessible to an untrusted client, note that this advice is consistent with guideline 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 Book class Book that cannot be refactored. This Refactoring might happenbe impossible, for example, when if the source code is not available for review or the class is part of a general library that cannot be extended.

...

This class does not commit to its locking strategy (that is, it reserves the right to change its locking strategy without notice). Furthermore, it does not document that callers can safely use client-side locking. The client class BookWrapper client class uses client-side locking in the renew() method by synchronizing on a Book instance.

Code Block
bgColor#FFCCCC
// Client
public class BookWrapper {
  private final Book book;

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

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

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

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

If the Book 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 guideline LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code. This is The BookWrapper class's locking strategy breaks because threads that call BookWrapper.getDueDate() may perform operations on the thread-safe Book using its new locking policy. However, threads that call method the renew() method will always synchronize on the intrinsic lock of the Book instance. Consequently, the implementation will use two different locks.

...

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

Code Block
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(days);
    }
  }

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

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

...

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

...

In this noncompliant code example, the PrintableIPAddressList class extends the thread-safe IPAddressList class IPAddressList. PrintableIPAddressList locks on IpAddressList.ips in the method addAndPrintIPAddresses() method. This is another example of client-side locking , because a subclass uses is using an object owned and locked by its superclass.

...

Wiki Markup
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| guideline [LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code], the subclass {{PrintableIPAddressList}} subclass 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 062006|AA. Java References#Goetz 06]\].

...

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

Composition offers encapsulation benefits; , usually with minimal overhead. Refer to guideline OBJ07-J. Understand how a superclass can affect a subclass for more information on composition.

...

Wiki Markup
In this case, composition allows the {{PrintableIPAddressList}} class to use its own intrinsic lock independent of the lock of the underlying list class's lock.  The underlying Thiscollection does not require the underlying collectionneed 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 062006|AA. Java References#Goetz 06]\].

...

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

Rule Guideline

Severity

Likelihood

Remediation Cost

Priority

Level

CON34 LCK11- J

low

probable

medium

P4

L3

Automated Detection

TODO

Related Vulnerabilities

...

References

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

...