Problem
I created a simple Bank application to make multiple transactions between multiple accounts. It is working as expected. But I want to know can I make the code more optimized.
public class BankingApplication {
public static void main(String[] args) throws Exception{
ExecutorService service = Executors.newFixedThreadPool(100);
Bank bankObj = new Bank();
for(int i =0; i <= 10000; i++){
service.submit(bankObj);
}
service.shutdown();
service.awaitTermination(1, TimeUnit.DAYS);
bankObj.getTotalBalanceWithStats();
}
}
class Account{
String accountName;
int balance;
Object lock1 = new Object();
Object lock2 = new Object();
Object lock3 = new Object();
Account(String name, int balance){
this.accountName = name;
this.balance = balance;
}
void deposit(int amount){
synchronized (lock1) {
this.balance = getBalance() + amount;
}
}
void withdraw(int amount) throws Exception {
synchronized (lock2) {
int balance = getBalance() - amount;
if(balance <= 0){
throw new Exception("Invalid Transaction");
}
this.balance = getBalance() - amount;
}
}
int getBalance(){
synchronized (lock3){
return this.balance;
}
}
}
class Bank implements Runnable{
List<Account> accountList = new ArrayList<>();
Bank(){
for(int i=0; i <= 30; i++){
accountList.add(new Account("A"+i, 1000));
}
getTotalBalanceWithStats();
}
public void run() {
Random random = new Random();
Account from = null;
Account to = null;
while(true) {
int a1 = random.nextInt(30);
int a2 = random.nextInt(30);
if(a1 == a2) continue;
from = accountList.get(a1);
to = accountList.get(a2);
break;
}
doTransaction(from, to, random.nextInt(400));
}
private void doTransaction(Account from, Account to, int nextInt) {
System.out.println("Transaction from " + from.accountName + " to " + to.accountName);
try {
from.withdraw(nextInt);
to.deposit(nextInt);
} catch (Exception e){
System.out.println(e.getMessage());
}
}
void getTotalBalanceWithStats(){
int total = 0;
for(Account acc : accountList){
System.out.println("Account " + acc.accountName + " balance " + acc.getBalance());
total += acc.getBalance();
}
System.out.println("Total Balance " + total);
}
}
As per my knowledge I feel deposit(), withdrawal() and getBalanace() API are independent of each other so creating different mutex for each process.
Some scenarios which I am trying to test
What I am trying to achieve is when Account1 is trying to send money to Account2 at the same time Account3 can also send money to Account2 or vice versa.
When Account1 is sending money to Account2 at same time Account2 can also send money to Account1.
When Account1 is sending money to Account2 and at same time Account3 is sending money to Account1. withdraw() method of Account1 should not block deposit().
Solution
This code is broken.
Imagine an Account with $100, two threads, one depositing $1, the other withdrawing $50.
Thread one acquired lock1, and begins executing:
this.balance = getBalance() + amount;
and reads $100 …
Thread two acquired lock2, reads the balance (still $100), subtracts $50, stores the new balance ($50) and releases lock2.
… (back to thread one) … adds $1, for a total of $101, which it stores in balance.
Free money! The withdrawal of $50 was forgotten.
You must lock all read-modify-write operations of an object’s member with the same lock.