LCK11-J

code” on page 41. 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 the renew()method will always synchronize on the intrinsic lock of the Book instance. Consequently, the implementation will use two different locks.

3.12.2 Compliant Solution (Private Final Lock Object)

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

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.

3.12.3 Noncompliant Code Example (Class Extension and Accessible Member Lock)

Goetz and colleagues describe the fragility of class extension for adding functionality to threadsafe classes [Goetz 2006]:

CMU/SEI-2010-TR-015 | 88

LCK11-J

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. PrintableIPAddressList locks on IPAddressList.ips in the addAndPrintIPAddresses()method. This is another example of client-side locking because a subclass is using an object owned and locked by its superclass.

//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 guideline “LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code” on page 41), the 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 in order to extend it [Goetz 2006].

3.12.4 Compliant Solution (Composition)

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

CMU/SEI-2010-TR-015 | 89

LCK11-J

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.”8

// 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 underlying list class’s lock. The underlying collection does not need 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 2006].

3.12.5 Risk Assessment

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

Guideline	Severity	Likelihood	Remediation Cost	Priority	Level
LCK11- J	low	probable	medium	P4	L3

3.12.6 References

[Goetz 2006]		Section 4.4.1, “Client-side Locking”
		Section 4.4.2, “Composition”
		Section 5.2.1, “ConcurrentHashMap”
[Lee 2009]		“Map & Compound Operation”
[Oaks 2004]		Section 8.2, “Synchronization and Collection Classes”
[Sun 2009b]		Class Vector, Class WeakReference, Class ConcurrentHashMap<K,V>
8	This guideline is described at https://www.securecoding.cert.org/confluence/display/java/.

CMU/SEI-2010-TR-015 | 90