Locking Middleware or Service to prevent race conditions

Question

We have a centralized IdentityServer4-based authentication server to power all of our various applications. When a user accesses an application for the first time, a piece of middleware, UserProvisioningMiddleware determines if the current user has been provisioned in the current application (effectively inserting some of their JWT claims into an application-specific database). Under rare circumstances, a race condition can occur where multiple requests are made simultaneously, the middleware is executed for each request, and it attempts to provision the user multiple times. This results in an internal server exception as the JWT sub is used as the primary key.

An easy work-around would be placing a final check before saving the user to the database, and wrapping that call in a try-catch to silently discard the duplicate primary key error, but that is sub-optimal. A second potential solution would be to maintain a static HashSet of all users currently undergoing provisioning, and to create Task with a timer within my IsUserProvisioned method that waits for the user to be dequeued and provisioned, but that still feels like it could cause some potential deadlocking.

Here is the implementation of my user provisioning service:

public class UserProvisioningService: IUserProvisioningService
{
    private readonly IClaimsUserService _claimsUserService;
    private readonly ICurrentUserService _currentUserService;
    private readonly IJobSchedulingContext _context;

    public UserProvisioningService(
        IClaimsUserService claimsUserService,
        ICurrentUserService currentUserService,
        IJobSchedulingContext context)
    {
        _claimsUserService = claimsUserService;
        _currentUserService = currentUserService;
        _context = context;
    }
    
    /// <inheritdoc />
    public Task<bool> IsProvisionedAsync()
    {
        var userId = _currentUserService.UserId;
        if (userId == null)
            throw new NotAuthorizedException();

        return _context.Users
            .NotCacheable()
            .AnyAsync(u => u.Id == userId);
    }

    /// <inheritdoc />
    public Task ProvisionAsync()
    {
        var userId = _currentUserService.UserId;
        if (userId == null)
            throw new NotAuthorizedException();

        var claimsUser = _claimsUserService.GetClaimsUser();
        if (claimsUser == null)
            throw new InvalidOperationException("Cannot provision user");
        var user = new ApplicationUser(
            userId.Value, 
            claimsUser.GivenName, 
            claimsUser.FamilyName, 
            claimsUser.Email);
        _context.Users.Add(user);
        return _context.SaveChangesAsync();
    }
}

And the middleware

public class UserProvisioningMiddleware
{
    private readonly RequestDelegate _next;

    public UserProvisioningMiddleware(RequestDelegate next)
    {
        _next = next;
    }

    [UsedImplicitly]
    public async Task Invoke(
        HttpContext context, 
        IUserProvisioningService provisioningService)
    {
        if (context.User?.Identity?.IsAuthenticated == true)
            if (!await provisioningService.IsProvisionedAsync())
                await provisioningService.ProvisionAsync();

        await _next(context);
    }
}

What are my other options to prevent this race condition from occurring again in the future, without sacrificing performance?

Answer 1

• The best way is to use it as an atomic function. One approach is to create a method like TryProvisionAsync() which executes a stored procedure that performs all of the selection/insertion business and RETURNS appropriate result.

You can prevent race conditions in sql server or other relational databases and here is an example.

• Another solution is using Redis distributed lock that helps avoiding race conditions.

• The last solution is to hold an application lock inside your service, but if you want my opinion, it won't be a good approach, especially after load balancing and distribution scenarios of your service. Because different nodes of your service hold their own lock inside their own memory and this will be the single point of your condition failure.

third one implementation:

public interface IUnitOfWork {
     public async Task<bool> TryProvisionOrBeOstrich();
}

public class ProvissioningUnitOfWork : IUnitOfWork {
      private static readonly object _lock= new object();          
      private readonly IServiceScopeFactory _serviceScopeFactory;
    

//inject all of your services //inject your service provider to create some scope and get transient services.

      public async Task<book> TryProvisionOrBeOstrich() {
           //get your username from context
            using (var scope = _serviceScopeFactory.CreateScope())
            {

              var _claimsUserService = scope.GetService<IClaimsUserService>();
              var _currentUserService = scope.GetService<ICurrentUserService>();
              var _context = scope.GetService< IJobSchedulingContext context>();
               var provisioningService = scope.ServiceProvider.GetService<UserProvisioningService>();
               lock(_lock) 
               {
                      if (!await provisioningService.IsProvisionedAsync())
                     await provisioningService.ProvisionAsync();
               }
            }
      }
}

• Now you can add UnitOfWork as SingletonService and inject it to your controllers and use its method without any problem.