...
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 Java Language Specification, §4.3.2
...
...
...
...
[JLS 2005]:
A class method that is declared
synchronized
synchronizes on the lock associated with theClass
object of the class.
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()
.
The programmer's actual intent should be clearly documented or annotated. Note that when a subclass fails to override an accessible noncompliant superclass's method, it inherits the method, which may lead to the false conclusion that the superclass's intrinsic lock is available in the subclass.
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 (see 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)
In this noncompliant code example, the parse()
method of the Base
class parses a date and synchronizes on the class object returned by getClass()
. The Derived
class also inherits the parse()
method. However, this inherited method synchronizes on Derived
's class object because the inherited parse method's invocation of getClass()
is really an invocation of this.getClass()
, and the this
argument is a reference to the instance of the Derived
class.
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 | ||
---|---|---|
| ||
[JLS 05|AA. Java References#JLS 05]\] describes how method synchronization works: {quote} A class method that is declared {{synchronized}} synchronizes on the lock associated with the {{Class}} object of the class. {quote} This does not mean that a subclass using {{getClass()}} can only synchronize on the {{Class}} object of the base class. In fact, it will lock on its own {{Class}} object, which may or may not be what the programmer had in mind. The intent should be clearly documented or annotated. Note that if a subclass does not override an accessible noncompliant superclass's method, it inherits the method which may lead to the false conclusion that the superclass's intrinsic lock is available in the subclass. h2. Noncompliant Code Example ({{getClass()}} lock object) In this noncompliant code example, the {{parse()}} method of class {{Base}} parses a date and synchronizes on the class object returned by {{getClass()}}. The class {{Derived}} also inherits the {{parse()}} method. However, this inherited method synchronizes on the class object of {{Derived}} because of the particular return value of {{getClass()}}. {{Derived}} also adds a method {{doSomethingAndParse()}} 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 class {{Derived}} has two different locking strategies and is not thread-safe. {code:bgColor=#FFcccc} class Base { static DateFormat FORMATformat = DateFormat.getDateInstance(DateFormat.MEDIUM); public Date parse(String str) throws ParseException { synchronized (getClass()) { return FORMATformat.parse(str); } } } class Derived extends Base { public Date doSomethingAndParse(String str) throws ParseException { synchronized (Base.class) { // ... return FORMATformat.parse(str); } } } {code} {mc} // Hidden main() method to be put in class Derived // Prints arbitrary date values and throws exceptions at times public static void main(String[] args) { for(int i = 0; i < 100; i++) |
Compliant Solution (Class Name Qualification)
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 { newsynchronized Thread(new Runnable((Base.class) { @Override public void run() { return format.parse(str); } } } // ... |
This code example always synchronizes on the Base.class
object, even when it is called from a Derived
object.
Compliant Solution (Class.forName()
)
This compliant solution uses the Class.forName()
method to synchronize on the Base
class's Class
object:
Code Block | ||
---|---|---|
| ||
class Base { static DateFormat format = try { System.out.println(new Derived().parse("Jan 1, 2010")); } catch (ParseException e) { // Forward to handler } } }).start(DateFormat.getDateInstance(DateFormat.MEDIUM); public Date parse(String str) throws ParseException { new Thread(new Runnable() try { @Override public void run()synchronized (Class.forName("Base")) { try {return format.parse(str); System.out.println(new Derived().doSomethingAndParse("Jan 2, 2010")); } } catch (ParseExceptionClassNotFoundException ex) { // Forward to handler } "Base" not found; handle error } }).start(); return null; } } // ... |
Never accept untrusted inputs as arguments while loading classes using Class.forName()
(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 | ||
---|---|---|
| ||
{mc} h2. Compliant Solution (class name qualification) Explicitly define the name of the class through name qualification ({{Base}} in this compliant solution) in the synchronized block. {code:bgColor=#ccccff} class Base { static DateFormat FORMATformat = DateFormat.getDateInstance(DateFormat.MEDIUM); public Date parse(String str) throws ParseException { synchronized (getClass()) { // Intend to synchronizes on Base.class) { return FORMATformat.parse(str); } } } // ... {code} This code example always synchronizes on the {{Base.class}} object, even if it is called from a {{Derived}} object. The class object that is being used for synchronization should not be accessible to untrusted code. If the class is package-private, callers from other packages may not access the class object, ensuring its trustworthiness as an intrinsic lock object. For more information, 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); } } } } |
The call to getClass()
in the Helper
class returns a Helper
class object instead of the Base
class object. Consequently, a thread that calls Base.parse()
locks on a different object than a thread that calls Base.doSomething()
. It is easy to overlook concurrency errors in inner classes because they exist within the body of the containing outer class. A reviewer might incorrectly assume that the two classes have the same locking strategy.
Compliant Solution (Class Name Qualification)
This compliant solution synchronizes using a Base
class literal in the parse()
and doSomethingAndParse()
methods:
Code Block | ||
---|---|---|
| ||
see [CON04-J. Synchronize using an internal private final lock object]. h2. Compliant Solution ({{Class.forName()}}) This compliant solution uses the {{Class.forName()}} method to synchronize on the {{Base}} class's {{Class}} object. {code:bgColor=#ccccff} class Base { static DateFormat FORMAT = DateFormat.getDateInstance(DateFormat.MEDIUM); // ... public Date parse(String str) throws ParseException { synchronized (Class.forName("Base"))Base.class) { return FORMATformat.parse(str); } } } // ... {code} The private class objectHelper that{ is being used forpublic synchronizationDate should also not be accessible to untrusted code, see [CON04-J. Synchronize using an internal private final lock object] for more information. Furthermore, care must be taken to ensure that untrusted inputs are not accepted as arguments while loading classes using {{Class.forname()}}. (See [SEC05-J. Do not expose standard APIs that use the immediate caller's class loader instance to untrusted code] for more information.) h2. Risk Assessment Synchronizing on the class object returned by {{getClass()}} can present a false sense of thread safety and result in non-deterministic behavior. || Rule || Severity || Likelihood || Remediation Cost || Priority || Level || | CON02- J | medium | probable | medium | {color:#cc9900}{*}P8{*}{color} | {color:#cc9900}{*}L2{*}{color} | h3. Automated Detection The following table summarizes the examples flagged as violations by FindBugs: ||Noncompliant Code Example||Flagged||Checker||Message|| |{{getClass()}} lock object|No|WL_USING_GETCLASS_RATHER_THAN_CLASS_LITERAL|n/a| The following table summarizes the examples flagged as violations by [SureLogic Flashlight|http://www.surelogic.com/]: ||Noncompliant Code Example||Flagged||Message|| |{{getClass()}} lock object|No|No data available about field accesses| h3. Related Vulnerabilities Search for vulnerabilities resulting from the violation of this rule on the [CERT website|https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+CON36-J]. h2. References \[[API 06|AA. Java References#API 06]\] \[[Findbugs 08|AA. Java References#Findbugs 08]\]. \[[Pugh 08|AA. Java References#Pugh 08]\] "Synchronization" \[[Miller 09|AA. Java References#Miller 09]\] Locking ---- [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!|VOID CON00-J. Synchronize access to shared mutable variables] [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!|11. Concurrency (CON)] [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!|CON03-J. Do not use background threads during class initialization] doSomethingAndParse(String str) throws ParseException { synchronized (Base.class) { // Synchronizes on Base class literal // ... return format.parse(str); } } } } |
Consequently, both Base
and Helper
lock on Base
's intrinsic lock. Similarly, the Class.forName()
method can be used instead of a class literal.
Risk Assessment
Synchronizing on the class object returned by getClass()
can result in nondeterministic behavior.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
LCK02-J | Medium | Probable | Medium | P8 | L2 |
Automated Detection
Some static analysis tools can detect violations of this rule
Tool | Version | Checker | Description | ||||||
---|---|---|---|---|---|---|---|---|---|
Parasoft Jtest |
| CERT.LCK02.SGC | Do not synchronize on the class object returned by the 'getClass' method | ||||||
SonarQube |
|
Bibliography
[API 2014] | |
[JLS 2005] | §4.3.2, "The Class Object" |
"Synchronization" | |
"Locking" |
...