Page History

To avoid data corruption in multithreaded Java programs, shared data must be protected from concurrent modifications and accesses. This Locking can be performed at the object level by using synchronized methods or , synchronized blocks, or by using the java.util.concurrent dynamic lock objects. However, excessive use of locking may can result in deadlocks (See CON08-J. Do not call alien methods that synchronize on the same objects as any callers in the execution chain). For instance, to avoid deadlocks, the .

Java neither prevents deadlocks nor requires their detection [JLS 2015]. Deadlock can occur when two or more threads request and release locks in different orders. Consequently, programs are required to avoid deadlock by acquiring and releasing locks in the same order.

Additionally, synchronization should be limited to cases where it is absolutely necessary. For example, the paintpaint(), dispose(), stop(), and destroy() methods should never be synchronized in an applet should not be synchronized because they are always called and used from dedicated threads.

"The Java programming language neither prevents nor requires detection of deadlock conditions." \[[JLS 05|AA. Java References#JLS 05]\]. Deadlocks can arise when two or more threads request and release locks in different orders. 

Noncompliant Code Example

Furthermore, the Thread.stop() and Thread.destroy() methods are deprecated (see THI05-J. Do not use Thread.stop() to terminate threads for more information).

This rule 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 successfully acquire the resource.

Noncompliant Code Example (Different Lock Orders)

This noncompliant code example can deadlock because of excessive synchronization. The balanceAmount field represents the total balance available for a particular BankAccount object. Users are allowed to initiate an operation that atomically transfers a specified amount from one account to anotherThis noncompliant code example can deadlock because of excessive synchronization. Assume that an attacker has two bank accounts and is capable of requesting two depositAllAmount() operations in succession, one each from the two threads started in main().

final class BankAccount {
  private intdouble balanceAmount;  // Total amount in bank account
	 
  private BankAccount(intdouble balance) {
    this.balanceAmount = balance;
  }

  // Deposits the amount from this object instance
  // to BankAccount instance argument ba 
  private void depositAllAmountdepositAmount(BankAccount ba, double amount) {
    synchronized (this) {
      synchronized (ba) {
        ba.balanceAmount += this.balanceAmount; if (amount > balanceAmount) {
        this.balanceAmount = 0;throw // withdraw all amount from this instance
new IllegalArgumentException(
               "Transfer  ba.displayAllAmount();  // Display the new balanceAmount in ba (may cause deadlock)
cannot be completed"
          );
        }
     } 
  }
  ba.balanceAmount += amount;
  private synchronized void displayAllAmount() {
  this.balanceAmount  System.out.println(balanceAmount)-= amount;
      }

   public static}
 void  }

  public static void initiateTransfer(final BankAccount first,
    final BankAccount second, final double amount) {

    Thread ttransfer = new Thread(new Runnable() {
        public void run() {
          first.depositAllAmountdepositAmount(second, amount);
        }
    });
    ttransfer.start();
  }
}

Objects of class BankAccount represent bank accounts. The balanceAmount field represents the total balance amount available for a particular object (bank account). A user is allowed to initiate an operation deposit all amount that transfers the balance amount from one account to another. This is equivalent to closing a bank account and transferring the balance to a different (existing or new) account.

Objects of this class are deadlock-prone. An attacker may cause the program to construct two threads that initiate balance transfers from two different BankAccount object instances, a and b. Consider the following code that does this:

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:

BankAccount a = new BankAccount(5000);
BankAccount b = new BankAccount(6000);
BankAccount.initiateTransfer(a, b, 1000

BankAccount a = new BankAccount(5000);
BankAccount b = new BankAccount(6000);
initiateTransfer(a, b); // starts thread 1
BankAccount.initiateTransfer(b, a, 1000); // starts thread 2

The two transfers are Each transfer is performed in their own threads, from instance a to b and b to aits own thread. The first thread atomically transfers the amount from a to b by depositing the balance from a to it in account b and then withdrawing the entire balance same amount from a. The second thread performs the reverse operation, ; that is, it transfers the balance amount from b to a and withdraws the balance from b. When executing depositAllAmountdepositAmount(), the first thread might acquire acquires a lock on object a while the . The second thread may could acquire a lock on object b before the first thread can. Subsequently, the first thread requests would request a lock on b, which is already held by the second thread and the . The second thread requests would request a lock on a, which is already held by the first thread. This constitutes a deadlock condition , as because neither thread can proceed.The threads in this program request monitors in varying order

This noncompliant code example may or may not deadlock, depending on the interleaving of method calls. If Thread T1 finishes executing before Thread T2, or T2 before T1, there are no issues because in these cases, locks are acquired and released in the same order. Sequences where the threads alternate, such as, T1, T2, T1, T2 may deadlock.

Compliant Solution (single private lock)

scheduling details of the platform. Deadlock occurs when (1) two threads request the same two locks in different orders, and (2) each thread obtains a lock that prevents the other thread from completing its transfer. Deadlock is avoided when two threads request the same two locks but one thread completes its transfer before the other thread begins. Similarly, deadlock is avoided 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 (Private Static Final Lock Object)

This compliant solution avoids deadlock by synchronizing on a private static final lock object before performing any account transfers:The deadlock can be avoided by using a single lock to acquire before doing any account transfers.

final class BankAccount {
  private intdouble balanceAmount;  // Total amount in bank account
	 
  private static final Object lock = new Object();

  private BankAccount(intdouble balance) {
    this.balanceAmount = balance;
    this.lock = new Object();
  }

  // Deposits the amount from this object instance
  // to BankAccount instance argument ba 
  private void depositAllAmountdepositAmount(BankAccount ba, double amount) {
    synchronized (lock) {
      ba.balanceAmount += this.balanceAmount;if (amount > balanceAmount) {
      this.balanceAmount = 0;throw // withdraw all amount from this instancenew IllegalArgumentException(
      ba.displayAllAmount();  // Display the new balanceAmount"Transfer incannot ba (may cause deadlock)be completed");
    } 
  }
  
   private void displayAllAmount() {ba.balanceAmount += amount;
    synchronized (lock) {
      System.out.println(balanceAmount)  this.balanceAmount -= amount;
    }
  }

  public static void initiateTransfer(final BankAccount first,
    final BankAccount second, final double amount) {

    Thread ttransfer = new Thread(new Runnable() {
        @Override public void run() {
          first.depositAllAmountdepositAmount(second, amount);
        }
    });
    ttransfer.start();
  }
}

In this scenario, if deadlock cannot occur when two threads with two different BankAccount objects try to tranfer transfer to each othersother's accounts simultaneously, deadlock cannot occur. One thread will acquire the private lock, complete its transfer, and release the lock , before the other thread may can proceed.

This solution comes with imposes a performance penalty , as because a private static lock restricts the system to only performing one transfer at a timeperforming transfers sequentially. Two transfers involving four distinct accounts (and with distinct target accounts) may not happen cannot be performed concurrently. The impact of this This penalty increases considerably as the number of BankAccount objects increase. Consequently, this solution does not fails to scale very well.

Compliant Solution (

...

Ordered Locks)

This compliant solution ensures that multiple locks are acquired and released in the same order. It requires that an a consistent ordering over BankAccount objects is available. The ordering is enforced by having the class BankAccount extend . Consequently, the BankAccount class implements the java.lang.Comparable interface and overriding overrides the compareTo() method.

final class BankAccount implements ComparableComparable<BankAccount> {
  private intdouble balanceAmount;  // Total amount in bank account	 
  private final Object lock;

  private BankAccount(int balance) {
    this.balanceAmount = balance;final long id; // Unique for each BankAccount
  private  this.lockstatic final AtomicLong nextID = new ObjectAtomicLong(0);
  }

  // DepositsNext the amount from this object instance to BankAccount instance argument ba 
  private void depositAllAmount(BankAccount baunused ID

  BankAccount(double balance) {
    BankAccount former, latterthis.balanceAmount = balance;
    if (compareTo(ba) < 0) {this.lock = new Object();
     this.id former = thisnextID.getAndIncrement();
  }

  @Override public latterint =compareTo(BankAccount ba;) {
    } else {
      former = ba;
      latter = thisreturn (this.id > ba.id) ? 1 : (this.id < ba.id) ? -1 : 0;
    }

  // Deposits synchronizedthe (former) {amount from this object instance
  // to BankAccount  synchronized (latterinstance argument ba
  public void depositAmount(BankAccount ba, double amount) {
    BankAccount    ba.balanceAmount += this.balanceAmountformer, latter;
    if (compareTo(ba) <  this.balanceAmount = 0; // withdraw all amount from this instance0) {
      former = this;
      latter = ba.displayAllAmount(); // Display the new balanceAmount in ba (may cause deadlock)
      } 
    }
;
    } else {
      former = ba;
      latter = this;
    }
 
  private synchronized void displayAllAmount(former) {
      synchronized System.out.println(balanceAmountlatter); {
  }

  public static void initiateTransfer(final BankAccount first,if final(amount BankAccount> secondbalanceAmount) {
    Thread t = new Thread(   throw new RunnableIllegalArgumentException() {
      public void run() {
     "Transfer cannot  first.depositAllAmount(secondbe completed");
        }
    });
    t.start();
  }

  public int compareTo(BankAccount ba) {
   if(ba.balanceAmount += amount;
        this.balanceAmount < ba.balanceAmount) {-= amount;
     return -1;}
    }
 else if(this.balanceAmount > ba.balanceAmount }

  public static void initiateTransfer(final BankAccount first,
    final BankAccount second, final double amount) {

    Thread transfer = new  return 1;
Thread(new Runnable() {
    } else {
  @Override public void return 0;run() {
   }
  }
}
       first.depositAmount(second, amount);
        }
    });
    transfer.start();
  }
}

Whenever In this compliant solution, whenever a transfer occurs, the two BankAccount objects are ordered , with so that the first object's lock being is acquired before the second object's lock. Consequently if When two threads attempt transfers between the same two accounts, they will both each try to acquire the first account's lock first, with the result that one thread will acquire before acquiring the second account's lock. Consequently, one thread acquires both locks, complete completes the transfer, and release releases both locks before the other may thread can proceed.

Unlike the previous compliant solution, this solution incurs no performance penalty, as multiple transfers can occur concurrently permits multiple concurrent transfers as long as the transfers involve distinct target accounts.

Compliant Solution (ReentrantLock)

In this compliant solution, each BankAccount has a java.util.concurrent.locks.ReentrantLock associated with it. This design permits the depositAllAmountdepositAmount() method to try acquiring attempt to acquire the locks of both accounts' locks, but releasing its , to release the locks if it fails, and trying to try again later if necessary.

final class BankAccount {
  private intdouble balanceAmount;  // Total amount in bank account
  private final Lock lock = new ReentrantLock();
  private static final intRandom TIMEnumber = 1000; // 1 second
	  new Random(123L);

  private BankAccountBankAccount(intdouble balance) {
    this.balanceAmount = balance;
  }

  // Deposits the amount from this object instance
  // to BankAccount instance argument ba 
  private void depositAllAmountdepositAmount(BankAccount ba, double amount)
                             throws InterruptedException {
    while (true) {
      if (this.lock.tryLock()) {
        try {
          if (ba.lock.tryLock()) {
            try {
              ba.balanceAmount += this.balanceAmount; if (amount > balanceAmount) {
              this.balanceAmount = 0;throw // withdraw all amount from this instancenew IllegalArgumentException(
                    "Transfer cannot be completed");
              }
              ba.displayAllAmount();  // Display the new balanceAmount in ba balanceAmount += amount;
              this.balanceAmount -= amount;
              break;
            } finally {
              ba.lock.unlock();
            }
          }
        } finally {
          this.lock.unlock();
        }
      }
      Thread.sleep(TIMEint n = number.nextInt(1000);
    }
  }
int TIME 
= 1000 private+ void displayAllAmount() throws InterruptedException {
    while (true) {n; // 1 second + random delay to prevent livelock
      if (lock.tryLock()) {Thread.sleep(TIME);
    }
  }

  trypublic {
static void initiateTransfer(final BankAccount first,
    final  System.out.println(balanceAmount);
          break;BankAccount second, final double amount) {

    Thread transfer = new } finallyThread(new Runnable() {
        public void lock.unlockrun(); {
          }try {
      }
      Threadfirst.sleep(TIMEdepositAmount(second, amount);
    }
   }

  public static} voidcatch initiateTransfer(finalInterruptedException BankAccount first, final BankAccount second) {
e) {
        Thread t = new Thread(new Runnable.currentThread().interrupt() {
; // Reset interrupted status
       public void run() {}
        try {}
    });
      firsttransfer.depositAllAmountstart(second);
        } catch (InterruptedException e) {
          // Forward to handler
        }
      }
    });
    t.start();
  }
}
}
}

