Java synchronization not working as (I) expected

Question

programmer mates. I was testing java threading capabilities with a very simple code (or at least it seemed simple). I have this class Account:

public class Account {
    protected double balance;

    public synchronized void withdraw(double value) {
        this.balance = this.balance - value;
    }

    public synchronized void deposit(double value) {
        this.balance = this.balance + value;
    }

    public synchronized double getBalance() {
        return this.balance;
    }
}

And I have two threads: Depositer, that deposits $10 a thousand times:

public class Depositer extends Thread {
    protected Account account;

    public Depositer(Account a) {
        account = a;
    }

    @Override
    public void run() {
        for(int i = 0; i < 1000; i++) {
            this.account.deposit(10);
        }
    }
}

And Withdrawer, that withdraws $10 a thousand times:

public class Withdrawer extends Thread {
    protected Account account;

    public Withdrawer(Account a) {
        account = a;
    }

    @Override
    public void run() {
        for(int i = 0; i < 1000; i++) {
            this.account.withdraw(10);
        }
    }
}

This arrangement is executed by:

public class Main {
    public static void main(String[] args) {
        Account account = new Account();
        Thread c1 = new Depositer(account);
        Thread c2 = new Withdrawer(account);

        c2.start();
        c1.start();

        System.out.println(account.getBalance());
    }
}

As the methods are sychronized, I just expected that the balance was always 0 at the end, but this not happens sometimes. And I sincerely cannot figure out why. Can someone see where is my fault?

Answer 1

The c2.start() and c1.start() runs the process asynchronously. Your main thread needs to wait for those two threads to finish before printing out the results.

Call

try {
    c2.join();
    c1.join();
} catch (InterruptedException e) {
    e.printStackTrace();
}

before calling println.

See join.

Waits for this thread to die.

Answer 2

You do the following:

c2.start();  // Start a thread in the background
c1.start();  // Start a 2nd thread in the background

// print out the balance while both threads are still running
System.out.println(account.getBalance());

You need to wait for these threads to complete their processing:

c2.start();  // Start a thread in the background
c1.start();  // Start a 2nd thread in the background

try {
    c2.join();  // Wait until the c2 thread completes
    c1.join();  // Wait until the c1 thread completes
} catch (InterruptedException e) {
    // LOG AN ERROR HERE
}

// print out the final balance
System.out.println(account.getBalance());

If you interrupt your main Thread then you'll need to do something with an interrupted exception. Assuming that none of your code does this, you should always, at a minimum, log the Exception. NOTE: You'll get the InterruptedException not if someone interrupts c1 or c2, but if someone interrupts your main thread, the thread that calls join(). If someone has called interrupt() on your main thread but you don't check for it, then you'll probably get the InterruptedException the moment you call join().