Synchronizing on the return value of the Object.getClass()
method can lead to unexpected behavior. Whenever the implementing class is subclassed, the subclass locks on the subclass's type. The Class
object of the subclass is entirely distinct from the Class
object of the parent class.
According to the The Java Language Specification, §4.3.2, "The Class Object" [JLS 2005]:
...
Programmers who interpret this to mean that a subclass using getClass()
will synchronize on the Class
object of the base class are incorrect. The subclass will actually lock on its own Class
object; , which may or may not be what the programmer intended. Consequently, programs must not synchronize on the class object returned by getClass()
.
...
When synchronizing on a class literal, the corresponding lock object should be inaccessible to untrusted code. Callers from other packages cannot access class objects that are package-private; consequently, synchronizing on the intrinsic lock object of such classes is permitted . For more information, (see rule LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code for more information).
Noncompliant Code Example (getClass()
Lock Object)
...
The Derived
class also adds a doSomethingAndParse()
method that locks on the class object of the Base
class because the developer misconstrued that the parse()
method in Base
always obtains a lock on the Base
class object, and doSomethingAndParse()
must follow the same locking policy. Consequently, the Derived
class has two different locking strategies and fails to be thread-safe.
Code Block | ||
---|---|---|
| ||
class Base {
static DateFormat format =
DateFormat.getDateInstance(DateFormat.MEDIUM);
public Date parse(String str) throws ParseException {
synchronized (getClass()) {
return format.parse(str);
}
}
}
class Derived extends Base {
public Date doSomethingAndParse(String str) throws ParseException {
synchronized (Base.class) {
// ...
return format.parse(str);
}
}
}
|
...
In this compliant solution, the class name providing the lock (Base
) is fully qualified.:
Code Block | ||
---|---|---|
| ||
class Base {
static DateFormat format =
DateFormat.getDateInstance(DateFormat.MEDIUM);
public Date parse(String str) throws ParseException {
synchronized (Base.class) {
return format.parse(str);
}
}
}
// ...
|
...
This compliant solution uses the Class.forName()
method to synchronize on the Base
class's Class
object.:
Code Block | ||
---|---|---|
| ||
class Base {
static DateFormat format =
DateFormat.getDateInstance(DateFormat.MEDIUM);
public Date parse(String str) throws ParseException {
try {
synchronized (Class.forName("Base")) {
return format.parse(str);
}
} catch (ClassNotFoundException x) {
// "Base" not found; handle error
}
return null;
}
}
// ...
|
Never accept untrusted inputs as arguments while loading classes using Class.forName()
. See rule (see SEC03-J. Do not load trusted classes after allowing untrusted code to load arbitrary classes for more information).
Noncompliant Code Example (getClass()
Lock Object, Inner Class)
This noncompliant code example synchronizes on the class object returned by getClass()
in the parse()
method of class Base
. The Base
class also has a nested Helper
class whose doSomethingAndParse()
method incorrectly synchronizes on the value returned by getClass()
.
Code Block | ||
---|---|---|
| ||
class Base { static DateFormat format = DateFormat.getDateInstance(DateFormat.MEDIUM); public Date parse(String str) throws ParseException { synchronized (getClass()) { // intendIntend to synchronizes on Base.class return format.parse(str); } } public Date doSomething(String str) throws ParseException { return new Helper().doSomethingAndParse(str); } private class Helper { public Date doSomethingAndParse(String str) throws ParseException { synchronized (getClass()) { // Synchronizes on Helper.class // ... return format.parse(str); } } } } |
...
This compliant solution synchronizes using a Base
class literal in the parse()
and doSomethingAndParse()
methods.:
Code Block | ||
---|---|---|
| ||
class Base {
// ...
public Date parse(String str) throws ParseException {
synchronized (Base.class) {
return format.parse(str);
}
}
private class Helper {
public Date doSomethingAndParse(String str) throws ParseException {
synchronized (Base.class) { // Synchronizes on Base class literal
// ...
return format.parse(str);
}
}
}
}
|
...
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
LCK02-J | medium Medium | probable Probable | medium Medium | P8 | L2 |
Bibliography
| |
| |
[JLS 2005] | §4.3.2, "The Class Object" |
"Synchronization" | |
"Locking" |
...