Deadlock is impossible in this compliant solution because locks are never held 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 lockDeadlock is impossible in this code, because no method grabs a lock and holds it indefinitely. If a lock is acquired, but the system cannot proceed immediately, it releases the lock and sleeps before requesting the lock again.

Code that uses this lock behaves locking strategy has behavior similar to that of synchronized code that uses the traditional monitor lock. ReentrantLock also provides several other capabilities. For example, for instance, the tryLock() method does not block waiting if immediately returns false when another thread is already holding holds the lock. The class Further, the java.util.concurrent.locks.ReentrantReadWriteLock can be used when some thread requires class has multiple-readers/single-writer semantics and is useful 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:

// Immutable WebRequest
public final class WebRequest

This noncompliant code example consists of an application to monitor a sports race. Each racer is asociated with a dedicated object instance of class Racer.

class Racer implements Cloneable {
  private doublefinal long currentSpeedbandwidth;
  private doublefinal long distanceresponseTime; 

  public double getCurrentSpeedWebRequest()long {
bandwidth, long   return currentSpeed;responseTime) {
  }

  publicthis.bandwidth void setCurrentSpeed(double currentSpeed) {= bandwidth;
    this.currentSpeedresponseTime = currentSpeedresponseTime;
  }

  public doublelong getDistancegetBandwidth() {
    return distancebandwidth;
  }

