Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Edited

To avoid data corruption in multithreaded Java programs, you have one has to protect the data that is touched by shared between multiple threads as described in detail in CON30-J. Synchronize access to shared mutable variables. This can be done at an the object level to protect the data in the synchronized block by using synchronized blocks (coarse-grained locking), thus locking out any other thread threads from accessing it. Locking that is achieved in this way has no danger of deadlocks.interfering. If synchronization is used judiciously, (See CON00-J. Use synchronization judiciously) deadlocks do not usually crop up.

If however, one follows But if you follow a fine-grained locking , approach by using member locks, you may end up in a deadlock unless you ensure that each deadlocks can still arise unless it is ensured that each and every thread always requests locks in the same order.

Compliant Solution

Noncompliant Code Example

In this noncompliant code example, a deadlock occurs if the transfer method acquires the monitors in decreasing numeric order, while the sumHelper method uses an increasing numeric order to acquire its locksTo implement a fine-grained strategy, take out a separate lock for each position in the balances array.

Code Block
bgColor#ccccff#FFcccc
class Stocks implements FundConstants {
    static int[] balances =
                       new int[noOfStocks];
    static Object[] locks =
                    new Object[noOfStocks];
    static
    {
        for (int n=0; n<noOfStocks; n++) {
            balances[n] = 10000;
            locks[n] = new Object();
        }
    }

    static void transfer(Transfer t) {
        int lo, hi;
        if (t.fundFrom <> t.fundTo) {
// acquires the monitors in decreasing numeric order
      lo = t.fundFrom;
            hi = t.fundTo;
        } else {
            lo = t.fundTo;
            hi = t.fundFrom;
     }
   }
   
     synchronized (locks[lo]) {
            synchronized (locks[hi]) {
                balances[t.fundFrom] -= t.amount;
                balances[t.fundTo] += t.amount;
            }
        }
    }

    static int sumHelper (int next) {
        synchronized (locks[next]) {
            if (next == (noOfStocks-1)) {
                return balances[next];
            } else {
                return balances[next] +
                       sumHelper(next+1);
            }
        }
    }

    static void checkSystem() {
        int actual = 0;
        actual = sumHelper(0);
        System.out.println("Actual balance is
                                   " + actual);
    }
}

Compliant Solution

Since you To implement a fine-grained locking strategy, request a separate lock for each position in the balances array. Since one cannot lock on primitive types, you cannot take a direct lock on the items in the balances array cannot be obtained. Instead, you have one has to create an array of Object (locks).

The code above avoids deadlock This compliant solution avoids deadlocks because every thread requests monitors in the same order , since it is always acquiring the locks in numeric order.

Noncompliant Code Example

Deadlock would occur if the transfer method acquired the monitors in decreasing numeric order, while the sumHelper method would still be using increasing numeric orderand thus the locks are acquired (and released) correctly.

Code Block
bgColor#FFcccc#ccccff
class Stocks implements FundConstants {
    static int[] balances =
                       new int[noOfStocks];
    static Object[] locks =
                    new Object[noOfStocks];
  
  static
    {
        for (int n=0; n<noOfStocks; n++) {
            balances[n] = 10000;
            locks[n] = new Object();
        }
    }

    static void transfer(Transfer t) {
        int lo, hi;
        if (t.fundFrom >< t.fundTo) {// acquires the monitors in decreasing numeric order
            lo = t.fundFrom;
            hi = t.fundTo;
        } else {
            lo = t.fundTo;
            hi = t.fundFrom;
        }
        synchronized (locks[lo]) {
            synchronized (locks[hi]) {
                balances[t.fundFrom] -= t.amount;
                balances[t.fundTo] += t.amount;
            }
        }
    }

    static int sumHelper (int next) {
        synchronized (locks[next]) {
            if (next == (noOfStocks-1)) {
                return balances[next];
            } else {
                return balances[next] +
                       sumHelper(next+1);
            }
        }
    }

    static void checkSystem() {
        int actual = 0;
        actual = sumHelper(0);
        System.out.println("Actual balance is
                                   " + actual);
    }
}

Risk Assessment

Fine-grained locking may lead result in deadlock deadlocks if each some thread does not always request locks in the same order as others.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

CON33-J

low

unlikely

high

P1

L3

...

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

References

Wiki Markup
\[[JLS 05|AA. Java References#JLS 05]\] [Chapter 17, Threads and Locks|http://java.sun.com/docs/books/jls/third_edition/html/memory.html]

Wiki Markup
\[[SDN 08|AA. Java References#SDN 08]\] [Sun Developer Network Tech tips|http://java.sun.com/developer/TechTips/2000/tt0328.html]&nbsp;

...