4

I'm learning java concurrency and used a Bank Account that is shared among multiple people example to try to practice principles of concurrency.

This is my Account class.

public class Account(){
    private int balance = 0;

    public int getBalance(){ return balance}              
    public synchronized void deposit(int val){//..}   
    void synchronized void withdrawal(int val){//..}
}

deposit and withdrawal methods are synchronized because they directly modify the state of the Account object, so to avoid data corruption if multiple users try to change it at the same time.

On the other hand, getBalance() does not change the state of the Account object. But I was thinking that if getBalance() is called while a deposit or withdrawal is happening, it will return an outdated value. What is the standard practice, to synchronize or not to synchronize getBalance()?

Pavel Smirnov
  • 4,611
  • 3
  • 18
  • 28
enzio902
  • 457
  • 6
  • 14
  • 1
    Not sure what kind of answer you expect here... Use `synchronized` if you want to make sure you definitely get an up-to-date value, and don't use `synchronized` if you don't care. – Robby Cornelissen Dec 23 '19 at 09:51

3 Answers3

3

if getBalance() is called while a deposit or withdrawal is happening, it will return the outdated value

There's no outdated value while these processes are running, simply because you can not say for sure whether they're going to fail or not (I'm assuming you're not using balance field for intermediate state representation).

So the question actually is - do you want to read the most recent state of balance and do it despite of whether withdrawal or deposit is running? Or, you want to wait until one of the processes finishes, so you can read the value?

In the first case you can simply make the field volatile (private volatile int balance = 0;) to ensure memory visibility between threads.
In the second case, simply use synchronized.

Pavel Smirnov
  • 4,611
  • 3
  • 18
  • 28
2

IMHO, synchronized is needed to avoid 'race condition' when you are trying to modify the property. https://netjs.blogspot.com/2015/06/race-condition-in-java-multi-threading.html

So that if you're modifying it you could predict the result. Otherwise, you cannot guarantee what would be the output.

Besides that I would, probably, wrap the balance property into some AtomicInteger or at least mark it as volatile.

Andriy Zhuk
  • 133
  • 6
1

you need to synchronize an Object not literal, as synchronize only the Object which is critical for better performance than function.

Object lock = new Object();

      public class Account(){
        private int balance = 0;

        public int getBalance(){ synchronized (lock){return balance} }              
        public void deposit(int val){synchronized (lock){...}  }   
        void withdrawal(int val){synchronized (lock){...}  }
      }
    }

This way you can add more logic inside the function to perform better and keep balance synchronized on updates.

Tal.Bary
  • 450
  • 2
  • 16
  • 1
    Don't use an `Integer` instead of an `int` just to synchronize on an object. Either `synchronize(this)` or synchronize on an `Object lock` you created just for that. – Matthieu Dec 23 '19 at 10:14