According to Goetz and colleagues \ [[Goetz 06|AA. Java References#Goetz 06]\Goetz 2006]: Wiki Markup
Client-side locking entails guarding client code that uses some object X with the lock X uses to guard its own state. In order to use client-side locking, you must know what lock X uses.
While client-side locking is acceptable if when the thread-safe class commits to and clearly documents its locking strategy and clearly documents it, Goetz cautions against its misuse \[[Goetz 06|AA. Java References#Goetz 06]\Goetz and colleagues caution against its misuse [Goetz 2006]: Wiki Markup
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.
...
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 06|AA. Java References#API 06]\]:API 2014] states that
... 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.
In general, use Use of client-side locking is permitted only when the documentation of the class recommends it. For example, the documentation of the wrapper method {{ Wiki Markup synchronizedList()
}} of class {{ wrapper method of java.util.Collections
}} \[[API 06|AA. Java References#API 06]\] class [API 2014] 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 that When the backing list is inaccessible to an untrusted client, this advice is consistent with CON11LCK04-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 the source code is not available unavailable for review or when the class is part of a general library that cannot be extended.
Code Block |
---|
final class Book { // MayCould 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; } } |
This class does not fails to commit to its locking strategy (that is, it reserves the right to change its locking strategy without notice). Furthermore, it does not fails to document that callers can safely use client-side locking. The BookWrapper
client class BookWrapper
uses client-side locking in the renew()
method by synchronizing on a Book
instance.
Code Block | ||
---|---|---|
| ||
// 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 class Book
changes the Book
class were to change its synchronization policy in the future, the BookWrapper
class's locking strategy might silently break. For instance, the Bookwrapper
BookWrapper
class's locking strategy breaks would break if Book
is were modified to use a private final lock object, as recommended by CON07 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 would perform operations on the thread-safe Book
using its new locking policy. However, threads that call method the renew()
will method would always synchronize on the intrinsic lock of the Book
instance. Consequently, the implementation will would use two different locks.
Compliant Solution (Private Final Lock Object)
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 | ||
---|---|---|
| ||
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
}
}
}
}
|
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 to thread-safe classes \ [[Goetz 06|AA. Java References#Goetz 06]\Goetz 2006]:
Extension is more fragile than adding code directly to a class, because the implementation of the synchronization policy is now distributed over multiple, separately maintained source files. If the underlying class were to change its synchronization policy by choosing a different lock to guard its state variables, the subclass would subtly and silently break , because it no longer used the right lock to control concurrent access to the base class's state.
In this noncompliant code example, the PrintableIPAddressList
class extends the thread-safe IPAddressList
class IPAddressList
. PrintableIPAddressList
locks on IpAddressListIPAddressList.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.
Code Block | ||
---|---|---|
| ||
// This class maycould 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 visibility is 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 [CON07-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]\]. class were 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, the PrintableIPAddressList
subclass would silently break. Moreover, if a wrapper such as Collections.synchronizedList()
were used, it would be difficult for a client to determine the type of the class being wrapped to extend it [Goetz 2006].
Compliant Solution (Composition)
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 (refer to OBJ07OBJ02-J. Understand how a superclass can affect a subclassPreserve dependencies in subclasses when changing superclasses for more information on composition).
Code Block | ||
---|---|---|
| ||
// 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 {{ Wiki Markup 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]\'s lock. The underlying collection lacks a requirement for thread-safety because the PrintableIPAddressList
wrapper prevents direct access to its methods by publishing its own synchronized equivalents. This approach provides consistent locking even when the underlying class changes its locking policy in the future [Goetz 2006].
Risk Assessment
Using client-side locking when the the thread-safe class does not fails to commit to its locking strategy can cause data inconsistencies and deadlock.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
CON34LCK11-J | low Low | probable Probable | medium Medium | P4 | L3 |
Automated Detection
TODO
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
References
Wiki Markup |
---|
\[[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" |
Bibliography
[API 2014] | Class Collections |
Section 8.2, "Synchronization and Collection Classes" | |
Section 4.4.1, "Client-side Locking" | |
[Lee 2009] | "Map & Compound Operation" |
...
CON33-J. Ensure ThreadLocal variables are reinitialized when using thread pools 11. Concurrency (CON) CON35-J. Document thread-safety and use annotations where applicable