  public voidlong setDistancegetResponseTime(double distance) {
    this.distance = distancereturn responseTime;
  }

  public Racer clone() {
    try}

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 serve incoming requests.

public final class WebRequestAnalyzer {
  private final Vector<WebRequest> requests return= (Racer) super.clonenew Vector<WebRequest>();

  public boolean } catch (CloneNotSupportedException xaddWebRequest(WebRequest request) {
    return  /* handle error */
requests.add(new WebRequest(request.getBandwidth(),
                   }
    return null request.getResponseTime()));
  }
}

Each racer has two statistics that can be reported about them: their current speed, and the current distance traveled. The class Racer provides methods getCurrentSpeed() and getDistance() for this purpose.

The monitoring application is built upon class Race which maintains a list of racers. To be thread-safe, it accepts a list of racers, and defensively clones them. Henceforth, any thread can change a Racer's distance or current speed, or get the average distance or average current speed of all racers. Changing a racer's statistics involves locking using the racer's intrinsic lock, while getting the average statistics for all racers involves locking all the racers' intrinsic locks until the average is calculated.

public class Race {
  private final Racer[] racers;
 
  public Race(Racer[] racers) {
    this.racers = cloneRacers( 0, racers);
  }

  // Defensively clone the racers array, but only after all racers are locked
  private Racer[] cloneRacers(int i, Racer[] racers) {
    if (i > racers.length - 1) {
      Racer[] result = new Racer[i];
      for (int j = 0; j < this.racers.length; j++) {
        result[j] = racers[j].clone();
      }
      return result;
    }

    synchronized(racers[i]) {
      return cloneRacers( ++i, racers);
    }
  }


