Two threads use same method despite using lock() in Java code?

Question

I have a simple code in Java. The purpose is to test entranceLock() of Java. There's a class named Bank (code below), which contains an array of Account objects (code for Account class is below).

Bank class can transfer money from one Account to another Account, using methods of Account objects named withdraw end deposit. (One of the data members is an array of Accounts named accounts).

AccountThread extends Thread which will run each Account as a Thread. It contains a reference to "Bank" object.

TestBank has the (public static void main(String args[]) method and will run the objects.

It will initialize a Bank object, make 10 AccountThread objects, pass reference to that Bank object to these AccountThread objects. Then it will start running all these 10 threads (running the AccountThread objects).

These 10 accounts transfer money only to each other. Each thread will run its own account.

These threads will in an infinite loop, randomly pick an account (one of the other 9 accounts) and transfer a random amount to that account.

After each 10 000 transaction the program will print some data, one of them is the "sum balance" (sum of the balance of all these 10 accounts).

That sum should never change, since the accounts only transfer money to each other.

I used lock.lock() and lock.unlock() in "deposit" and "withdraw" methods so that while there's a deposit or withdrawing in one account, other threads should not access those methods.

But there are some problems:

But the problem is, I do get some deviations from the sum, I don't understand what causes that.

I tried using "AccountThread" as class implements Runnable, but the results are the same.

There's what I tried:

1) And I suspected the devitaion maybe is caused by that threads are still running when printing the data (which may cause inconsistent data). So I changed something on the transfer method in Bank class, made a code to stop all threads from running before test() (and unlock all after running test()).

for (Account account: accounts) {account.getLock().lock();} before calling test() and toString()

and after test() and toString() I did call

for (Account account: accounts) {account.getLock().unlock();} 

But the results are the same.

2) I only locked the two accounts that just used in transfer before runing test(), so that other threads don't use these to accounts. But it doesn't work either.

I have tried other combinations too, but the results are not the desired results.

Ohter two problems are:

• Another problem is, while the toString() and test() methods are called after each other, when running the program these methods get called in random order**

• A thirdy problem is the test() results show not after each 10 000 transactions but often more than 10 000 transactions

Can multiple threads do access same deposit() or withdraw() method from same class at the same time despite the the methods use "lock.lock() and "lock.unlock()" methods?

Why is there some devitaion here ? How can I make sure the total-balance is always the same ?

Here's Account class:

import java.util.concurrent.locks.*;

class Account {
  private int balance;
  private int accountNumber;
  private Lock lock;
  private Condition lockCondition;

  public Account(int accountNumber, int balance) {
    this.accountNumber = accountNumber;
    this.balance = balance;
    this.lock = new ReentrantLock();
    this.lockCondition = lock.newCondition();
  }


/* HERE IS THE WITHDRAW AND DEPOSIT METHODS THAT ARE LOCKED */
  void withdraw(int amount) { 
      lock.lock(); // Acquire the lock
      try {
        while (balance < amount) {
          lockCondition.await();
        }
        balance -= amount;
      }
      catch (InterruptedException ex) {
        ex.printStackTrace();
      }
      finally {
        lock.unlock(); // Release the lock
      }
  }

  void deposit(int amount) {
      lock.lock(); // Acquire the lock
      try {
        balance += amount;
        // Signal thread waiting on the condition
        lockCondition.signalAll();
      }
      finally {
        lock.unlock(); // Release the lock
      }
  }

  int getAccountNumber() {
    return accountNumber;
  }

  public int getBalance() {
    return balance;
  }

  Lock getLock() {
      return lock;
  }
}

Here's AccountThread class:

import java.util.Random;

class AccountThread extends Thread {
  private Bank bank;
  private boolean debug;
  private int accountIndex;
  private int maxTransferAmount;
  private Random random;

  public AccountThread(Bank b, int index, int max, boolean debug) {
    this.bank = b;
    this.accountIndex = index;
    this.maxTransferAmount = max;
    this.debug = debug;
    this.random = new Random();
  }

  public void run() {
    try {
      while (!interrupted()) {
        for (int i = 0; i < maxTransferAmount; i++) {
            int toAccount = random.nextInt(bank.nrOfAccounts());
            int amount = random.nextInt(maxTransferAmount);
            bank.transfer(accountIndex, toAccount, amount);
            sleep(2);
        }
      }
    } catch (InterruptedException ignored) {
    }
  }
}

Here's Bank class:

import java.util.concurrent.locks.*;

