Home > Net >  Using lock statement in c#
Using lock statement in c#

Time:05-26

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?

CodePudding user response:

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 (cash1 < Account), validation's passed
  2. Thread #2 tries to withdraw cash2, which is valid (cash2 < 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);
  } 
}
  • Related