Using lock statement in c#

I need to use the lock construction, and edit the following methods to execute in parallel:

    public void Withdraw(int amountToWithdraw)
            {
                if (amountToWithdraw <= 0)
                {
                    throw new ArgumentException("The amount should be greater than 0.");
                }
    
                if (amountToWithdraw > MaxAmountPerTransaction)
                {
                    throw new ArgumentException($"The value {amountToWithdraw} exceeds transaction limit: {MaxAmountPerTransaction}.");
                }
    
                if (amountToWithdraw > Amount)
                {
                    throw new ArgumentException("Insufficient funds.");
                }
                
                WithdrawAndEmulateTransactionDelay(amountToWithdraw);
    
            }

Here is the result

private readonly object balanceLock = new object();
public void Withdraw(int amountToWithdraw)
        {
            if (amountToWithdraw <= 0)
            {
                throw new ArgumentException("The amount should be greater than 0.");
            }

            if (amountToWithdraw > MaxAmountPerTransaction)
            {
                throw new ArgumentException($"The value {amountToWithdraw} exceeds transaction limit: {MaxAmountPerTransaction}.");
            }

            if (amountToWithdraw > Amount)
            {
                throw new ArgumentException("Insufficient funds.");
            }

            lock (balanceLock)
            {
                WithdrawAndEmulateTransactionDelay(amountToWithdraw);
            }

        }

This is a description of the method WithdrawAndEmulateTransactionDelay which shouldn’t be changed

private void WithdrawAndEmulateTransactionDelay(int amountToWithdraw)
        {
            Thread.Sleep(1000);
            Amount -= amountToWithdraw;
        }

However, the unit test failed. Where is the mistake in my code?

>Solution :

It seems, that you should put the last validation within the lock: in your current implementation it’s possible that

1. Thread #1 tries to withdraw cash1, which is valid (account1 < Account), validation’s passed
2. Thread #2 tries to withdraw cash2, which is valid (account2 < Account), validation’s passed
3. However cash1 + cash2 > Account
4. Thread #1 calls for WithdrawAndEmulateTransactionDelay, now Amount == Amount - cash1 < cash2
5. Thread #2 calls for WithdrawAndEmulateTransactionDelay; since Amount - cash1 < cash2 you have the test failed

private readonly object balanceLock = new object();

public void Withdraw(int amountToWithdraw) {
  // These validations are not depended on Amount, they don't want lock
  if (amountToWithdraw <= 0)
    throw new ArgumentOutOfRangeException(nameof(amountToWithdraw), 
      "The amount should be greater than 0.");

  if (amountToWithdraw > MaxAmountPerTransaction)
    throw new ArgumentOutOfRangeException(nameof(amountToWithdraw), 
      $"The value {amountToWithdraw} exceeds transaction limit: {MaxAmountPerTransaction}.");

  // from now on we start using Amount, so we need the lock:
  lock (balanceLock) {
    if (amountToWithdraw > Amount)
      throw new ArgumentException("Insufficient funds.", nameof(amountToWithdraw));

    WithdrawAndEmulateTransactionDelay(amountToWithdraw);
  } 
}