Implementing bank account in Java

I am new开发者_StackOverflow社区 to thread programming in Java. To understand threading I'm trying to write a simple program to simulate a bank account. I just implemented withdrawal and trying to test it. First few lines of the output is below.

Balance before T2 withdrawl: 1000

Balance after T2 withdrawl: 990

Balance before T1 withdrawl: 1000

Balance after T1 withdrawl: 980

Balance before T2 withdrawl: 980

Balance after T2 withdrawl: 970

Balance before T1 withdrawl: 970

Balance after T1 withdrawl: 960

My question is why the line 3 (Balance before T1 withdrawl: 1000) in the output gives 1000 instead of 990. If it was correct it should be on line 2. Am I missing some thing. Is my approach is correct?

My guess is that both threads trying to write to write to the console and thread T1 simply did not get a chance to write it on the second line.

class BankAccount {

    private volatile int balance;

    public BankAccount(int b){
        balance = b;
    }

    public BankAccount(){
        balance = 0;
    }


    synchronized public int getBalance(){
        return balance;
    }

    synchronized public int withdraw(int w)
    {
        int b = getBalance();
        if(w <= b){
            balance = balance-w;
            return w;
        }
        else
            return 0;
    }
}

class WithdrawAccount implements Runnable{

    private BankAccount acc;
    private int amount;

    public WithdrawAccount(){
        acc = null;
        amount = 0;
    }

    public WithdrawAccount(BankAccount acc,int amount){
        this.acc = acc;
        this.amount = amount;
    }

    public void run() {
        int w; 

        for(int i =0; i<20; i++){
            try {
                Thread.sleep(200);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }

            System.out.println("Balance before "+Thread.currentThread().getName()+" withdrawl: "+acc.getBalance());
            w = acc.withdraw(amount);
            System.out.println("Balance after "+Thread.currentThread().getName()+" withdrawl: "+acc.getBalance());
            //System.out.println("amount with drawn by: "+Thread.currentThread().getName()+" "+w);

        }

    }

}

public class TestBankAccount{

    public static void main(String[] args) {

        BankAccount b = new BankAccount(1000);
        WithdrawAccount w = new WithdrawAccount(b,10);
        Thread wt1 = new Thread(w);
        wt1.setName("T1");

        Thread wt2 = new Thread(w);
        wt2.setName("T2");

        wt1.start();
        wt2.start();
    }
}

You've done nothing to synchronize your run method, so the printlns before and after the withdrawal and the withdrawal itself are not atomic. You're getting thread interleaving.

Probably one thread calls withdraw() between those lines:

System.out.println("Balance before "+Thread.currentThread().getName()+" withdrawl: "+acc.getBalance());
w = acc.withdraw(amount);

Possible scenario:

1. T2 calls getBalance();
2. T1 calls getBalance();
3. T2 outs balance;
4. T2 calls withdraw();
5. T2 outs balance after withdraw;
6. T1 outs balance;
7. T1 calls withdraw();
8. T1 outs balance after withdraw;

that's one of the possible interleaving of threads:

    wt2: calls getBalance()   // retrieves 1000
    wt2: prints "Balance before T2 withdrawl: 1000"

    wt1: calls getBalance()   // retrieves 1000 also

    wt2: acc.withdraw(amount) // balance is now 990
    wt2: prints "Balance after T2 withdrawl: 990"

    wt1: acc.withdraw(amount)    // balance was 990 after wt1 withdraws. wt1 now withdraws again so balance is 980 
    wt1: prints "Balance after T2 withdrawl: 980"

It's a concurrency problem

1. T1 retrieves 1000
2. T1 prints "before" to system out 1000
3. T2 retrieves 1000
4. T1 makes the withdraw
5. T1 prints "after" to system out 990
6. T2 prints "before" to system out 1000
7. T2 makes the withdraw
8. T2 prints "after" to system out 980.
9. ...

Could not be exactly executed in that order, but you get the idea. To make it work like you want use synchronized block.

  synchronized (acc) {
    System.out.println("Balance before " + Thread.currentThread().getName() + " withdrawl: " + acc.getBalance());
    w = acc.withdraw(amount);
    System.out.println("Balance after " + Thread.currentThread().getName() + " withdrawl: " + acc.getBalance());
  }