Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: REM cost reform

Overriding thread-safe methods with methods that are unsafe for concurrent use can result in improper synchronization when a client that depends on the thread-safety promised by the parent inadvertently operates on an instance of the subclass. For example, an overridden synchronized method's contract can be violated when a subclass provides an implementation that is unsafe for concurrent use. Such overriding can easily result in errors that are difficult to diagnose. Consequently, programs must not override thread-safe methods with methods that are unsafe for concurrent use.

The locking strategy of classes designed for inheritance should always be documented. This information can subsequently be used to determine an appropriate locking strategy for subclasses (see LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code and LCK11-J. Avoid client-side locking when using classes that do not commit to their locking strategy).

Noncompliant Code Example (Synchronized Method)

This noncompliant code example overrides the synchronized doSomething() method in the Base class with an unsynchronized method in the Derived class:

Code Block
bgColor#FFCCCC
Wiki Markup
Concurrency related issues manifest themselves when assumptions are made about the multithreaded behavior of derived classes. An overridden synchronized method's contract may be violated if a subclass defines an implementation that is not safe for concurrent access. 

{mc}
// This line is not necessary because thread-safe implies all kinds of synchronization
Furthermore, if an overridden method makes concurrency guarantees, even if it is not itself synchronized, the subclass must adhere to those guarantees.
{mc}

The guideline [CON04-J. Synchronize using an internal private lock object] recommends documenting the locking strategy in use for classes designed for inheritance. This information is useful when deciding the locking strategy of subclasses. 

h2. Noncompliant Code Example (synchronized method)

This noncompliant code example defines a {{synchronized}} {{doSomething()}} method in class {{Base}}. 

