2

I have a webapi and I want to make my logic inside this controller thread safe.

I want user can only update payroll when the last one updated and two update at the same time should not be happend.

As you can see in the code, I added a column in Payroll entity with the name of IsLock as boolean and try to handle multiple request for update in this way but it is not thread-safe.

How can I make it thread-safe ?

 [HttpPut("{year}/{month}")]
    public async Task<NoContentResult> Approve([FromRoute] int year, [FromRoute] int month)
    {
         var payroll = _dataContext.Payrolls
            .SingleOrDefaultAsync(p =>
            p.Month == month && p.Year == year);

        if (payroll.IsLock)
        {
            throw new ValidationException(
                $"The payroll {payroll.Id} is locked.");
        }

        try
        {
            payroll.IsLock = true;
            _dataContext.Payrolls.Update(payroll);
            await _dataContext.SaveChangesAsync(cancellationToken);

            payroll.Status = PayrollStatus.Approved;
            _dataContext.Payrolls.Update(payroll);
            await _dataContext.SaveChangesAsync(cancellationToken);

            payroll.IsLock = false;
            _dataContext.Payrolls.Update(payroll);
            await _dataContext.SaveChangesAsync(cancellationToken);

             return NoContent();
        }
        catch (Exception)
        {
            payroll.IsLock = false;
            _dataContext.Payrolls.Update(payroll);
            await _dataContext.SaveChangesAsync(cancellationToken);
            throw;
        }
    }
on ggg
  • 31
  • 1
  • Is this helpful? [C# Lock and Async Method](https://stackoverflow.com/questions/20084695/c-sharp-lock-and-async-method) – Theodor Zoulias Jan 22 '22 at 22:04
  • @TheodorZoulias I'm not sure if adding lock or semaphore for a webapi endpoint is a good idea or not. I mean is this the best practice for it ? What is the best way to handle this situation ? – on ggg Jan 22 '22 at 22:10
  • Note: making it thread safe won't work as soon as you scale out and more than 1 instance is serving requests. Then you need a distributed lock or something else like a queue. – Peter Bons Jan 22 '22 at 22:12
  • You can use transactions or EF Core's [concurrency tokens](https://learn.microsoft.com/en-us/ef/core/modeling/concurrency?tabs=data-annotations). Making 3 requests instead of one does not make much sense TBH. – Guru Stron Jan 22 '22 at 22:47
  • Adding a `SemaphoreSlim(1, 1)` is the best way to prevent concurrent execution of a block of code that contains `await` on a single process. I am not sure that the single-process requirement is satisfied for WebAPIs though. In case it's not, you would need a named semaphore (which is cross-process) as shown [here](https://stackoverflow.com/questions/69354109/async-friendly-cross-process-read-write-lock-net/69361877#69361877). – Theodor Zoulias Jan 22 '22 at 22:55
  • @GuruStron Could you please provide an example based on my code ? – on ggg Jan 22 '22 at 22:57

1 Answers1

2

You are looking for Concurrency Tokens. Each row in the payroll table would have one. When a user loaded the edit interface for a payroll, the concurrency token would be sent to the client. The client would include the concurrency token in the request to update the payroll. The update would only succeed of the concurrency token had not changed - meaning that the data had not changed since the user fetched it to start the edit.

Entity Framework uses the concurrency tokens internally, as well, so it won't save changes from a stale entity (where the data has changed since it was loaded).

The current IsLocked solution has some flaws. If two API requests are received at the same time, both may read the payroll data and see that it isn't locked. Both requests would then lock the row, make competing changes, and release the lock without ever realizing there were simultaneous edits.

John Glenn
  • 1,469
  • 8
  • 13