Page History

{quote}
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

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

h2. Compliant Solution (Volatile Flag)

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

{code

:bgColor

=#ccccff

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

  // ...
}
{code}

The volatile flag establishes a [happens-before|BB. Definitions#happens-before order] relationship between this thread and any other thread that sets {{done

}}.


h2. Compliant Solution ({{Thread.interrupt()}})

A better solution for methods that call {{sleep()}} is to use thread interruption, which causes the sleeping thread to wake up immediately and handle the interruption.

{code

:bgColor

=#ccccff

}
final class ControlledStop implements Runnable {
  @Override public void run() {
    while (!Thread.interrupted()) {
      try {
        Thread.sleep(1000);
      } catch (InterruptedException e) {
        Thread.currentThread().interrupt();
      }
    }
  }

  public void shutdown() {
    Thread.currentThread().interrupt();
  }
}
{code}


h2. Noncompliant Code Example ({{getState()}})

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

{code

:bgColor

=#FFCCCC

}
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;
  }
}
{code}

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.This is true because a blocked thread is not always required to enter the {{WAITING}} or {{TIMED_WAITING}} state in cases where the JVM implements blocking using spin-waiting \[[Goetz 2006|AA. Java References#Goetz 06]\]. Because the thread may never enter the {{WAITING}} state, the {{stop()}} method may not terminate the thread.


h2.

 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()}} on a thread that is not blocked on a {{wait()}} invocation has no effect.

{code

:bgColor

=#ccccff

}
public class Waiter {
  // ...

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


{mc} This does not talk about invoking getState() and comparing with TERMINATE, RUNNABLE and other states. Should we? {mc}

h2. Risk Assessment

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

|| Guideline || Severity || Likelihood || Remediation Cost || Priority || Level ||
| THI00- 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+CON44-J].


h2. References

\[[JLS 2005|AA. Java References#JLS 05]\] section 17.9 "Sleep and Yield"

h2. Issue Tracking 

{tasklist:Review List} 
||Completed||Priority||Locked||CreatedDate||CompletedDate||Assignee||Name|| 
{tasklist}

----
[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!|LCK08-J. Ensure actively held locks are released on exceptional conditions]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!|12. Locking (LCK)]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!|THI01-J. Do not invoke ThreadGroup methods]