...
This noncompliant code example can deadlock because of excessive synchronization. The balanceAmount field represents the total balance amount available for a particular BankAccount object. Users are allowed to initiate an operation that atomically transfers a specified amount from one account to another.
| Code Block | ||
|---|---|---|
| ||
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();
}
}
|
Objects of this class are prone to deadlock. An attacker who 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);
BankAccount.initiateTransfer(a, b, 1000); // starts thread 1
BankAccount.initiateTransfer(b, a, 1000); // starts thread 2
|
...
This compliant solution avoids deadlock by synchronizing on a private static final lock object before performing any account transfers.
| Code Block | ||
|---|---|---|
| ||
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();
}
}
|
...
This compliant solution ensures that multiple locks are acquired and released in the same order. It requires a consistent ordering over BankAccount objects. Consequently, the BankAccount class implements the java.lang.Comparable interface and overrides 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();
}
}
|
...
In this compliant solution, each BankAccount has a java.util.concurrent.locks.ReentrantLock. This design permits the depositAmount() method to attempt to acquire the locks of both accounts, to release the locks if it fails, and to try again later if necessary.
| Code Block | ||
|---|---|---|
| ||
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 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();
}
}
|
...
The following immutable WebRequest class encapsulates a web request received by a server:
| Code Block |
|---|
// 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;
}
}
|
...
This noncompliant code example monitors web requests and provides routines for calculating the average bandwidth and response time required to serve 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();
// Acquires locks in increasing order
return calculateAverageBandwidth(++i, bandwidth);
}
}
private double calculateAverageResponseTime(int i, long responseTime) {
if (i <= -1) {
return responseTime / requests.size();
}
synchronized (requests.elementAt(i)) {
responseTime += requests.get(i).getResponseTime();
// Acquires locks in decreasing order
return calculateAverageResponseTime(--i, responseTime);
}
}
}
|
...
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)) {
// Acquires locks in increasing order
responseTime += requests.get(i).getResponseTime();
return calculateAverageResponseTime(++i, responseTime);
}
}
}
|
...
Some static analysis tools can detect violations of this rule.
| Tool | Version | Checker | Description | ||||||
|---|---|---|---|---|---|---|---|---|---|
| ThreadSafe |
| CCE_DL_DEADLOCK | Implemented |
Related Guidelines
CWE-833. Deadlock |
...