  public void setCurrentSpeed(int index, double currentSpeed) {
    synchronized( racers[index]) {
      racers[index].setCurrentSpeed( currentSpeed);
    }
  }

  public void setDistance(int index, double distance) {
    synchronized( racers[index]) {
      racers[index].setDistance( distance);
    }
  }


  double getAverageCurrentSpeed() {
    return averageCurrentSpeedCalculator(0, 0.0);
  }

  double averageCurrentSpeedCalculator(int i, double currentSpeed) {
    // Acquires locks in increasing order
    if (i > racers.length - 1) {
      return currentSpeed / racers.length;
    }

    synchronized(racers[i]) {
      currentSpeed += racers[i].getCurrentSpeed();
      return averageCurrentSpeedCalculator(++i, currentSpeed);
    }	 
  }


  double getAverageDistance() {
    return averageDistanceCalculator(racers.length - 1, 0.0);
  }

  double averageDistanceCalculator(int i, double distance) {
    // Acquires locks in decreasing order
    if (i <= -1) {		 
      return distance / racers.length;
    } 

    synchronized(racers[i]) {
      distance += racers[i].getDistance();
      return averageDistanceCalculator(--i, distance);
    }
  }
}

Consequently, the statistics reported by the methods are accurate at the time the methods actually return their results.

