Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Migration of unmigrated content due to installation of a new plugin

Synchronizing on the return value of the Object.getClass() method , rather than a class literal can lead to unexpected behavior. Whenever the implementing class is subclassed, the subclass locks on the subclass's type, which is a completely different Class object.. The Class object of the subclass is entirely distinct from the Class object of the parent class.

According to The Java Language Specification, §4 Wiki MarkupSection 4.3.2 , "The Class Object" of the Java Language specification \[[JLS 05|AA. Java References#JLS 05]\] describes how method synchronization works[JLS 2005]:

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

This does not Programmers who interpret this to mean that a subclass using getClass() can only will synchronize on the Class object of the base class . In fact, it will 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 had in mind. The intent should be clearly documented or annotated. Note that if when a subclass does not 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)

This 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
bgColor#FFcccc

class Base {
  static DateFormat format =
      DateFormat.getDateInstance(DateFormat.MEDIUM);

  public voidDate doSomethingparse(String str) throws ParseException {
    synchronized (getClass()) {
      //return ... format.parse(str);
    }
  }
}

class Derived extends Base {
  public BaseDate getBasedoSomethingAndParse();String str) throws ParseException {
    //synchronized ...(Base.class) {
  }

  public void doSomethingElse() {// ...
    Base obj =return getBaseformat.parse(str);
    obj.doSomething(); }
  }
}

...

Compliant Solution (

...

Class Name Qualification)

Explicitly define the name of the class through name qualification (Base in this compliant solution) in the synchronized block. In this compliant solution, the class name providing the lock (Base) is fully qualified:

Code Block
bgColor#ccccff

class Base {
  static DateFormat format =
      DateFormat.getDateInstance(DateFormat.MEDIUM);

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

// ...

This code example always synchronizes on the Base.class object, even if when 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, see CON04-J. Synchronize using an internal private final lock object.

Compliant Solution (Class.forName())

This compliant solution uses the Class.forName() method to synchronize on the Base class's Class object.:

Code Block
bgColor#ccccff

class Base {
  static DateFormat format =
      DateFormat.getDateInstance(DateFormat.MEDIUM);

  public voidDate doSomethingparse(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 The class object that is being used for synchronization 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.fornameforName(). (See SEC05see SEC03-J. Do not expose standard APIs that use the immediate caller's class loader instance to untrusted code for more information.)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
bgColor#FFcccc
class Base {
  static DateFormat format =
      DateFormat.getDateInstance(DateFormat.MEDIUM);

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

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
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);
      }
    }
  }
}

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 present a false sense of thread safety and result in non-deterministic nondeterministic behavior.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

CON02

LCK02-J

medium

Medium

probable

Probable

medium

Medium

P8

L2

Automated Detection

The following table summarizes the examples flagged as violations by FindBugs:

Some static analysis tools can detect violations of this rule

ToolVersion

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:

Noncompliant Code Example

Flagged

Message

getClass() lock object

No

No data available about field accesses

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

References

Wiki Markup
\[[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

Description
Parasoft Jtest

Include Page
Parasoft_V
Parasoft_V

CERT.LCK02.SGCDo not synchronize on the class object returned by the 'getClass' method
SonarQube
Include Page
SonarQube_V
SonarQube_V

S3067




Bibliography


...

Image Added Image Added Image AddedVOID CON00-J. Synchronize access to shared mutable variables      11. Concurrency (CON)      CON03-J. Do not use background threads during class initialization