CODEWITHSUNDEEP
javasynchronizationsynchronized
user3487554
user3487554

Reputation: 25

Issue with synchronizing threads in java

I'm trying to write an example program which consists of a bank account object that the amount of money can be modified in, and 10 slave objects that move 1 unit of currency a million times each from 2 different bank account objects. My intended outcome is for the sum of the bank account currencies to be 0 at the end, like what I start with. I've tried synchronizing the "setBalance" method in BankAccount, as I believe that's the one that should be synchronized, however that doesn't work. When I tried it on getBalance or run even those don't work either, so I'm not sure what i'm doing wrong.

BankAccount.java

public class BankAccount{

    private int balance;

    public BankAccount()
    {
        balance = 0;
    }

    public int getBalance()
    {
        return balance;
    }

    public void setBalance(int balance)
    {
        this.balance = balance;
    }
}

Slave.java

public class Slave extends Thread {

    BankAccount source;
    BankAccount target;

    int currency = 1;

    public Slave (BankAccount source, BankAccount target) {
        this.source = source;
        this.target = target;

    }

    public void run() {
        for (int i = 0; i < 1000000; i++)
        {
            target.setBalance(target.getBalance() + currency);
            source.setBalance(source.getBalance() - currency);
        }
    }
}

Master.java

public class Master {

    public static void main(String[] args) {

        BankAccount account1 = new BankAccount();
        BankAccount account2 = new BankAccount();

        Slave[] slaves = new Slave[10];

        for (int i = 0; i < 10; i++)
        {
            if (i < 5)
            {
                slaves[i] = new Slave(account1, account2);
            }
            else
            {
                slaves[i] = new Slave(account2, account1);
            }
        }

        for (int i = 0; i < 10; i++)
        {
            slaves[i].start();
        }

        try
        {
            for (Slave s : slaves)
                s.join();
        }
        catch (InterruptedException e) {
            System.out.println("Interruption before completion of the joins" + e);
        }


        System.out.println("Sum of balances: " + (account1.getBalance() + account2.getBalance()));
    }
}

Upvotes: 1

Views: 72

Answers (2)

John Bollinger
John Bollinger

Reputation: 180093

This code ...

            target.setBalance(target.getBalance() + currency);

... has its intended effect in a thread T1 only if target's balance is not modified by a different thread between T1's invocation of getBalance() and its invocation of setBalance(). Synchronizing one or both of those methods does nothing to prevent such an intervening modification, and your multiple threads are performing so many of those, so fast, that it's reasonably likely that you'll hit that issue.

The appropriate critical region thus encompasses both method invocations, which you can implement by putting them inside a synchronized block:

            synchronized(target) {
                target.setBalance(target.getBalance() + currency);
            }

Of course, you need to treat the update of source similarly. Note well that for each critical region, all contending threads need to synchronize on the same object for the synchronization to have the desired effect.

Upvotes: 0

Joe C
Joe C

Reputation: 15684

The synchronized keyword, if used in a method signature, takes a lock on the object being invoked. If you wish to lock on an alternative object, you need to use the keyword in this way:

synchronized(objectToLock) {
    doAtomicStuff();
}

In your case, your atomic operation is getting, adjusting and setting the balance, and you will want to lock on the relevant BankAccount objects.

That should be enough for you to work out where and how to use synchronized.

Upvotes: 1

Related Questions