...
To
...
avoid
...
data
...
corruption
...
in
...
multithreaded
...
Java
...
programs,
...
shared
...
data
...
must
...
be
...
protected
...
from
...
concurrent
...
modifications
...
and
...
accesses.
...
Locking
...
can
...
be
...
performed
...
at
...
the
...
object
...
level
...
using
...
synchronized
...
methods,
...
synchronized
...
blocks,
...
or
...
the
...
java.util.concurrent
...
dynamic
...
lock
...
objects.
...
However,
...
excessive
...
use
...
of
...
locking
...
can
...
result
...
in
...
deadlocks.
...
| Wiki Markup |
|---|
Java does not prevent deadlocks or require their detection \[[JLS |
...
2005|AA. Java References#JLS 05]\]. |
...
Deadlock can occur when two or more threads request and release locks in different orders. Consequently, deadlock can be avoided by acquiring and releasing locks in the same order. |
...
Additionally,
...
synchronization
...
should
...
be
...
limited
...
to
...
cases
...
where
...
it
...
is
...
absolutely
...
necessary.
...
For
...
example,
...
the
...
paint()
...
,
...
dispose()
...
,
...
stop()
...
,
...
and
...
destroy()
...
methods
...
should
...
never
...
be
...
synchronized
...
in
...
an
...
applet
...
because
...
they
...
are
...
always
...
called
...
and
...
used
...
from
...
dedicated
...
threads.
...
The
...
Thread.stop()
...
and
...
Thread.destroy()
...
methods
...
are
...
deprecated.
...
For
...
more
...
information,
...
see
...
guideline THI05-J.
...
...
...
...
...
...
This guideline also applies to programs that need to work with a limited set of resources. For example, liveness issues can arise when two or more threads are waiting for each other to release resources such as database connections. These issues can be resolved by letting each waiting thread retry the operation at random intervals, until they succeed in acquiring the resource successfully.
Noncompliant Code Example (Different Lock Orders)
This noncompliant code example can deadlock because of excessive synchronization. The balanceAmount field represents the total balance amount available for a particular BankAccount object. A user is allowed to initiate an operation that atomically transfers a specified amount from one account to another.
| Code Block | ||
|---|---|---|
| ||
threads|THI05-J. Do not use Thread.stop() to terminate threads]. This guideline also applies to programs that need to work with a limited set of resources. For example, liveness issues can arise when two or more threads are waiting for each other to release resources such as database connections. These issues can be resolved by letting each waiting thread retry the operation at random intervals, until they succeed in acquiring the resource successfully. h2. Noncompliant Code Example (Different Lock Orders) This noncompliant code example can deadlock because of excessive synchronization. The {{balanceAmount}} field represents the total balance amount available for a particular {{BankAccount}} object. A user is allowed to initiate an operation that atomically transfers a specified amount from one account to another. {code:bgColor=#FFcccc} final class BankAccount { private double balanceAmount; // Total amount in bank account BankAccount(double balance) { this.balanceAmount = balance; } // Deposits the amount from this object instance to BankAccount instance argument ba private void depositAmount(BankAccount ba, double amount) { synchronized (this) { synchronized(ba) { if(amount > balanceAmount) { throw new IllegalArgumentException("Transfer cannot be completed"); } ba.balanceAmount += amount; this.balanceAmount -= amount; } } } public static void initiateTransfer(final BankAccount first, final BankAccount second, final double amount) { Thread transfer = new Thread(new Runnable() { public void run() { first.depositAmount(second, amount); } }); transfer.start(); } } {code} |
Objects
...
of
...
this
...
class
...
are
...
prone
...
to
...
deadlock.
...
An
...
attacker
...
that
...
has
...
two
...
bank
...
accounts
...
can
...
construct
...
two
...
threads
...
that
...
initiate
...
balance
...
transfers
...
from
...
two
...
different
...
BankAccount
...
object
...
instances
...
a
...
and
...
b
...
.
...
For
...
example,
...
consider
...
the
...
following
...
code:
| Code Block |
|---|
} BankAccount a = new BankAccount(5000); BankAccount b = new BankAccount(6000); initiateTransfer(a, b, 1000); // starts thread 1 initiateTransfer(b, a, 1000); // starts thread 2 {code} |
Each
...
transfer
...
is
...
performed
...
in
...
its
...
own
...
thread.
...
The
...
first
...
thread
...
atomically
...
transfers
...
the
...
amount
...
from
...
a
...
to
...
b
...
by
...
depositing
...
it
...
in
...
account
...
b
...
and
...
then
...
withdrawing
...
the
...
same
...
amount
...
from
...
a
...
.
...
The
...
second
...
thread
...
performs
...
the
...
reverse
...
operation;
...
that
...
is,
...
it
...
transfers
...
the
...
amount
...
from
...
b
...
to
...
a
...
.
...
When
...
executing
...
depositAmount()
...
,
...
the
...
first
...
thread
...
acquires
...
a
...
lock
...
on
...
object
...
a
...
.
...
The
...
second
...
thread
...
could
...
acquire
...
a
...
lock
...
on
...
object
...
b
...
before
...
the
...
first
...
thread
...
can.
...
Subsequently,
...
the
...
first
...
thread
...
would
...
request
...
a
...
lock
...
on
...
b
...
,
...
which
...
is
...
already
...
held
...
by
...
the
...
second
...
thread.
...
The
...
second
...
thread
...
would
...
request
...
a
...
lock
...
on
...
a
...
,
...
which
...
is
...
already
...
held
...
by
...
the
...
first
...
thread.
...
This
...
constitutes
...
a
...
deadlock
...
condition,
...
because
...
neither
...
thread
...
can
...
proceed.
...
This
...
noncompliant
...
code
...
example
...
may
...
or
...
may
...
not
...
deadlock,
...
depending
...
on
...
the
...
scheduling
...
details
...
of
...
the
...
platform.
...
Deadlock
...
will
...
occur
...
when
...
two
...
threads
...
request
...
the
...
same
...
two
...
locks
...
in
...
different
...
orders
...
and
...
each
...
thread
...
obtains
...
a
...
lock
...
that
...
prevents
...
the
...
other
...
thread
...
from
...
completing
...
its
...
transfer.
...
Deadlock
...
will
...
not
...
occur
...
when
...
two
...
threads
...
request
...
the
...
same
...
two
...
locks
...
but
...
one
...
thread
...
completes
...
its
...
transfer
...
before
...
the
...
other
...
thread
...
begins.
...
Similiarly,
...
deadlock
...
will
...
not
...
occur
...
if
...
the
...
two
...
threads
...
request
...
the
...
same
...
two
...
locks
...
in
...
the
...
same
...
order
...
(which
...
would
...
happen
...
if
...
they
...
both
...
transfer
...
money
...
from
...
one
...
account
...
to
...
a
...
second
...
account)
...
or
...
if
...
two
...
transfers
...
involving
...
distinct
...
accounts
...
occur
...
concurrently.
Compliant Solution (static Private Final Lock Object)
The deadlock can be avoided by synchronizing on a private static final lock object before performing any account transfers.
| Code Block | ||
|---|---|---|
| ||
h2. Compliant Solution ({{static}} Private Final Lock Object) The deadlock can be avoided by synchronizing on a {{private static final}} lock object before performing any account transfers. {code:bgColor=#ccccff} final class BankAccount { private double balanceAmount; // Total amount in bank account private static final Object lock = new Object(); BankAccount(double balance) { this.balanceAmount = balance; } // Deposits the amount from this object instance to BankAccount instance argument ba private void depositAmount(BankAccount ba, double amount) { synchronized (lock) { if (amount > balanceAmount) { throw new IllegalArgumentException("Transfer cannot be completed"); } ba.balanceAmount += amount; this.balanceAmount -= amount; } } public static void initiateTransfer(final BankAccount first, final BankAccount second, final double amount) { Thread transfer = new Thread(new Runnable() { @Override public void run() { first.depositAmount(second, amount); } }); transfer.start(); } } {code} In this |
In this scenario,
...
deadlock
...
cannot
...
occur
...
when
...
two
...
threads
...
with
...
two
...
different
...
BankAccount
...
objects
...
try
...
to
...
transfer
...
to
...
each
...
others'
...
accounts
...
simultaneously.
...
One
...
thread
...
will
...
acquire
...
the
...
private
...
lock,
...
complete
...
its
...
transfer,
...
and
...
release
...
the
...
lock
...
before
...
the
...
other
...
thread
...
can
...
proceed.
...
This
...
solution
...
comes
...
with
...
a
...
performance
...
penalty
...
because
...
a
...
private
...
static
...
lock
...
restricts
...
the
...
system
...
to
...
performing
...
only
...
one
...
transfer
...
at
...
a
...
time.
...
Two
...
transfers
...
involving
...
four
...
distinct
...
accounts
...
(with
...
distinct
...
target
...
accounts)
...
cannot
...
be
...
performed
...
concurrently.
...
This
...
penalty
...
increases
...
considerably
...
as
...
the
...
number
...
of
...
BankAccount
...
objects
...
increase.
...
Consequently,
...
this
...
solution
...
does
...
not
...
scale
...
well.
...
Compliant Solution (Ordered
...
Locks)
...
This
...
compliant
...
solution
...
ensures
...
that
...
multiple
...
locks
...
are
...
acquired
...
and
...
released
...
in
...
the
...
same
...
order.
...
It
...
requires
...
that
...
an
...
ordering
...
over
...
BankAccount
...
objects
...
is
...
available.
...
The
...
ordering
...
is
...
enforced
...
by
...
having
...
the
...
BankAccount
...
class
...
implement
...
the
...
java.lang.Comparable
...
interface
...
and
...
override
...
the
...
compareTo()
...
method.
| Code Block | ||||
|---|---|---|---|---|
| =
| |||
} final class BankAccount implements Comparable<BankAccount> { private double balanceAmount; // Total amount in bank account private final Object lock; private final long id; // Unique for each BankAccount private static long NextID = 0; // Next unused id BankAccount(double balance) { this.balanceAmount = balance; this.lock = new Object(); this.id = this.NextID++; } @Override public int compareTo(BankAccount ba) { return (this.id > ba.id) ? 1 : (this.id < ba.id) ? -1 : 0; } // Deposits the amount from this object instance to BankAccount instance argument ba public void depositAmount(BankAccount ba, double amount) { BankAccount former, latter; if (compareTo(ba) < 0) { former = this; latter = ba; } else { former = ba; latter = this; } synchronized (former) { synchronized (latter) { if (amount > balanceAmount) { throw new IllegalArgumentException("Transfer cannot be completed"); } ba.balanceAmount += amount; this.balanceAmount -= amount; } } } public static void initiateTransfer(final BankAccount first, final BankAccount second, final double amount) { Thread transfer = new Thread(new Runnable() { @Override public void run() { first.depositAmount(second, amount); } }); transfer.start(); } } {code} |
Whenever
...
a
...
transfer
...
occurs,
...
the
...
two
...
BankAccount
...
objects
...
are
...
ordered
...
so
...
that
...
the
...
first
...
object's
...
lock
...
is
...
acquired
...
before
...
the
...
second
...
object's
...
lock.
...
Consequently,
...
if
...
two
...
threads
...
attempt
...
transfers
...
between
...
the
...
same
...
two
...
accounts,
...
they
...
will
...
both
...
try
...
to
...
acquire
...
the
...
first
...
account's
...
lock
...
before
...
the
...
second
...
's
...
.
...
As
...
a
...
result,
...
one
...
thread
...
will
...
acquire
...
both
...
locks,
...
complete
...
the
...
transfer,
...
and
...
release
...
both
...
locks
...
before
...
the
...
other
...
thread
...
can
...
proceed.
...
Unlike
...
in
...
the
...
previous
...
compliant
...
solution,
...
multiple
...
transfers
...
can
...
happen concurrently,
...
as
...
long
...
as
...
they
...
involve
...
distinct
...
target
...
accounts.
...
Compliant Solution (ReentrantLock)
In this compliant solution, each BankAccount has a java.util.concurrent.locks.ReentrantLock
...
.
...
This
...
design permits
...
the
...
depositAmount()
...
method
...
to try to acquire the locks of both accounts and to release the locks if it fails and try again later.
| Code Block | ||
|---|---|---|
| ||
try acquiring both accounts' locks, but to release the locks if it fails and try again later. {code:bgColor=#ccccff} final class BankAccount { private double balanceAmount; // Total amount in bank account private final Lock lock = new ReentrantLock(); private final Random number = new Random(123L); BankAccount(double balance) { this.balanceAmount = balance; } // Deposits the amount from this object instance to BankAccount instance argument ba private void depositAmount(BankAccount ba, double amount) throws InterruptedException { while (true) { if (this.lock.tryLock()) { try { if (ba.lock.tryLock()) { try { if (amount > balanceAmount) { throw new IllegalArgumentException("Transfer cannot be completed"); } ba.balanceAmount += amount; this.balanceAmount -= amount; break; } finally { ba.lock.unlock(); } } } finally { this.lock.unlock(); } } int n = number.nextInt(1000); int TIME = 1000 + n; // 1 second + random delay to prevent livelock Thread.sleep(TIME); } } public static void initiateTransfer(final BankAccount first, final BankAccount second, final double amount) { Thread transfer = new Thread(new Runnable() { public void run() { try { first.depositAmount(second, amount); } catch (InterruptedException e) { Thread.currentThread().interrupt(); // Reset interrupted status } } }); transfer.start(); } } {code} |
Deadlock
...
is
...
impossible
...
in
...
this
...
compliant
...
solution
...
because
...
no
...
method
...
grabs
...
a
...
lock
...
and
...
holds
...
it
...
indefinitely.
...
If
...
the
...
current
...
object's
...
lock
...
is
...
acquired
...
but
...
the
...
second
...
lock
...
is
...
unavailable,
...
the
...
first
...
lock
...
is
...
released
...
and
...
the
...
thread
...
sleeps
...
for
...
some
...
specified
...
amount
...
of
...
time
...
before
...
attempting
...
to
...
reacquire
...
the
...
lock.
...
Code
...
that
...
uses
...
this
...
lock has behavior similar to that of synchronized code that uses the traditional monitor lock. ReentrantLock provides several other capabilities. For example, the tryLock() method does not block waiting, if another thread is already holding the lock. The java.util.concurrent.locks.ReentrantReadWriteLock
...
class
...
can
...
be
...
used
...
when
...
some
...
threads
...
require
...
a
...
lock
...
to
...
write
...
information,
...
while
...
other
...
threads
...
require
...
the
...
lock
...
to
...
concurrently
...
read
...
the
...
information.
...
Noncompliant Code Example (Different
...
Lock
...
Orders,
...
Recursive)
...
The
...
following
...
immutable WebRequest class encapsulates a web request received by a server:
| Code Block |
|---|
class {{WebRequest}} encapsulates a web request received by a server: {code} // Immutable WebRequest public final class WebRequest { private final long bandwidth; private final long responseTime; public WebRequest(long bandwidth, long responseTime) { this.bandwidth = bandwidth; this.responseTime = responseTime; } public long getBandwidth() { return bandwidth; } public long getResponseTime() { return responseTime; } } {code} |
Each
...
request
...
has
...
a
...
response
...
time
...
associated
...
with
...
it
...
, along
...
with
...
a
...
measurement
...
of
...
the
...
network
...
bandwidth
...
required
...
to
...
fulfill
...
the
...
request.
...
This
...
noncompliant
...
code
...
example
...
monitors
...
web
...
requests
...
and
...
provides
...
routines
...
for
...
calculating
...
the
...
average
...
bandwidth
...
and
...
response
...
time
...
required
...
to
...
service
...
incoming
...
requests.
| Code Block | ||||
|---|---|---|---|---|
| =
| |||
} public final class WebRequestAnalyzer { private final Vector<WebRequest> requests = new Vector<WebRequest>(); public boolean addWebRequest(WebRequest request) { return requests.add(new WebRequest(request.getBandwidth(), request.getResponseTime())); } public double getAverageBandwidth() { if (requests.size() == 0) { throw new IllegalStateException("The vector is empty!"); } return calculateAverageBandwidth(0, 0); } public double getAverageResponseTime() { if (requests.size() == 0) { throw new IllegalStateException("The vector is empty!"); } return calculateAverageResponseTime(requests.size() - 1, 0); } private double calculateAverageBandwidth(int i, long bandwidth) { if (i == requests.size()) { return bandwidth / requests.size(); } synchronized (requests.elementAt(i)) { bandwidth += requests.get(i).getBandwidth(); return calculateAverageBandwidth(++// Acquires locks in increasing order return calculateAverageBandwidth(++i, bandwidth); // Acquires locks in increasing order } } private double calculateAverageResponseTime(int i, long responseTime) { if (i <= -1) { return responseTime / requests.size(); } synchronized (requests.elementAt(i)) { responseTime += requests.get(i).getResponseTime(); return calculateAverageResponseTime(--i, responseTime); // Acquires locks in decreasing order } } } {code} {mc} // Hidden main() method public static void main(String[] args) { final WebRequestAnalyzer wra = new WebRequestAnalyzer(); wra.addWebRequest(new WebRequest(10,20)); wra.addWebRequest(new WebRequest(30,60)); new Thread(new Runnable() { public void run() { wra.getAverageResponseTime(); } }).start(); new Thread(new Runnable() { public void run() { wra.getAverageBandwidth(); } }).start(); } {mc} The monitoring application is built around the {{WebRequestAnalyzer}} class that maintains a list of web requests using the {{requests}} vector and includes the {{addWebRequest()}} setter method. Any thread can request the average bandwidth or average response time of all web requests by invoking the {{getAverageBandwidth()}} and {{getAverageResponseTime()}} methods. These methods use fine-grained locking by holding locks on individual elements (web requests) of the vector. These locks permit new requests to be added while the computations are still underway. Consequently, the statistics reported by the methods are accurate when they return the results. Unfortunately, this noncompliant code example is prone to deadlock because the recursive calls within the synchronized regions of these methods acquire the intrinsic locks in opposite numerical orders. That is, {{calculateAverageBandwidth()}} requests locks from index 0 up to {{requests.size()}} \- 1, whereas {{calculateAverageResponseTime()}} requests them from index {{requests.size()}} \- 1 down to 0. Because of recursion, no previously acquired locks are released by either method. Deadlock occurs when two threads call these methods out of order, because one thread calls {{calculateAverageBandwidth()}}, while the other calls {{calculateAverageResponseTime()}} before either method has finished executing. For example, if there are 20 requests in the vector, and one thread calls {{getAverageBandwidth()}}, it acquires the intrinsic lock of {{WebRequest}} 0, the first element in the vector. Meanwhile, if a second thread calls {{getAverageResponseTime()}}, it acquires the intrinsic lock of {{WebRequest}} 19, the last element in the vector. Consequently, deadlock results because neither thread can acquire all of the locks and proceed with the calculations. Note that the {{addWebRequest()}} method also has a race condition with {{calculateAverageResponseTime()}}. While iterating over the vector, new elements can be added to the vector, invalidating the results of the previous computation. This race condition can be prevented by locking on the last element of the vector (when it contains at least one element) before inserting the element. h2. Compliant Solution In this compliant solution, the two calculation methods acquire and release locks in the same order, beginning with the first web request in the vector. {code:bgColor=#ccccff} public final class WebRequestAnalyzer { private final Vector<WebRequest> requests = new Vector<WebRequest>(); public boolean addWebRequest(WebRequest request) { return requests.add(new WebRequest(request.getBandwidth(), request.getResponseTime())); } public double getAverageBandwidth() { if (requests.size() == 0) { throw new IllegalStateException("The vector is empty!"); } return calculateAverageBandwidth(0, 0); } public double getAverageResponseTime() { if (requests.size() == 0) { throw new IllegalStateException("The vector is empty!"); } return calculateAverageResponseTime(0, 0); } private double calculateAverageBandwidth(int i, long bandwidth) { if (i == requests.size()) { return bandwidth / requests.size(); } synchronized (requests.elementAt(i)) { // Acquires locks in increasing order bandwidth += requests.get(i).getBandwidth(); return calculateAverageBandwidth(++i, bandwidth); } } private double calculateAverageResponseTime(int i, long responseTime) { if (i == requests.size()) { return responseTime / requests.size(); } synchronized (requests.elementAt(i)) { responseTime += requests.get(i).getResponseTime(); return calculateAverageResponseTime(++i, responseTime); // Acquires locks in increasing order } } } {code} Consequently, while one thread is calculating the average bandwidth or response time, another thread cannot interfere or induce deadlock. That is because the other thread first needs to synchronize on the first {{WebRequest}}, which cannot happen before the first calculation completes. There is no need to lock on the last element of the vector in {{addWebRequest()}}, because locks are acquired in increasing order in all the methods and because updates to the vector are reflected in the results of the computations. h2. Risk Assessment Acquiring and releasing locks in the wrong order can result in deadlock. || Rule || Severity || Likelihood || Remediation Cost || Priority || Level || | CON14\- J | low | likely | high | {color:green}{*}P3{*}{color} | {color:green}{*}L3{*}{color} | h3. Automated Detection [SureLogic Flashlight|http://www.surelogic.com/] can detect violations of this guideline. It flags both the noncompliant code examples by specifying: "potential for deadlock." The Coverity Prevent Version 5.0 *LOCK_INVERSION*, *LOCK_ORDERING* checker can detect the instance where threads may try to acquire two locks in different orders, potentially causing deadlock. h3. Related Vulnerabilities Any vulnerabilities resulting from the violation of this rule are listed on the [CERT website|https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+CON34-J]. h2. References \[[JLS 05|AA. Java References#JLS 05]\] [Chapter 17, Threads and Locks|http://java.sun.com/docs/books/jls/third_edition/html/memory.html] \[[Halloway 00|AA. Java References#Halloway 00]\] \[[MITRE 09|AA. Java References#MITRE 09]\] [CWE ID 412|http://cwe.mitre.org/data/definitions/412.html] "Unrestricted Lock on Critical Resource" ---- [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!|LCK06-J. Do not use an instance lock to protect shared static data] [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!|12. Locking (LCK)] [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!|TSM00-J. Do not override thread-safe methods with methods that are not thread-safe]return calculateAverageResponseTime(--i, responseTime); } } } |
The monitoring application is built around the WebRequestAnalyzer class that maintains a list of web requests using the requests vector and includes the addWebRequest() setter method. Any thread can request the average bandwidth or average response time of all web requests by invoking the getAverageBandwidth() and getAverageResponseTime() methods.
These methods use fine-grained locking by holding locks on individual elements (web requests) of the vector. These locks permit new requests to be added while the computations are still underway. Consequently, the statistics reported by the methods are accurate when they return the results.
Unfortunately, this noncompliant code example is prone to deadlock because the recursive calls within the synchronized regions of these methods acquire the intrinsic locks in opposite numerical orders. That is, calculateAverageBandwidth() requests locks from index 0 up to requests.size() - 1, whereas calculateAverageResponseTime() requests them from index requests.size() - 1 down to 0. Because of recursion, no previously acquired locks are released by either method. Deadlock occurs when two threads call these methods out of order, because one thread calls calculateAverageBandwidth(), while the other calls calculateAverageResponseTime() before either method has finished executing.
For example, if there are 20 requests in the vector, and one thread calls getAverageBandwidth(), the thread acquires the intrinsic lock of WebRequest 0, the first element in the vector. Meanwhile, if a second thread calls getAverageResponseTime(), it acquires the intrinsic lock of WebRequest 19, the last element in the vector. Consequently, deadlock results because neither thread can acquire all of the locks and proceed with the calculations.
Note that the addWebRequest() method also has a race condition with calculateAverageResponseTime(). While iterating over the vector, new elements can be added to the vector, invalidating the results of the previous computation. This race condition can be prevented by locking on the last element of the vector (when it contains at least one element) before inserting the element.
Compliant Solution
In this compliant solution, the two calculation methods acquire and release locks in the same order, beginning with the first web request in the vector.
| Code Block | ||
|---|---|---|
| ||
public final class WebRequestAnalyzer {
private final Vector<WebRequest> requests = new Vector<WebRequest>();
public boolean addWebRequest(WebRequest request) {
return requests.add(new WebRequest(request.getBandwidth(), request.getResponseTime()));
}
public double getAverageBandwidth() {
if (requests.size() == 0) {
throw new IllegalStateException("The vector is empty!");
}
return calculateAverageBandwidth(0, 0);
}
public double getAverageResponseTime() {
if (requests.size() == 0) {
throw new IllegalStateException("The vector is empty!");
}
return calculateAverageResponseTime(0, 0);
}
private double calculateAverageBandwidth(int i, long bandwidth) {
if (i == requests.size()) {
return bandwidth / requests.size();
}
synchronized (requests.elementAt(i)) { // Acquires locks in increasing order
bandwidth += requests.get(i).getBandwidth();
return calculateAverageBandwidth(++i, bandwidth);
}
}
private double calculateAverageResponseTime(int i, long responseTime) {
if (i == requests.size()) {
return responseTime / requests.size();
}
synchronized (requests.elementAt(i)) {
responseTime += requests.get(i).getResponseTime();
// Acquires locks in increasing order
return calculateAverageResponseTime(++i, responseTime);
}
}
}
|
Consequently, while one thread is calculating the average bandwidth or response time, another thread cannot interfere or induce deadlock. That is because the other thread first needs to synchronize on the first WebRequest, which cannot happen before the first calculation completes.
There is no need to lock on the last element of the vector in addWebRequest() for two reasons: (1)because locks are acquired in increasing order in all the methods and (2)because updates to the vector are reflected in the results of the computations.
Risk Assessment
Acquiring and releasing locks in the wrong order can result in deadlock.
Guideline | Severity | Likelihood | Remediation Cost | Priority | Level |
|---|---|---|---|---|---|
LCK07- J | low | likely | high | P3 | L3 |
References
| Wiki Markup |
|---|
\[[JLS 2005|AA. Java References#JLS 05]\] [Chapter 17, Threads and Locks|http://java.sun.com/docs/books/jls/third_edition/html/memory.html]
\[[Halloway 2000|AA. Java References#Halloway 00]\]
\[[MITRE 2009|AA. Java References#MITRE 09]\] [CWE ID 412|http://cwe.mitre.org/data/definitions/412.html] "Unrestricted Lock on Critical Resource" |
...