Search code examples
javaconcurrencysynchronized

Why is the synchronized method not getting locked unless I use .join()?


In Java, marking a method synchronized should disable the race condition that would result from two threads calling the method which is accessing and modifying a field in the same object.

But for some reason, synchronized is not working as expected in the following example unless I call .join() on both threads in the main program. Why is that?

package example;

public class Account {
    private double balance;

    public Account(double balance) {
        super();
        this.balance = balance;
    }

    public synchronized void deposit(double amount) {
        balance += amount;
    }

    public double getBalance() {
        return balance;
    }
}
package example;

public class AccountTester extends Thread {
    private Account account;
    private double amount;

    public AccountTester(Account account, double amount) {
        this.account = account;
        this.amount = amount;
    }

    public static void main(String[] args) {
        Account account = new Account(0);
        AccountTester tester1 = new AccountTester(account, 1.0);
        AccountTester tester2 = new AccountTester(account, 2.0);
        tester1.start();
        tester2.start();
        // Why do I need the main thread to join threads
        //      tester1 and tester2 for synchronized to work?
        try {
            tester1.join();
            tester2.join();
        } catch (InterruptedException e) {
            System.err.println(e);
        }
        System.out.println("End balance: " + account.getBalance());
    }

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

Solution

  • As clarified by @flakes, the code needed the main thread to join() both threads to guarantee that both threads are terminated, i.e. done with their modification to the balance, before it printed out the end balance.

    A cleaner way to implement this would be by using the java.util.concurrent.ExecutorService interface. Here its shutdown() method guarantees that both threads in the thread pool are completed before printing the end balance. Here's my implementation:

    package example;
    
    import java.util.concurrent.ExecutorService;
    import java.util.concurrent.Executors;
    
    public class AccountTester implements Runnable {
        private Account account;
        private double amount;
    
        public AccountTester(Account account, double amount) {
            this.account = account;
            this.amount = amount;
        }
    
        public static void main(String[] args) {
            Account account = new Account(0);
            ExecutorService executorService = Executors.newFixedThreadPool(2);
            executorService.execute(new AccountTester(account, 1.0));
            executorService.execute(new AccountTester(account, 2.0));
            executorService.shutdown();
            System.out.println("End balance: " + account.getBalance());
        }
    
        @Override
        public void run() {
            for (int i = 0; i < 1000; i++) {
                account.deposit(amount);
            }
        }
    }