Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Wiki Markup
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 Section 4.3.2, "The Class Object," of the _Java Language Specification_ \[[JLS 2005|AA. Bibliography#JLS 05]\]:

{quote}
A class method that is declared {{synchronized}} synchronizes on the lock associated with the {{Class}} object of the class.
{quote}

Programmers who interpret this to mean that a subclass using {{getClass()}} will synchronize on the {{Class}} object of the base class are incorrect. In fact, the subclass will lock on its own {{Class}} object; which may or may not be what the programmer intended. The programmer's actual 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.

When synchronizing on a class literal, the corresponding lock object 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, see rule [LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code].


h2. 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 of the particular return value of {{getClass()}}.

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 is not thread-safe.

{code:bgColor=#FFcccc}
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);
    }
  }
}
{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++) {
new Thread(new Runnable() {
@Override public void run() {
try {
System.out.println(new Derived().parse("Jan 1, 2010"));
} catch (ParseException e) {
// Forward to handler
}
}
}).start();

new Thread(new Runnable() {
@Override public void run() {
try {
System.out.println(new Derived().doSomethingAndParse("Jan 2, 2010"));
} catch (ParseException e) {
// Forward to handler
}
}
}).start();
}
}
{mc} 

h2. Compliant Solution (Class Name Qualification)

In this compliant solution, the class name providing the lock ({{Base}}) is fully qualified.

{code:bgColor=#ccccff}
class Base {
  static DateFormat format =
    DateFormat.getDateInstance(DateFormat.MEDIUM);

  public Date parse(String str) throws ParseException {
    synchronized (Base.class) {
      return format.parse(str);
    }
  }
}

// ...
{code}

This code example always synchronizes on the {{Base.class}} object, even if it is called from a {{Derived}} 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")) {
      return format.parse(str);
    }
  } 
}

// ...
{code}

It is important that untrusted inputs are not accepted as arguments while loading classes using {{Class.forName()}}. See rule [SEC05-J. Do not expose standard APIs that use the immediate caller's class loader instance to untrusted code] for more information.


h2. 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 {{Base}} class. The {{Base}} class also has a nested {{Helper}} class whose {{doSomethingAndParse()}} method incorrectly synchronizes on the value returned by {{getClass()}}.

{code:bgColor=#FFcccc}
class Base {
  static DateFormat format =
    DateFormat.getDateInstance(DateFormat.MEDIUM);

  public Date parse(String str) throws ParseException {
    synchronized (getClass()) {
      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 getClass()
        // ...
        return format.parse(str);
      }
    }
  }
}
{code}

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.

h2. Compliant Solution (Class Name Qualification)

This compliant solution synchronizes using a {{Base}} class literal in the {{parse()}} and {{doSomethingAndParse()}} methods.

{code:bgColor=#ccccff}
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);
      }
    }
  }
}
{code}

Consequently, both {{Base}} and {{Helper}} lock on {{Base}}'s intrinsic lock. Similarly, the {{Class.forname()}} method can be used instead of a class literal.


h3. 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 | {color:#cc9900}{*}P8{*}{color} | {color:#cc9900}{*}L2{*}{color} |


h3. Bibliography

| \[[API 2006|AA. Bibliography#API 06]\] | |
| \[[Findbugs 2008|AA. Bibliography#Findbugs 08]\]. | |
| \[[Pugh 2008|AA. Bibliography#Pugh 08]\] | "Synchronization" |
| \[[Miller 2009|AA. Bibliography#Miller 09]\] | Locking |


h3. Automated Detection

{mc}
// Ignore
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|
{mc}

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].

----
[!The CERT Oracle Secure Coding Standard for Java^button_arrow_left.png!|LCK01-J. Do not synchronize on objects that may be reused]&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[!The CERT Oracle Secure Coding Standard for Java^button_arrow_up.png!|08. Locking (LCK)]&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[!The CERT Oracle Secure Coding Standard for Java^button_arrow_right.png!|LCK03-J. Do not synchronize on the intrinsic locks of high-level concurrency objects]