Thread lock ordering versus a simple synchronized block

Question

While going through Java Concurrency in Practice book, came across this piece of code where "fromAccount" and ""toAccount" objects are locked one after the other to prevent dynamic lock order deadlock.

public void transferMoney(Account fromAccount,Account toAccount) {
    **synchronized (fromAccount) {**
        **synchronized (toAccount) {**
               ........
        }
    }
}

I am confused as to why this lock ordering is needed at all.If we just wanted to make sure that both the objects are locked at the same time then wouldn't you get the same effect if there was just a regular synchronization block inside which fromAccount and toAccount objects are accessed. I am sure that I am missing some fundamental concept here. Thank you for your help.

public void transferMoney(Account fromAccount,Account toAccount) {
    synchronized (this) {
        fromAccount.someMethod();
        toAccount.someMethod();        
    }
}

Answer 1

Your alternative to the lock-ordering example is what you want to avoid: having a central lock that everything is using, because then you don't get concurrent transfers, everything waits for that one lock and only one transfer can proceed at a time. It's not clear what this is or what its scope can possibly be, but if there are multiple instances of this transfer service then locking doesn't do any good, because one transfer involving one account can go through one instance while another transfer involving that same account can go through another. Therefore it seems like there can only be one of them, which diminishes your concurrency to one transfer at a time. You won't deadlock, but you won't process a lot of transfers quickly either.

The idea behind this toy example (which you shouldn't mistake for anything like how anybody would transfer money) is it's trying to get better concurrency by locking on the individual accounts involved in the transfer, because for a lot of transfers the accounts involved aren't involved in other concurrent transfers and you'd like to be able to process them concurrently and maximize your concurrency by minimizing the scope of the locking going on to the individual accounts. But this scheme runs into trouble if some account is involved in multiple concurrent transfers and the locks are acquired in a different order for some transfers.

Answer 2

First, it should be noted that the example you have brought (based on your comment, it's page 208, listing 10.2) is a bad example - one that ends in a deadlock. The objects are not locked one after the other to prevent dynamic lock order deadlock, they are an example of where dynamic lock order will happen!

Now, you are suggesting locking on this, but what is this this anyway, and what is the scope of locking?

• It's clear that the same object has to be used for all operations - withdraw, deposit, transfer. If separate objects are used for them, then one thread could do a deposit on account A, while another thread transfers from account A to account B, and they won't be using the same lock so the balance will be compromised. So the lock object for all accesses to the same account should be the same one.
• As Nathan Hughes explained, one needs to localize the locking. We can't use one central lock object for all the accounts, or we'll have them all waiting for each other despite not actually working on the same resources. So using a central locking object is also out of the question.

So it appears that we need to localize the locks so that each account's balance will have its own lock, so as to allow parallel operations between unrelated accounts, but that this lock has to be used for all operations - withdraw, deposit and transfer.

And here comes the problem - when it's just withdraw or deposit, you are operating on just one account, and so you need to just lock that account. But when you transfer, you have two objects involved. So you need to have both their balances locked in case there are other threads that want to operate on either.

Any object that holds a single lock for two or more accounts will break one of the two points above. Either it won't be used for all operations, or it will not be localized enough.

This is why they are attempting to lock the two locks one after another. Their solution was to make the Account object itself the lock for the account - which fulfils both the "all operations" condition and the "locality" condition. But still we need to make sure we have the locks for both accounts before we can transfer the money.

---

But again, this source is an example of a deadlock prone code. This is because one thread may want to transfer from account A to account B, while another will want to transfer from account B to account A. In that case, the first one locks the A account, the second locks the B account, and then they are deadlocked because they have performed the locking in opposite order.

Answer 3

The basic fundamental here is to avoid race condition. In your case if there will be another method in any other class who is also doing transfer money to toAccount then incorrect amount may get update in the toAccount. e.g. There are 2 classes which performs money transfer.

One class has a method:

public void transferMoney(Account fromAccount,Account toAccount) {
    synchronized (this) {
        fromAccount.someMethod();
        toAccount.someMethod();        
    }
}

and other class contains:

public void transferMoneyNow(Account fromAccount1,Account toAccount) {
    synchronized (this) {
        fromAccount1.someMethod();
        toAccount.someMethod();        
    }
}

If both method takes place at the same time, due to race condition incorrect amount may get update in toAccount.