While I was reading some concurrency code samples on the internet I found this one (money transfer operation between 2 bank accounts):
class Account {
double balance;
int id;
public Account(int id, double balance){
this.balance = balance;
this.id = id;
}
void withdraw(double amount){
balance -= amount;
}
void deposit(double amount){
balance += amount;
}
}
class Main{
public static void main(String [] args){
final Account a = new Account(1,1000);
final Account b = new Account(2,300);
Thread a = new Thread(){
public void run(){
transfer(a,b,200);
}
};
Thread b = new Thread(){
public void run(){
transfer(b,a,300);
}
};
a.start();
b.start();
}
And this piece of code that deals with the concurrency issue with the use of ReentrantLock:
private final Lock lock = new ReentrantLock(); //Addition to the Account class
public static void transfer(Account from, Account to, double amount)
{
while(true)
{
if(from.lock.tryLock()){
try {
if (to.lock.tryLock()){
try{
from.withdraw(amount);
to.deposit(amount);
break;
}
finally {
to.lock.unlock();
}
}
}
finally {
from.lock.unlock();
}
Thread.sleep(someRandomTimeToPreventLiveLock);
}
}
My question is: shouldn't the Acount's withdraw() and deposit() methods be somehow protected (synchronized or locked with the ReentrantLock field) for this example to work? Isn't it possible for other thread to creep in and invoke withdraw or deposit method? Also, what if there's a getBalance() method? Should it be protected too (synchronized or locked with the ReentrantLock)?
There are two options:
(1) You make your class thread-safe meaning that any operation on any instance of this class is guarded by some internal mechanism and is absolutely safe in a multithreading environment. The caller side shouldn't care about thread-safety.
That's what I would prefer here. As a consumer of your API, I would except both Account#withdraw
and Account#deposit
to be self-sufficient, so no extra actions would be required.
That's how a good API looks to me.
(2) You place the responsibility of providing correctness and thread-safety on the caller side. You don't care how it's going to be achieved.
That's how your snippets currently work. The method transfer
is thread-safe, but it doesn't make the account operations such.