This implementation is prone to deadlock because the recursive calls occur within the synchronized regions of these methods and acquire locks in opposite numerical orders. That is, averageCurrentSpeedCalculator() requests locks from index 0 to MAX - 1 (19) whereas averageDistanceCalculator() requests them from index MAX - 1 (19) to 0. Because of recursion, no previously acquired locks are released by either method. A deadlock occurs when two threads call these methods out of order in that, one thread calls averageSpeedCalculator() while the other calls averageTimeCalculator() before either method has finished executing.

For example, if one thread calls getCurrentSpeed(), it acquires the intrinsic lock for Racer 0, the first in the array. Meanwhile if a second thread calls getCurrentDistance(), it acquires the intrinsic lock for Racer 19, the last in the array. Consequently, deadlock results, as neither thread can acquire all of the locks and proceed with the calculation.

Compliant Solution

In this compliant solution, the two calculation methods acquire and release locks in the same order.

public class Race {
  double averageCurrentSpeedCalculator(int i, double currentSpeed) {
    // Acquires locks in increasing order
    if (i > racers.length - 1) {
      return currentSpeed / racers.length;
    }

    synchronized(racers[i]) {
      currentSpeed += racers[i].getCurrentSpeed();
      return averageCurrentSpeedCalculator(++i, currentSpeed);
    }	
  }


  double averageDistanceCalculator(int i, double distance) {
    // Acquires locks in increasing order
    if (i > racers.length - 1) {
      return distance / racers.length;
    } 

    synchronized(racers[i]) {
      distance += racers[i].getDistance();
      return averageDistanceCalculator(++i, distance);
    }
  }
}

Consequently, while one thread is calculating the average speed or distance, another thread cannot interfere or induce a deadlock. This is because the other thread would first have to synchronize on racers0, which is impossible until the first calculation is complete.

Noncompliant Code Example

// Immutable Racer
public final class Racer {
  private final double currentSpeed;
  private final double distanceTraveled; 

  public Racer(double speed, double distance) {
    currentSpeed = speed;
    distanceTraveled = distance;
  }
  
  public double getCurrentSpeed() {
    return currentSpeed;
  }
  
  public double getDistance() {
    return distanceTraveled;
  }
}

public final class Race {
  private final Vector<Racer> racers;
  private final Object lock = new Object();
  
  public Race(Vector<Racer> racer) {
    racers = (Vector<Racer>) racer.clone(); 
  }
  
  public boolean addRacer(Racer racer){	  
    synchronized(racers.elementAt(racers.size() - 1)) {
      return racers.add(racer);
    }
  }
  
  public boolean removeRacer(Racer racer) {
    synchronized(racers.elementAt(racers.indexOf(racer))) { 
      return racers.remove(racer);      
    }
  }
  
  private double getAverageCurrentSpeed(int i, double currentSpeed) { 
    if (i > racers.size()) {
      return currentSpeed / racers.size();
    }
    synchronized(racers.elementAt(i)) {
      currentSpeed += racers.get(i).getCurrentSpeed();
      return getAverageCurrentSpeed(++i, currentSpeed);  
    }
  }

  private double getAverageCurrentDistance(int i, double distance) { 
    if (i <= -1) {		 
      return distance / racers.size();
    }     
    synchronized(racers.elementAt(i)) {
      distance += racers.get(i).getDistance();
      return getAverageCurrentDistance(--i, distance);
    }
  }
  
  public void getStatisticsAtSomePointInRace() {
    synchronized(lock) {
      getAverageCurrentSpeed(0, 0.0);
      getAverageCurrentDistance(racers.size()-1, 0.0);
    }
  }
}

Compliant Solution

public final class Race {
  private final Vector<Racer> racers;
  private final Object lock = new Object();
  
  public Race(Vector<Racer> racer) {
    racers = (Vector<Racer>) racer.clone(); 
  }
  
  public boolean addRacer(Racer racer){	  
    return racers.add(racer);
  }
  
  public boolean removeRacer(Racer racer) {
    synchronized(racers.elementAt(racers.indexOf(racer))) { 
  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 racers.remove(racer);      calculateAverageBandwidth(++i, bandwidth);
    }
  }
  
