Nested synchronized block

Question

Let's imagine I have next classes:

public class Service {
    public void transferMoney(Account fromAcct, Account toAcct, int amount) {
      synchronized (fromAcct) {
        synchronized (toAccount) { // could we use here only one synchronized block?
            fromAcct.credit(amount);
            toAccount.debit(amount);
        }
      }
    }
}

class Account {
  private int amount = 0;

  public void credit(int sum) {
    amount = amount + sum;
  }

  public void debit(int sum) {
    amount = amount - sum;
  }
}

For example I know that we could change state of fromAcct and toAcct objects only in transferMoney method. So could we rewrite our method with one synchronized block?

public class Service {
 private final Object mux = new Object();

 public void transferMoney(Account fromAcct, Account toAcct, int amount) {
      synchronized(mux) {
        fromAcct.credit(amount);
        toAcct.debit(amount);
      }
 }
}

Answer 1

Unless you have a very unusual and particular need that I can't understand, it seems to me that your goal should be to protect the account balance from being corrupted by multiple threads attempting to credit or debit an account at the same time.

The way to do that would be like this:

public class Service {
    public void transferMoney(Account fromAcct, Account toAcct, int amount) {
        fromAcct.credit(amount);
        toAccount.debit(amount);
    }
}

class Account {
    private final object syncObject = new Object();
    private int amount = 0;

    public void credit(int sum) {
        synchronized(syncObject) {
            amount = amount + sum;
        }
    }

    public void debit(int sum) {
        synchronized(syncObject) {
            amount = amount - sum;
        }
    }
}

If your goal during the money transfer is to always ensure that both the credit and debit actions occur as one transaction, or atomically, then using synchronizing is not the right approach. Even in a synchronized block, if an exception were to occur, then you lose the guarantee that both actions will occur atomically.

Implementing transactions yourself is a very complex topic, which is why we normally use databases to do that for us.

EDIT: OP asked: What is the difference between my example (one synchronized block mux) and yours synchronized in Account class?

That's a fair question. There are a few differences. But I would say that the main difference is that, ironically, your example over-synchronizes. In other words, even though you are now using a single synchronized block, your performance is actually going to be much worse, potentially.

Consider the following example: You have 4 different bank accounts: Let's name them A, B, C, D. And now you have 2 money transfers that are initiated at the exact same time:

1. Money transfer from account A to account B.
2. Money transfer from account C to account D.

I think that you would agree that because the 2 money transfers are occurring on completely separate accounts, that there should be no harm (no risk of corruption) in executing both money transfers at the same time, right?

Yet, with your example, money transfers can only execute one after another. With mine, both money transfers occur simultaneously, yet safely as well. I will only block if both money transfers attempt to "touch" the same accounts.

Now imagine if you use this code to process hundreds, thousands or more concurrent money transfers. Then there is no doubt that my example will perform A LOT better than yours, while still retaining thread safety, and protecting the correctness of an account's balance.

In effect, my version of the code conceptually behaves a lot more like your original 2-synchronized block code. Except for the following improvements:

• Fixes the potential deadlock scenario.
• Intent is clearer.
• Provides better encapsulation. (Meaning that even if some other code outside the transferMoney method were to try to debit or credit some amounts, I would still retain thread safety, while you would not. I know you said that this is not your case, but with my version, the design absolutely guarantees it)

Answer 2

It looks like you want to implement Transaction by synchronization. There is nothing common between them. Transaction provides integrity of the operation - do all actions or roll them all back. Synchronization assures that data is changed from only one thread at a time. For example, transaction assures that if you take money from one account and not put them to another then first action is undo - money are not withdrawn. Synchronization checks that if two different dudes put 2 pennies to the bank in exact same moment then bank will have 4 pennies and not only 2 because your program adds money to the same account based on previous value.