CON10-J. Do not override thread-safe methods with methods that are not thread-safe

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 provides an implementation that is not safe for concurrent use.

The guideline CON04-J. Use private final lock objects to synchronize classes that may interact with untrusted code recommends documenting the locking strategy of classes designed for inheritance. This information is useful for deciding the locking strategy of subclasses.

Noncompliant Code Example (synchronized method)

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

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

class Derived extends Base {
  public void doSomething() {
    // ...		
  }
}

The doSomething() method of class Base can be safely used by multiple threads. However, if a subclass Derived 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.

Compliant Solution (synchronized method)

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

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

class Derived extends Base {
  public synchronized void doSomething() {
    // ...		
  }
}

This compliant solution does not violate CON04-J. Use private final lock objects to synchronize classes that may interact with untrusted code because the accessibility of the class is package-private which is allowable when untrusted code cannot infiltrate the package.

Noncompliant Code Example (private lock)

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

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

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

class Derived extends Base {
  public void doSomething() {
    try {
      super.doSomething();
    } finally {
      logger.log(Level.FINE, "Did something"); 
    }
  }
}

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

Compliant Solution (private lock)

This compliant solution synchronizes the doSomething() method of the subclass using a private final lock object.

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

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

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

  public void doSomething() {
    synchronized (lock) {
      try {
        super.doSomething();
      } finally {
        logger.log(Level.FINE, "Did something"); 
      }
    }
  }
}

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

Risk Assessment

Overriding thread-safe methods with methods that are not thread-safe can result in unexpected behavior.

Automated Detection

TODO

Related Vulnerabilities

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

References

[[API 06]]
[[SDN 08]] Sun bug database, Bug ID 4294756

VOID CON09-J. Do not invoke alien methods that rely on invariants protected by the same lock object      11. Concurrency (CON)      CON11-J. Do not assume that declaring an object reference volatile guarantees visibility of its members