Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Parasoft Jtest 2021.1

Threads that invoke Object.wait() expect to wake up and resume execution when their condition predicate becomes true. To be compliant with rule THI03-J. Always invoke wait() and await() methods inside a loop, waiting threads must test their condition predicates upon receiving notifications and must resume waiting if the predicates are false.

...

The java.util.concurrent.locks utilities provide the Condition.signal() and Condition.signalAll() methods to awaken threads that are blocked on a Condition.await() call. Condition objects are required when using java.util.concurrent.locks.Lock objects. Although Lock objects allow the use of Object.wait(), Object.notify(), and Object.notifyAll() methods, however this their use is prohibited by rule "LCK03-J. Do not synchronize on the intrinsic locks of high-level concurrency objects". Code that synchronizes using a Lock object uses one or more Condition objects associated with the Lock object rather than using its own intrinsic lock. These objects interact directly with the locking policy enforced by the Lock object. Consequently, the await(), signal(), and signalAll() methods are used in place of the wait(), notify(), and notifyAll() methods.

...

  • The Condition object is identical for each waiting thread.
  • All threads must perform the same set of operations after waking up. This , which means that any one thread can be selected to wake up and resume for a single invocation of signal().
  • Only one thread is required to wake upon receiving the signal.

...

When used securely, the signal() method has better performance than signalAll().

When notify() or signal() is used to waken a waiting thread, and the thread is not prepared to resume execution, it often resumes waiting. Consequently, no thread wakens, which may cause the system to hang.

Noncompliant Code Example (notify())

This noncompliant code example shows a complex, multistep process being undertaken by several threads. Each thread executes the step identified by the time field. Each thread waits for the time field to indicate that it is time to perform the corresponding thread's step. After performing the step, each thread first increments time and then notifies the thread that is responsible for the next step.

Code Block
bgColor#FFcccc

public final class ProcessStep implements Runnable {
  private static final Object lock = new Object();
  private static int time = 0;
  private final int step; // Do Perform operations when field time 
                          // reaches this value

  public ProcessStep(int step) {
    this.step = step;
  }

  @Override public void run() {
    try {
      synchronized (lock) {
        while (time != step) {
          lock.wait();
        }

        // Perform operations

        time++;
        lock.notify();
      }
    } catch (InterruptedException ie) {
      Thread.currentThread().interrupt(); // Reset interrupted status
    }
  }

  public static void main(String[] args) {
    for (int i = 4; i >= 0; i--) {
      new Thread(new ProcessStep(i)).start();
    }
  }
}

...

Only the run() method from the noncompliant code example is modified, as follows:

Code Block
bgColor#ccccff

public final class ProcessStep implements Runnable {
  private static final Object lock = new Object();
  private static int time = 0;
  private final int step; // Perform operations when field time 
                          // reaches this value
  public ProcessStep(int step) {
    this.step = step;
  }

  @Override public void run() {
    try {
      synchronized (lock) {
        while (time != step) {
          lock.wait();
        }
  
        // Perform operations
  
        time++;
        lock.notifyAll(); // Use notifyAll() instead of notify()
      }
    } catch (InterruptedException ie) {
      Thread.currentThread().interrupt(); // Reset interrupted status
    }
  }

}

...

This noncompliant code example is similar to the noncompliant code example for notify() but uses the Condition interface for waiting and notification.:

Code Block
bgColor#FFcccc

public class ProcessStep implements Runnable {
  private static final Lock lock = new ReentrantLock();
  private static final Condition condition = lock.newCondition();
  private static int time = 0;
  private final int step; // Perform operations when field time 
                          // reaches this value
  public ProcessStep(int step) {
    this.step = step;
  }

  @Override public void run() {
    lock.lock();
    try {
      while (time != step) {
        condition.await();
      }

      // Perform operations

      time++;
      condition.signal();
    } catch (InterruptedException ie) {
      Thread.currentThread().interrupt(); // Reset interrupted status
    } finally {
      lock.unlock();
    }
  }

  public static void main(String[] args) {
    for (int i = 4; i >= 0; i--) {
      new Thread(new ProcessStep(i)).start();
    }
  }
}

...

This compliant solution uses the signalAll() method to notify all waiting threads. Before await() returns, the current thread reacquires the lock associated with this condition. When the thread returns, it is guaranteed to hold this lock [API 20062014]. The thread that is ready can perform its task while all the threads whose condition predicates are false resume waiting.

Only the run() method from the noncompliant code example is modified, as follows:

Code Block
bgColor#ccccff

public class ProcessStep implements Runnable {
  private static final Lock lock = new ReentrantLock();
  private static final Condition condition = lock.newCondition();
  private static int time = 0;
  private final int step; // Perform operations when field time 
                          // reaches this value
  public ProcessStep(int step) {
    this.step = step;
  }

  @Override public void run() {
    lock.lock();
    try {
      while (time != step) {
        condition.await();
      }
  
      // Perform operations

      time++;
      condition.signalAll();
    } catch (InterruptedException ie) {
      Thread.currentThread().interrupt(); // Reset interrupted status
    } finally {
      lock.unlock();
    }
  }

}

...

This compliant solution assigns each thread its own condition. All the Condition objects are accessible to all the threads.:

Code Block
bgColor#ccccff

// Declare class as final because its constructor throws an exception
public final class ProcessStep implements Runnable {
  private static final Lock lock = new ReentrantLock();
  private static int time = 0;
  private final int step; // Perform operations when field time 
                          // reaches this value
  private static final int MAX_STEPS = 5;
  private static final Condition[] conditions = new Condition[MAX_STEPS];

  public ProcessStep(int step) {
    if (step <= MAX_STEPS) {
      this.step = step;
      conditions[step] = lock.newCondition();
    } else {
      throw new IllegalArgumentException("Too many threads");
    }
  }

  @Override public void run() {
    lock.lock();
    try {
      while (time != step) {
        conditions[step].await();
      }

      // Perform operations

      time++;
      if (step + 1 < conditions.length) {
        conditions[step + 1].signal();
      }
    } catch (InterruptedException ie) {
      Thread.currentThread().interrupt(); // Reset interrupted status
    } finally {
      lock.unlock();
    }
  }

  public static void main(String[] args) {
    for (int i = MAX_STEPS - 1; i >= 0; i--) {
      ProcessStep ps = new ProcessStep(i);
      new Thread(ps).start();
    }
  }
}

...

This compliant solution is safe only when untrusted code cannot create a thread with an instance of this class.

...

Notifying a single thread rather than all waiting threads can violate the the liveness property of the system.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

THI02-J

low

Low

unlikely

Unlikely

medium

Medium

P2

L3

Related Guidelines

Bibliography

Automated Detection

ToolVersionCheckerDescription
Parasoft Jtest
Include Page
Parasoft_V
Parasoft_V
CERT.THI02.ANFDo not use 'notify()'; use 'notifyAll()' instead so that all waiting threads will be notified
SonarQube
Include Page
SonarQube_V
SonarQube_V
S2446"notifyAll" should be used

Related Guidelines

Bibliography

[API 2006]

Interface java.util.concurrent.locks.Condition

[Bloch 2001]

Item 50, "Never Invoke wait Outside a Loop"

[API 2006]

java.util.concurrent.locks.Condition interface

[JLS 2005]

Chapter 17, Threads and Locks

[Goetz 2006]

Section 14.2.4, "Notification"

[

Bloch 2001] Item 50. Never invoke wait outside a loop

JLS 2015]

Chapter 17, "Threads and Locks"


...

Image Modified Image Modified Image Modified