You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 22 Next »

According to the Java Language Specification [[JLS 05]], section 17.9 "Sleep and Yield":

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.

The assumption that thread suspension and yielding flush the cached registers and reload the values when execution resumes, is misleading and paves the way for potential coding errors.

The Thread.getState() method returns the current state of a thread. Using this method for synchronization control such as checking whether a thread is blocked on a wait is inappropriate because a blocked thread is not always required to enter the WAITING or TIMED_WAITING state in cases where the JVM chooses to implement the blocking using spin-waiting [[Goetz 06]].

Noncompliant Code Example (sleep())

This noncompliant code example declares a non-volatile Boolean flag. The programmer assumes

Unknown macro: {mc}

incorrectly

that a separate thread sets done to true, causing the loop to terminate.

private Boolean done;
while (!this.done) {
  Thread.sleep(1000);
}

"The compiler is free to read the field this.done just once, and reuse the cached value in each execution of the loop. This would mean that the loop would never terminate, even if another thread changed the value of this.done." [[JLS 05]]. This occurs because Thread.sleep() does not establish a [happens-before] relation.

Compliant Solution

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

private volatile Boolean done;
while (!this.done) {
  Thread.sleep(1000);
}

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

Noncompliant Code Example (getState())

This noncompliant code example defines a doSomething() method that starts a thread. 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.

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

The stop() method incorrectly uses the Thread.getState() method to check whether the thread is still blocked and has not terminated, before delivering the notification. Because the thread may never enter the WAITING state, the stop() method may not 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() on a thread that is not blocked on a wait() invocation has no effect.

public class Waiter {
  // ...

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

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

Risk Assessment

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

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

CON16- J

low

probable

medium

P4

L3

Automated Detection

TODO

Related Vulnerabilities

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

References

[[JLS 05]] section 17.9 "Sleep and Yield"


[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!]

  • No labels