Considering the code below.
public class Account {
private static List<Double> log = new ArrayList<Double>();
private double balance;
public Account(){balance = 0.0;}
public synchronized void deposit(double val){
balance = balance val;
log.add(val);
}
public synchronized void withdraw(double val){
balance = balance - val;
log.add(val);
}
}
Is the synchronization correct?
I would say yes, because the method that access the variable log are atomic (thanks to the word Synchronized). I tested it in this way:
Account a = new Account();
Thread t = new Thread(()->a.deposit(30));
Thread j = new Thread(()->a.withdraw(20));
t.start();
j.start();
sleep(300);
System.out.println(Account.getLog());
It seems to me that blocking is fine, as long as one thread is using the log variable, the other is not using it.
However, the text shows this solution (not explaining why)(-> stands for substitution):
log.add(val)->
synchronized(log)
{
log.add(val)
}
log.add(-val)->
synchronized(log)
{
log.add(-val)
}
class Account {
private static List<Double> log = new ArrayList<Double>();
private double balance;
public Account() {
balance = 0.0;
}
public synchronized void deposit(double val) {
balance = balance val;
synchronized (log) {
log.add(val)
}
}
public synchronized void withdraw(double val) {
balance = balance - val;
synchronized (log) {
log.add(-val);
}
}
}
Why should I lock the variable log? Is it not enough to synchronize the method?
CodePudding user response:
The solution code looks thread-safe to me:
- The
balance
variable for each account is updated while holding the account's mutex. - The
log
is updated while holding the list's mutex.
One small quibble is that log
should be final
. If anything was able to (for example accidentally) assign a new value to log
, different threads are not guaranteed to be seeing / updating the same list object.
However, your version of the code is NOT thread-safe. It is not locking log
while updating it. That means that two threads updating different Account
objects could simultaneously attempt to update the log
list. That could corrupt it.
There are various other things wrong with (both) solutions. The most critical one is that you are using double
to represent currency values. That is a violation of normal accounting practice. (And some bank customers will get antsy if you "steal" some of their pennies ... due to floating point rounding errors.)