According to the Java Language Specification \[[JLS 05|AA. Java References#JLS 05]\], section 17.9 "Sleep and Yield":
{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}}.
{quote}
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|AA. Java References#Goetz 06]\].
h2. Noncompliant Code Example ({{sleep()}})
This noncompliant code example declares a non-volatile {{Booleanboolean}} flag. The programmer assumes {mc} incorrectly {mc} misconstrues that a separate thread that sets {{done}} to true, causingcauses the loop to terminate.
{code:bgColor=#FFCCCC}
final class ControlledStop implements Runnable {
private boolean done = false;
public void run() {
while (!this.done) {
try {
Thread.sleep(1000);
} catch (InterruptedException e) {
Thread.currentThread().interrupt(); // ...handle error...Reset interrupted status
}
}
}
public void shutdown() {
this.done = true;
}
}
{code}
"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|AA. Java References#JLS 05]\]. This occurs because {{Thread.sleep()}} does not establish a [happens-before|BB. Definitions#happens-before order] relation.
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] relation between this thread and any other thread that sets {{done}}.
h2. Compliant Solution ({{Thread.interrupt()}})
A better approach for routines that use {{sleep()}} is to use thread interruption, which willcauses cause {{sleep()}} to wake up prematurely and handle the interruption.
{code:bgColor=#ccccff}
final class ControlledStop implements Runnable {
@Override public void run() {
dowhile (!Thread.interrupted()) {
try {
Thread.sleep(1000);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
} while (!Thread.interrupted());
}
public void shutdown() {
Thread.currentThread().interrupt();
}
}
{code}
h2. 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.
{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}
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.
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 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 | {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 05|AA. Java References#JLS 05]\] section 17.9 "Sleep and Yield"
h2. Issue Tracking
{tasklist:Review List}
||Completed||Priority||Locked||CreatedDate||CompletedDate||Assignee||Name||
|F|M|F|1270214574335| |svoboda|"The programmer assumes that a separate thread sets done to true, *causing the loop to terminate.*" => this does not cause the loop to terminate; should be eliminated...or reword as "The programmer incorrectly assumes that the loop is terminated when another thread sets done to true"|
{tasklist}
----
[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!|CON15-J. Ensure actively held locks are released on exceptional conditions] [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!|11. Concurrency (CON)] [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!|CON17-J. Avoid using ThreadGroup APIs]
| 