{code:bgColor=#FFCCCC}
class Base {
  public synchronized void doSomething() {
    // ...	
  }
}

class SubDerived extends Base {
  @Override public void doSomething() {
    // ...		
  }
}
{code}

The {{

The doSomething()

...

method

...

of the Base class can be safely used by multiple threads, but instances of the Derived subclass cannot.

This programming error can be difficult to diagnose because threads that accept instances of Base can also accept instances of its subclasses. Consequently, clients could be unaware that they are operating on a thread-unsafe instance of a subclass of a thread-safe class.

Compliant Solution (Synchronized Method)

This compliant solution synchronizes the doSomething() method of the subclass:

Code Block
bgColor#ccccff
 class {{Base}} can be safely used by multiple threads. However, if a subclass {{Sub}} overrides the method but leaves it unsynchronized, its instance cannot be safely used by multiple threads. 

This problem is hard to notice because threads that accept instances of {{Base}} also accept instances of its subclasses. Consequently, the threads may incorrectly assume that the subclasses are thread-safe.


h2. Compliant Solution (synchronized method)

This compliant solution synchronizes the {{doSomething()}} method of the subclass. 

{code:bgColor=#ccccff}
class Base {
  public synchronized void doSomething() {
    // ...	
  }
}

class SubDerived extends Base {
  @Override public synchronized void doSomething() {
    // ...		
  }
}
{code}

This compliant solution does not violate [CON04-J. Synchronize using an internal private lock object] because the accessibility of the class is package-private which is allowable when untrusted code cannot infiltrate the package.


h2. Noncompliant Code Example (private lock)

This noncompliant code example defines a {{doSomething()}} method in class {{Base}} that uses an internal private lock, in accordance with [CON04-J. Synchronize using an internal private lock object].

{code:bgColor=#FFCCCC}
public class

This solution also complies with LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code because the accessibility of the class is package-private. Package-private accessibility is permitted when untrusted code cannot infiltrate the package.

Compliant Solution (Private Final Lock Object)

This compliant solution ensures that the Derived class is thread-safe by overriding the synchronized doSomething() method of the Base class with a method that synchronizes on a private final lock object.

Code Block
bgColor#ccccff
class Base {

  public synchronized void doSomething() {
    // ...
  }
}

class Derived extends Base {
  private final Object lock = new Object();

  @Override public void doSomething() {
    synchronized (lock) {
      // ...	
    }
  }
}

class

This is an acceptable solution, provided the locking policy of the Derived class is consistent with that of the Base class.

Noncompliant Code Example (Private Lock)

This noncompliant code example defines a doSomething() method in the Base class that uses a private final lock in accordance with LCK00-J. Use private final lock objects to synchronize classes that may interact with untrusted code.

Code Block
bgColor#FFCCCC
 Sub extends Base {
  public void doSomething() {
    // ...
  }
}
{code}

The {{doSomething()}} method of class {{Sub}} cannot be safely used by multiple threads because it is not thread-safe. 

h2. Compliant Solution (private lock)

This compliant solution synchronizes the {{doSomething()}} method of the subclass. 

{code:bgColor=#ccccff}
public class Base {
  private final Object lock = new Object();

  public void doSomething() {
    synchronized (lock) {
      // ...	
    }
  }
}

class SubDerived extends Base {
  privateLogger finallogger Object lock = new Object();// Initialize

  @Override public void doSomething() {
    synchronized (lock)try {
      // ...super.doSomething();
    }
 finally }
}
{code}

Note  that the {{Base.lock}} and {{Sub.lock}} objects are distinct and inaccessible from each others' classes. Consequently, {{Sub}} can provide thread-safety guarantees independent of {{Base}} logger.log(Level.FINE, "Did something");
    }
  }
}

It is possible for multiple threads to cause the entries to be logged in an order that differs from the order in which the tasks are performed. Consequently, the doSomething() method of the Derived class cannot be used safely by multiple threads because it is not thread-safe.

Compliant Solution (Private Lock)

This compliant solution synchronizes the doSomething() method of the subclass using its own private final lock object:

Code Block
bgColor#ccccff
class Base {
  private final Object lock = new Object();

  public void doSomething() {
    synchronized (lock) {
      .

h2. Exceptions

*EX1*: If the class {{Base}} performs a single operation, such as calling the corresponding super class method, or throwing an exception, it need not be synchronized.

{code:bgColor=#ccccff}
// ...
class Sub extends Base {
  public void doSomething() {
    super.doSomething(); }
  }
}
{code}

*EX2*: If the class {{Base}} calls multiple thread-safe APIs that are independent of each other, that is, they do not operate on the same shared data, this guideline may be violated.

{code:bgColor=#ccccff}
// ...
class Sub extends Base {
  public void doSomething() {

class Derived extends Base {
  Logger logger = // Initialize

  private final Object lock = new Object();

  @Override public void doSomething() {
    synchronized (lock) {
      try {
        super.doSomething();
      } finally() {
        logger.log(Level.FINE, "Did something");
  // Independent of previous statements}
    }
  }
}
{code}


h2. Risk Assessment

Overriding thread-safe methods with thread-unsafe methods can result in unexpected behavior.

|| Rule || Severity || Likelihood || Remediation Cost || Priority || Level ||
| CON10- J | low | probable | medium | {color:green}{*}P4{*}{color} | {color:green}{*}L3{*}{color} |

h3. Automated Detection

TODO

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+CON06-J].

h2. References

\[[API 06|AA. Java References#API 06]\]
\[[SDN 08|AA. Java References#SDN 08]\] Sun bug database, [Bug ID 4294756|http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4294756]

----
[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!|VOID CON09-J. Do not invoke alien methods that rely on invariants protected by the same lock object]      [!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!|CON11-J. Do not assume that declaring an object volatile guarantees visibility of its members]

Note that the Base and Derived objects maintain distinct locks that are inaccessible from each other's classes. Consequently, Derived can provide thread-safety guarantees independent of Base.

Risk Assessment

Overriding thread-safe methods with methods that are unsafe for concurrent access can result in unexpected behavior.

Rule

Severity

Likelihood

Detectable

Repairable

Priority

Level

TSM00-J

Low

Probable

Yes

No

P4

L3

Automated Detection

Sound automated detection is infeasible; heuristic checks could be useful.

ToolVersionCheckerDescription
Parasoft Jtest

Include Page
Parasoft_V
Parasoft_V

CERT.TSM00.OSNSAvoid overriding synchronized methods with non-synchronized methods

Bibliography


...

Image Added Image Added Image Added