...
According
...
to
...
Section
...
17.9,
...
"Sleep
...
and
...
Yield,"
...
of
...
the
...
Java
...
Language
...
Specification
...
[JLS
...
...
],
...
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.
Code that depends on thread suspension or yielding to
- flush cached registers
- reload any values
- 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 visibility 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 | ||
|---|---|---|
| ||
}}. {quote} Code that depends on thread suspension or yielding to * flush cached registers * reload any values * provide any [happens-before|BB. Glossary#happens-before order] relationships when execution resumes is incorrect and is consequently disallowed. Programs must ensure that communication between threads has proper synchronization, happens-before, and visibility semantics. h2. 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: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} |
However,
...
the
...
compiler
...
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
...
[
...
...
...
].
...
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
...
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
...
flag
...
establishes
...
a
...
...
...
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.
...
Note
...
that
...
the
...
interrupting
...
thread
...
must
...
know
...
which
...
thread
...
to
...
interrupt;
...
logic
...
for
...
tracking
...
this
...
relationship
...
has
...
been
...
elided.
| Code Block | ||||
|---|---|---|---|---|
| =
| |||
} 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) { th.interrupt(); } } {code} h2. Noncompliant Code Example ({{ |
Noncompliant Code Example (getState()
...
)
...
This
...
noncompliant
...
code
...
example
...
contains
...
a
...
doSomething()
...
method
...
that
...
starts
...
a
...
thread.
...
The
...
thread
...
supports
...
interruption
...
by
...
checking
...
a
...
volatile
...
flag
...
and
...
blocks
...
waiting
...
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 | ||||
|---|---|---|---|---|
| =
| |||
} 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.
...
Java
...
Virtual
...
Machines
...
(JVMs)
...
are
...
permitted
...
to
...
implement
...
blocking
...
using
...
spin-waiting;
...
consequently,
...
a
...
blocked
...
thread
...
may
...
never
...
enter
...
the
...
WAITING
...
or
...
TIMED_WAITING
...
state
...
[
...
...
...
].
...
Because
...
the
...
thread
...
may
...
never
...
enter
...
the
...
WAITING
...
state,
...
the
...
stop()
...
method
...
might
...
fail
...
to
...
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() affects only threads that are blocked on a wait() invocation.
| 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()}} affects only threads that are blocked on a {{wait()}} invocation. {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) { 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. || Rule || Severity || Likelihood || Remediation Cost || Priority || Level || | THI00-J | low | probable | medium | {color:green}{*}P4{*}{color} | {color:green}{*}L3{*}{color} | h2. Related Guidelines | [MITRE CWE|http://cwe.mitre.org/] | [CWE ID 821|http://cwe.mitre.org/data/definitions/821.html], "Incorrect Synchronization" | h2. Bibliography | \[[JLS 2005|AA. References#JLS 05]\] | Section 17.9, "Sleep and Yield" | h2. Issue Tracking {tasklist:Review List} |
Risk Assessment
Relying on the Thread class's sleep(), yield(), and getState() methods for synchronization control can cause unexpected behavior.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
|---|---|---|---|---|---|
THI00-J | low | probable | medium | P4 | L3 |
Related Guidelines
CWE ID 821, "Incorrect Synchronization" |
Bibliography
[JLS 2005] | Section 17.9, "Sleep and Yield" |
Issue Tracking
| Tasklist | ||||
|---|---|---|---|---|
| ||||
||Completed||Priority||Locked||CreatedDate||CompletedDate||Assignee||Name||
{tasklist}
----
[!The CERT Oracle Secure Coding Standard for Java^button_arrow_left.png!|java:09. Thread APIs (THI)] [!The CERT Oracle Secure Coding Standard for Java^button_arrow_up.png!|java:09. Thread APIs (THI)] [!The CERT Oracle Secure Coding Standard for Java^button_arrow_right.png!|java:THI01-J. Do not invoke ThreadGroup methods]
|
...