Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: filled out the examples sothat they compile and you can see the problem
Wiki Markup
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.

...



Section 4.3.2 "The Class Object" of the Java Language specification \[[JLS 05|AA. Java References#JLS 05]\] describes how method synchronization works:

...

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

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.

Noncompliant Code Example (getClass() lock object)

This noncompliant code example synchronizes on the class object returned by getClass().

Code Block
bgColor#FFcccc

class Base {
  public void doSomething() 

{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 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 getBase(); {
    // ...
  }

 .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 doSomethingElserun() {	    	 
    Base   obj = getBase();try {
  	  obj.doSomething(); 
  }
}

The actual Class object that gets locked when Base.doSomething() gets invoked depends on the return value of getBase(). If getBase() returns a Derived, then doSomething() locks on Derived.class, not Base.class.

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 Block
bgColor#ccccff

class Base {
  public void doSomething()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)

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 FORMAT =
    DateFormat.getDateInstance(DateFormat.MEDIUM);
	  
  public Date parse(String str) throws ParseException {
    synchronized (Base.class) {
      //return ... FORMAT.parse(str);
    }
  }
}

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, 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


// ...
{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, 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 voidDate doSomethingparse(String str) throws ParseException {
    synchronized (Class.forName("Base")) {
      //return ... FORMAT.parse(str);
    }
  }
}

...



// ...
{code}

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.forname()}}. (See [SEC05-J. Do not expose standard APIs that use the immediate caller's class loader instance to untrusted code] for more information.)

...

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

P8

L2

Automated Detection

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

...

Noncompliant Code Example

...

Flagged

...

Checker

...

Message

...

getClass() lock object

...

No




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

...

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

...

|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]&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!|11. Concurrency (CON)]&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!|CON03-J. Do not use background threads during class initialization]