class Bank {
  private static final int TEST_FREQUENCY = 10000;
  private static final int TO_STRING_FREQUENCY = 10000;
  private Lock lock;
  private int deviationCount;
  private int initialBalance;
  private Account[] accounts;
  private long transactionCount;
  private boolean debug;
  private int testCount;


  public Bank(int accountAmount, int initialBalance, boolean debug) {
    accounts = new Account[accountAmount];
    this.initialBalance = initialBalance;
    this.debug = debug;
    int i;
    for (i = 0; i < accounts.length; i++)
      accounts[i] = new Account(i, initialBalance);
    this.transactionCount = 0;
    this.deviationCount = 0;
    this.lock = new ReentrantLock();
  }

  public void transfer(int fromAccount, int toAccount, int amount) {
      accounts[fromAccount].withdraw(amount);
      accounts[toAccount].deposit(amount);
      this.transactionCount++;
//    accounts[fromAccount].getLock().lock();
//    accounts[toAccount].getLock().lock();
    //  for (Account account: accounts) {account.getLock().lock();} 
      lock.lock();
      try {
          if (transactionCount % TEST_FREQUENCY == 0) {
              test();
          }
          if (transactionCount % TO_STRING_FREQUENCY == 0) {
              System.out.println(toString());
          }

    //    accounts[fromAccount].getLock().unlock();
//        accounts[toAccount].getLock().unlock();
      } finally {
          lock.unlock();
      }
//    for (Account account: accounts) {account.getLock().unlock();}
  }


  public void test() {
    int sum = 0;
    for (Account account : accounts) {sum += account.getBalance(); }

    if (sum != nrOfAccounts()*initialBalance) {deviationCount++; }

    System.out.println("Transactions:" + getTransactionCount() + " Balance: " + sum
            + " Deviation count: " + getDeviationCount());
    testCount++;
  }

  @Override
  public String toString() {
      String string = String.format("\nTransactions; %d%n"
            + "Initial balance: %d%nNumber of accounts: %d%n"
            + "Deviation count: %d%nTestCount: %d%n"
            + "Error percentage: %.2f%n", 
            getTransactionCount(), initialBalance, nrOfAccounts(), 
            getDeviationCount(), testCount, getErrorPercentage());
      if (debug) {
          for (Account account :accounts) {
              string = string.concat(String.format("Account nr.: %d, Balance: %d%n", 
                      account.getAccountNumber(), account.getBalance()));
          }
      }
      return string;
  }

  int nrOfAccounts() {
    return accounts.length;
  }

  private long getTransactionCount() {
      return transactionCount;
  }

  private int getDeviationCount() {
      return deviationCount;
  }

  private double getErrorPercentage() {
      double dividend = getDeviationCount();
      double divisor = testCount;
      double result = dividend / divisor;
      return result;
  }
}

Here's the BankTest class:

    public class BankTest {
    private static final boolean DEBUG = true;
    private static final int ACCOUNT_AMOUNT = 10;
    private static final int INITIAL_BALANCE = 100000;

    public BankTest() {};

    public static void main(String[] args) {
        Bank b = new Bank(ACCOUNT_AMOUNT, INITIAL_BALANCE, DEBUG);
        int i;
        for (i = 0; i < ACCOUNT_AMOUNT; i++) {
            AccountThread t = new AccountThread(b, i,
                    INITIAL_BALANCE, DEBUG);
            t.setPriority(Thread.NORM_PRIORITY + i % 2);
            t.start();
        }
    }
}

Answer 1

Q: What happens when there's a test() in progress, and some other AccountThread deducts money from one account that the test already has sampled, and transfers it to an account that the test has not sampled yet?

A: The money that was moved will be counted twice.

You have the opposite problem if money is transferred from an account that has not been sampled yet to an account that already has been sampled. That money will not be counted at all.

The locking that you've tried so far ensures that the total in the bank always will be correct at any single instant in time, but your test() can't be completed in a single instant. You need to shut down the bank—prevent any and all transfers—during the counting.

IMO: The best way to do that would be to use a ReadWriteLock. The normal use-case for a ReadWriteLock is a shared data structure that threads frequently want to examine, but seldom want to update. It will allows any number of simultaneous "readers" to access the structure *OR* it will allow one "writer" to access it, but not both.

In your application, the threads that want to transfer funds are the "readers." This may sound strange because those threads want to actually change the bank structure, but the point is, you want to allow any number of them to do their thing at the same time. On the other hand, a thread that wants to call test(), even though it modifies nothing, must play the "writer" role because it must have exclusive access to the bank. No transfers must be allowed to happen while a test() is in progress.