Overriding thread-safe methods with methods that are unsafe for concurrent use can result in improper synchronization when a client that depends on the thread-safety promised by the parent inadvertently operates on an instance of the subclass. For example, an overridden synchronized method's contract can be violated when a subclass provides an implementation that is unsafe for concurrent use. Such overriding can easily result in errors that are difficult to diagnose. Consequently, programs must not override thread-safe methods with methods that are unsafe for concurrent use.
The locking strategy of classes designed for inheritance should always be documented. This information can subsequently be used to determine an appropriate locking strategy for subclasses (see rules LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code and LCK11-J. Avoid client-side locking when using classes that do not commit to their locking strategy).
...
This noncompliant code example overrides the synchronized doSomething() method in the Base class with an unsynchronized method in the Derived class.:
| Code Block | ||
|---|---|---|
| ||
class Base {
public synchronized void doSomething() {
// ...
}
}
class Derived extends Base {
@Override public void doSomething() {
// ...
}
}
|
...
This programming error can be difficult to diagnose , because threads that accept instances of Base can also accept instances of its subclasses. Consequently, clients could be unaware that they are operating on a thread-unsafe instance of a subclass of a a thread-safe class.
Compliant Solution (Synchronized Method)
This compliant solution synchronizes the doSomething() method of the subclass.:
| Code Block | ||
|---|---|---|
| ||
class Base {
public synchronized void doSomething() {
// ...
}
}
class Derived extends Base {
@Override public synchronized void doSomething() {
// ...
}
}
|
This solution also complies with rule LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code because the accessibility of the class is package-private. Package-private accessibility is permitted when untrusted code cannot infiltrate the package.
...
This compliant solution ensures that the Derived class is is thread-safe by overriding the synchronized doSomething() method of the Base class with a method that synchronizes on a private final lock object.
| Code Block | ||
|---|---|---|
| ||
class Base {
public synchronized void doSomething() {
// ...
}
}
class Derived extends Base {
private final Object lock = new Object();
@Override public void doSomething() {
synchronized (lock) {
// ...
}
}
}
|
...
This noncompliant code example defines a doSomething() method in the Base class that uses a private final lock in accordance with rule LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code.
| Code Block | ||
|---|---|---|
| ||
class Base { private final Object lock = new Object(); public void doSomething() { synchronized (lock) { // ... } } } class Derived extends Base { Logger logger = // initializeInitialize @Override public void doSomething() { try { super.doSomething(); } finally { logger.log(Level.FINE, "Did something"); } } } |
It is possible for multiple threads to cause the entries to be logged in an order that differs from the order in which the tasks are performed. Consequently, the doSomething() method of the Derived class cannot be used safely by multiple threads because it is not thread-safe.
Compliant Solution (Private Lock)
This compliant solution solution synchronizes the doSomething() method of the subclass using its own private final lock object.:
| Code Block | ||
|---|---|---|
| ||
class Base { private final Object lock = new Object(); public void doSomething() { synchronized (lock) { // ... } } } class Derived extends Base { Logger logger = // initializeInitialize private final Object lock = new Object(); @Override public void doSomething() { synchronized (lock) { try { super.doSomething(); } finally { logger.log(Level.FINE, "Did something"); } } } } |
Note that the Base and Derived objects maintain distinct locks that are inaccessible from each other's classes. Consequently, Derived can provide thread-safety guarantees independent of Base.
Risk Assessment
Overriding Overriding thread-safe methods with methods that are unsafe for concurrent access can result in unexpected behavior.
Rule | Severity | Likelihood | Detectable |
|---|
Repairable | Priority | Level |
|---|---|---|
TSM00-J |
Low | Probable |
Yes |
No | P4 | L3 |
Automated Detection
Sound automated detection is infeasible; heuristic checks could be useful.
| Tool | Version | Checker | Description | ||||||
|---|---|---|---|---|---|---|---|---|---|
| Parasoft Jtest |
| CERT.TSM00.OSNS | Avoid overriding synchronized methods with non-synchronized methods |
Bibliography
...
Rule 12: Thread-Safety Miscellaneous (TSM) Rule 12: Thread-Safety Miscellaneous (TSM)