Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

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,

...

which

...

is

...

a

...

completely

...

different

...

Class

...

object.

...

Wiki Markup
Section 4.3.2 "The Class Object" of the _Java Language Specification_ describes how method synchronization works \[[JLS 

...

2005|AA. Java References#JLS 05]\]:

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

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 guideline LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code.

Noncompliant Code Example (getClass() Lock Object)

In this noncompliant code example, the parse() method of 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 Block
bgColor#FFcccc


{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 intended. 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.

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 [LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code|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 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.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}

Compliant Solution (Class Name Qualification)

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

...

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 Date parse(String str) throws ParseException {
    try {
      synchronized (Class.forName("Base")) {
        return format.parse(str);
      }
    } catch (ClassNotFoundException e) {
      // Forward to handler
    }
    return null;
  }
}

// ...
{code}

It

...

is

...

important

...

that

...

untrusted

...

inputs

...

are

...

not

...

accepted

...

as

...

arguments

...

while

...

loading

...

classes

...

using

...

Class.forName()

...

.

...

See guideline SEC05-J.

...

Do

...

not

...

expose

...

standard

...

APIs

...

that

...

use

...

the

...

immediate

...

caller's

...

class

...

loader

...

instance

...

to

...

untrusted

...

code

...

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

...

Base

...

class.

...

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()) {
      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.

...

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 {
  static DateFormat format =
    DateFormat.getDateInstance(DateFormat.MEDIUM);// ...

  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.

Risk Assessment

Synchronizing on the class object returned by getClass() can result in nondeterministic behavior.

Guideline

Severity

Likelihood

Remediation Cost

Priority

Level

LCK02-J

medium

probable

medium

P8

L2

References

Wiki Markup
\[[API 2006|AA. Java References#API 06]\]
\[[Findbugs 

...

2008|AA. Java References#Findbugs 08]\].
\[[Pugh 

...

2008|AA. Java References#Pugh 08]\] "Synchronization"
\[[Miller 

...

2009|AA. Java References#Miller 09]\] Locking

...

...

Image Added      12. Locking (LCK)      Image Added