CON51-J. Do not assume that the sleep(), yield() or getState() methods provide synchronization semantics

According to Section 17.9, "Sleep and Yield," of the Java Language Specification [[JLS 2005]],

It is important to note that neither Thread.sleep nor Thread.yield have any synchronization semantics. In particular, the compiler does not have to flush writes cached in registers out to shared memory before a call to Thread.sleep or Thread.yield, nor does the compiler have to reload values cached in registers after a call to Thread.sleep or Thread.yield.

Code that depends on thread suspension or yielding to

• flush cached registers
• reload any values
• provide any [happens-before] relationships when execution resumes
  is incorrect and is consequently disallowed. Programs must ensure that communication between threads has proper synchronization, happens-before, and visibility semantics.

Noncompliant Code Example (sleep())

This noncompliant code attempts to use the nonvolatile primitive Boolean member done as a flag to terminate execution of a thread. A separate thread sets done to true by calling the shutdown() method.

final class ControlledStop implements Runnable {
  private boolean done = false;

  @Override public void run() {
    while (!done) {
      try {
        Thread.sleep(1000);
      } catch (InterruptedException e) {
        Thread.currentThread().interrupt(); // Reset interrupted status
      }
    }
  }

  public void shutdown() {
    this.done = true;
  }
}

However, the compiler is free to read the field this.done once and to reuse the cached value in each execution of the loop. Consequently, the while loop might never terminate, even when another thread calls the shutdown() method to change the value of this.done [[JLS 2005]]. This error could have resulted from the programmer incorrectly assuming that the call to Thread.sleep() would cause cached values to be reloaded.

Compliant Solution (Volatile Flag)

This compliant solution declares the flag field volatile to ensure that updates to its value are made visible across multiple threads.

final class ControlledStop implements Runnable {
  private volatile boolean done = false;

  // ...
  @Override public void run() {
  //...
  }
}

The volatile flag establishes a [happens-before] relationship between this thread and any other thread that sets done.

Compliant Solution (Thread.interrupt())

A better solution for methods that call sleep() is to use thread interruption, which causes the sleeping thread to wake immediately and handle the interruption. Note that the interrupting thread must know which thread to interrupt; logic for tracking this relationship has been elided.

final class ControlledStop implements Runnable {

  @Override public void run() {
    // record current thread, so others can interrupt it
    myThread = currentThread();
    while (!Thread.interrupted()) {
      try {
        Thread.sleep(1000);
      } catch (InterruptedException e) {
        Thread.currentThread().interrupt();
      }
    }
  }

  public void shutdown(Thread th) {
    th.interrupt();
  }
}

Noncompliant Code Example (getState())

This noncompliant code example contains a doSomething() method that starts a thread. The thread supports interruption by checking a volatile flag and blocks waiting until notified. The stop() method checks to see whether the thread is blocked on the wait; if so, it sets the flag to true and notifies the thread so that the thread can terminate.

public class Waiter {
  private Thread thread;
  private volatile boolean flag;
  private final Object lock = new Object();

  public void doSomething() {
    thread = new Thread(new Runnable() {
      @Override public void run() {
        synchronized(lock) {
          while (!flag) {
            try {
              lock.wait();
              // ...
            } catch (InterruptedException e) {
              // Forward to handler
            }
          }
        }
      }
    });
    thread.start();
  }

  public boolean stop() {
    if (thread != null) {
      if (thread.getState() == Thread.State.WAITING) {
        flag = true;
        synchronized (lock) {
          lock.notifyAll();
        }
        return true;
      }
    }
    return false;
  }
}

Unfortunately, the stop() method incorrectly uses the Thread.getState() method to check whether the thread is blocked and has not terminated before delivering the notification. Using the Thread.getState() method for synchronization control, such as checking whether a thread is blocked on a wait, is inappropriate. Java Virtual Machines (JVMs) are permitted to implement blocking using spin-waiting; consequently, a blocked thread may never enter the WAITING or TIMED_WAITING state [[Goetz 2006]]. Because the thread may never enter the WAITING state, the stop() method might fail to terminate the thread.

Compliant Solution

This compliant solution removes the check for determining whether the thread is in the WAITING state. This check is unnecessary because invoking notifyAll() affects only threads that are blocked on a wait() invocation.

public class Waiter {
  // ...
  private Thread thread;
  private volatile boolean flag;
  private final Object lock = new Object();

  public boolean stop() {
    if (thread != null) {
      flag = true;
      synchronized (lock) {
        lock.notifyAll();
      }
      return true;
    }
    return false;
  }
}

Risk Assessment

Relying on the Thread class's sleep(), yield(), and getState() methods for synchronization control can cause unexpected behavior.

Related Guidelines

Bibliography

Issue Tracking

[!The CERT Oracle Secure Coding Standard for Java^button_arrow_left.png!]      [!The CERT Oracle Secure Coding Standard for Java^button_arrow_up.png!]      null