  private double getAverageCurrentSpeedcalculateAverageResponseTime(int i, doublelong currentSpeedresponseTime) { 
    if (i > racers.size())<= -1) {
      return currentSpeedresponseTime / racersrequests.size();
    }
    synchronized (racersrequests.elementAt(i)) {
      currentSpeedresponseTime += racersrequests.get(i).getCurrentSpeedgetResponseTime();
      return getAverageCurrentSpeed(++i, currentSpeed);  
    }
  }

  private double getAverageCurrentDistance(int i, double distance) { // Acquires locks in decreasing order
    if  return calculateAverageResponseTime(--i > racers.size()) {		 , responseTime);
      return distance / racers.size();
    }     
    synchronized(racers.elementAt(i)) {
      distance += racers.get(i).getDistance();
      return getAverageCurrentDistance(++i, distance);
    }
  }
  
  public void getStatisticsAtSomePointInRace() {
    synchronized(lock) {
      getAverageCurrentSpeed(0, 0.0);
      getAverageCurrentDistance(0, 0.0);
    }
  }
}

Noncompliant Code Example

}
  }
}

The monitoring application is built around the WebRequestAnalyzer class, which 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, previously acquired locks are never 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, when 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 required to proceed with its 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.

public final class WebRequestAnalyzer

public class Book {
  private String title;
  private double width;
  private double weight;

  public String getTitle() {
    return title;
  }
  public double getWidth() {
    return width;
  }
  public double getWeight() {
    return weight;
  }
 
  public Book(String title, double width, double weight) {
    this.title = title;
    this.width = width;
    this.weight = weight;
  }
}

public class Bookshelf {
  private final Vector<Book>Vector<WebRequest> booksrequests = new Vector<Book>Vector<WebRequest>();
  
  public boolean addBookaddWebRequest(BookWebRequest bookrequest) {
    return requests.add(new WebRequest(request.getBandwidth(),
         // lock on last element to prevent race cond with calculation methods
    synchronized(booksrequest.lastElementgetResponseTime()) {);
  }

  public double getAverageBandwidth() {
   return books.add(newif Book( bookrequests.getTitlesize(), book.getWidth(), book.getWeight())); == 0) {
    }
  }
throw new 
IllegalStateException("The vector // only one remove can happen at a timeis empty!");
    }
  public final synchronizedreturn boolean removeBook(String title) {calculateAverageBandwidth(0, 0);
  }

  public double for getAverageResponseTime(int) i{
 = 0; i <if books(requests.size(); i++ == 0) {
      throw ifnew (books.getTitle().equals( title)) {IllegalStateException("The vector is empty!");
    }
    // lock on book to prevent race cond with calculation routines
  return calculateAverageResponseTime(0, 0);
  }

  private double calculateAverageBandwidth(int i, long bandwidth) {
    if (i synchronized== (booksrequests.elementAtsize(i)) {
      return bandwidth /  return books.remove( irequests.size();
        }
      }
    }synchronized (requests.elementAt(i)) {
    return false; // bookAcquires locks notin onincreasing bookshelforder
  }

  
  privatebandwidth double getTotalWidth(int i, double width) { 
+= requests.get(i).getBandwidth();
      ifreturn calculateAverageBandwidth(++i > books.size()) {, bandwidth);
    }
  }

  private double return width;
    }calculateAverageResponseTime(int i, long responseTime) {
    synchronizedif (books.elementAt(ii == requests.size()) {
      numberOfPages += books.get(i).getWidthreturn responseTime / requests.size();
    }
    return getTotalWidth(++i, width);  synchronized (requests.elementAt(i)) {
    }
  }

// Acquires privatelocks doublein getTotalWeight(int i, double weight) { 
increasing order
     if (iresponseTime <+= -1) {		 requests.get(i).getResponseTime();
      return weight calculateAverageResponseTime(++i, responseTime);
    }
     
    synchronized (books.elementAt(i)) {
      weight += books.get(i).getWeight();
      return getTotalWeight(--i, weight);
    }
  }
}

Risk Assessment

Acquiring and releasing locks in the wrong order may result in deadlocks.

Automated Detection

TODO

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

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"

}
}

Consequently, while one thread is calculating the average bandwidth or response time, another thread cannot interfere or induce deadlock. Each thread must first synchronize on the first web request, which cannot happen until any prior calculation completes.

Locking on the last element of the vector in addWebRequest() is unnecessary for two reasons. First, the locks are acquired in increasing order in all the methods. Second, 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.

Automated Detection

Some static analysis tools can detect violations of this rule.

Related Guidelines

Bibliography

...

Image Added Image Added Image AddedCON11-J. Do not assume that declaring an object volatile guarantees visibility of its members      11. Concurrency (CON)      CON13-J. Do not try to force thread shutdown