aspnet / Identity

[Archived] ASP.NET Core Identity is the membership system for building ASP.NET Core web applications, including membership, login, and user data. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
1.96k stars 869 forks source link

Race condition in userManager.CreateAsync #1942

Closed vankampenp closed 5 years ago

vankampenp commented 6 years ago

In a site where I create users on the fly I have the following code:

var user = await userManager.FindByNameAsync(userName);
if (user == null) //does not exist create it
{
     user = new IdentityUser
    {
           UserName = userName
    ..
    }                     
    IdentityResult idresult = await userManager.CreateAsync(user, password);
    if (!idresult.Succeeded)
    {
           if (idresult.Errors.FirstOrDefault(e => e.Code == "DuplicateUserName") == null)
           {           
                    ...
            }
            //duplicate user from a different thread
             user = await userManager.FindByNameAsync(userName);
        }
...

In a race condition, where a user tried to create the user the same time on a mobile and a PC (with different IP addresses), I got an uncatched error on CreateAsync

Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while updating the entries. See the inner exception for details. ---> System.Data.SqlClient.SqlException: Cannot insert duplicate key row in object 'dbo.AspNetUsers' with unique index 'UserNameIndex'. The duplicate key value is (...).

I don't understand why the store did not catch the errror, as it can return a DuplicateUserName error, but that was not returned here.

blowdart commented 6 years ago

@HaoK

vankampenp commented 5 years ago

2018-09-30 16:37:07.7274|86.85.146.189|||ERROR|8.9.26.0|176|Base|SessionUser.MoveNext|User create error in FindOrCreate|Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while updating the entries. See the inner exception for details. ---> System.Data.SqlClient.SqlException: Cannot insert duplicate key row in object 'dbo.AspNetUsers' with unique index 'UserNameIndex'. The duplicate key value is (...). The statement has been terminated. at System.Data.SqlClient.SqlCommand.<>c.b__1220(Task1 result) at System.Threading.Tasks.ContinuationResultTaskFromResultTask2.InnerInvoke() at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) --- End of stack trace from previous location where exception was thrown --- at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot) --- End of stack trace from previous location where exception was thrown --- at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.ExecuteAsync(IRelationalConnection connection, DbCommandMethod executeMethod, IReadOnlyDictionary`2 parameterValues, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken) --- End of inner exception stack trace --- at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(DbContext , ValueTuple2 parameters, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func4 operation, Func4 verifySucceeded, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IReadOnlyList1 entriesToSave, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken) at Microsoft.AspNetCore.Identity.EntityFrameworkCore.UserStore9.CreateAsync(TUser user, CancellationToken cancellationToken) at Microsoft.AspNetCore.Identity.UserManager1.CreateAsync(TUser user) at Microsoft.AspNetCore.Identity.UserManager1.CreateAsync(TUser user, String password) at DTOWEB.Dal.SessionUser.FindOrCreate(UserManager1 userManager, SignInManager`1 signInManager, String loginId, Int32 dtoId, Int32 dtoClientId, Int32 genderId, String languageId)

HaoK commented 5 years ago

This behavior sounds expected to me if you have two requests trying to create the user, we aren't locking so they both check that the user name isn't taken and they both try to SaveChanges, and its the UserNameIndex unique constraint that is causing one of the SaveChanges to fail when both requests are trying to commit the update...

@ajcvickers does this seem reasonable behavior to you?

blowdart commented 5 years ago

We're closing this issue as the behaviour discussed seems to be by design.

vankampenp commented 5 years ago

I agree with the analyses, but I would expect it to catch this error catch (Microsoft.EntityFrameworkCore.DbUpdateException e)

and then return a DuplicateUserName error