| Wiki Markup |
|---|
According to Section 17.9, "Sleep and Yield" of the Java Language Specification \[[JLS |
...
2005|AA. Java References#JLS 05]\]: |
...
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.
Incorrectly assuming that thread suspension and yielding do any of the following can result in unexpected behavior:
*flush the cached registers
*reload any values
*provide any happens-before relationships when execution resumes
Noncompliant Code Example (sleep())
This noncompliant code attempts to use a non-volatile Boolean done as a flag to terminate execution of a thread. A separate thread sets done to true by calling the shutdown() method.
| Code Block | ||
|---|---|---|
| ||
}}. {quote} Incorrectly assuming that thread suspension and yielding flush the cached registers, reload any values, or provide any [happens-before|BB. Definitions#happens-before order] relations when execution resumes can result in unexpected behavior. h2. Noncompliant Code Example ({{sleep()}}) This noncompliant code attempts to use a non-volatile {{boolean done}} as a flag to terminate execution of a thread. A separate thread sets {{done}} to true by calling {{shutdown()}}. {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 } } } public void shutdown() { this.done = true; } } {code} |
| Wiki Markup |
|---|
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. |
Compliant Solution (Volatile Flag)
This compliant solution declares the flag volatile to ensure that updates to it are made visible across multiple threads.
| Code Block | ||
|---|---|---|
| ||
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 ({{ |
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
...
up
...
immediately
...
and
...
handle
...
the
...
interruption.
| Code Block | ||||
|---|---|---|---|---|
| =
| |||
} 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 ({{ |
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 Block | ||||
|---|---|---|---|---|
| =
| |||
} 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} |
| Wiki Markup |
|---|
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. |
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 Block | ||
|---|---|---|
| ||
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 || | CON18\- 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 |
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 | P4 | L3 |
References
| Wiki Markup |
|---|
\[[JLS 2005|AA. Java References#JLS 05]\] section 17.9 "Sleep and Yield" |
...
...