...
According
...
to
...
the
...
Java
...
Language Specification, §17.3, "Sleep and Yield" [JLS 2013],
It is important to note that neither
Thread.sleep
norThread.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 toThread.sleep
orThread.yield
, nor does the compiler have to reload values cached in registers after a call toThread.sleep
orThread.yield
.
Code that bases its concurrency safety on thread suspension or yields to processes that
- Flush cached registers,
- Reload any values,
- Or 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 safe publication 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.
Code Block | ||
---|---|---|
| ||
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 {{boolean}} flag. The programmer misconstrues that a separate thread that sets {{done}} to true causes the loop to terminate. {code:bgColor=#FFCCCC} 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 Thread.currentThread().interrupt(); } } } 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} |
The compiler, in this case, 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 2013]. This error could have resulted from the programmer incorrectly assuming that the call to Thread.sleep()
causes 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:
Code Block | ||
---|---|---|
| ||
final class ControlledStop implements Runnable { private volatile boolean done = false; @Override public void run() { // ... } {code // ... } |
The
...
volatile keyword 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.
Code Block | ||
---|---|---|
| ||
causes {{sleep()}} to wake up prematurely and handle the interruption. {code:bgColor=#ccccff} 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) { Threadth.currentThread().interrupt(); } } |
Note that the interrupting thread must know which thread to interrupt; logic for tracking this relationship has been omitted from this solution.
Noncompliant Code Example (getState()
)
This noncompliant code example contains a doSomething()
method that starts a thread. The thread supports interruption by checking a flag and waits 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.
Code Block | ||
---|---|---|
| ||
{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) { flagsynchronized = true; (lock) { synchronizedflag (lock) {= true; lock.notifyAll(); } return true; } } return false; } } {code} The {{ |
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 thread can be blocked without entering 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.
If doSomething()
and stop()
are called from different threads, the stop()
method could fail to see the initialized thread
, even though doSomething()
was called earlier, unless there is a happens-before relationship between the two calls. If the two methods are invoked by the same thread, they automatically have a happens-before relationship and consequently cannot encounter this problem.
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 an invocation of wait()
:
Code Block | ||
---|---|---|
| ||
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 { // ... private Thread thread; private volatile boolean flag; private final Object lock = new Object(); public boolean stop() { if (thread != null) { flagsynchronized = true; (lock) { synchronizedflag (lock) {= true; 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] |
Applicability
Relying on the Thread
class's sleep()
, yield()
, and getState()
methods for synchronization control can cause unexpected behavior.
Bibliography
...