Skip to content
This repository has been archived by the owner on Dec 20, 2018. It is now read-only.

System.InvalidOperationException: The instance of entity type 'ApplicationUser' cannot be tracked because ... when trying to AddLoginAsync #914

Closed
gdoron opened this issue Jul 20, 2016 · 15 comments
Assignees

Comments

@gdoron
Copy link

gdoron commented Jul 20, 2016

@divega @HaoK @rustd I saw you all (in 3 different now closed issues) asked for repro code or stacktrace for the error:

System.InvalidOperationException: The instance of entity type 'ApplicationUser' cannot be tracked because another instance of this type with the same key is already being tracked. When adding new entities, for most key types a unique temporary key value will be created if no key is set (i.e. if the key property is assigned the default value for its type). If you are explicitly setting key values for new entities, ensure they do not collide with existing entities or temporary values generated for other new entities. When attaching existing entities, ensure that only one entity instance with a given key value is attached to the context.

So here are exact repro steps, code and stacktrace.

Startup:

services.AddDbContext<ApplicationDbContext>(options =>
        {
            options.UseSqlServer(Configuration.GetConnectionString("DefaultConnection"),
                b => b.MigrationsAssembly("CompanyName.Web")).ConfigureWarnings(x=> x.Log());
        },
        // When Scoped it works, when transient it doesn't 
        ServiceLifetime.Transient);

AccountController:

public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null, string remoteError = null)
{
    if (remoteError != null)
    {
        ModelState.AddModelError(string.Empty, $"Error from external provider: {remoteError}");
        return View(nameof(Login));
    }

    var info = await _signInManager.GetExternalLoginInfoAsync();
    if (info == null)
    {
        return RedirectToAction(nameof(Login));
    }

    // Sign in the user with this external login provider if the user already has a login.

    var result = await _signInManager.ExternalLoginSignInAsync(info.LoginProvider, info.ProviderKey, isPersistent: true);
    _logger.LogInformation("{name} - {email} tried to login with {provider}, {success}", info.Principal.Identity.Name, info.Principal.FindFirst(ClaimTypes.Email), info.LoginProvider, result.Succeeded);

    if (result.Succeeded)
    {
        _logger.LogInformation(5, "User logged in with {Name} provider.", info.LoginProvider);
        return RedirectToLocal(returnUrl);
    }
    if (result.RequiresTwoFactor)
    {
        return RedirectToAction(nameof(SendCode), new {ReturnUrl = returnUrl});
    }
    if (result.IsLockedOut)
    {
        return View("Lockout");
    }
    else
    {
        var picture = info.Principal.FindFirst("picture")?.Value;
        var firstName = info.Principal.FindFirst(ClaimTypes.GivenName)?.Value;
        var lastName = info.Principal.FindFirst(ClaimTypes.Surname)?.Value;
        var email = info.Principal.FindFirst(ClaimTypes.Email).Value;

        ApplicationUser user;
        if (!_context.Users.Any(x => x.Email == email))
        {
            _logger.LogInformation("{name} {email} doesn't exist in the DB", info.Principal.Identity.Name, info.Principal.FindFirst(ClaimTypes.Email));
            user = new ApplicationUser
                            {
                                UserName = email,
                                Email = email,
                                PictureUrl = picture,
                                FirstName = firstName,
                                LastName = lastName,
                                EmailConfirmed = true
                            };
            var createUserResult = await _userManager.CreateAsync(user);
            if (!createUserResult.Succeeded)
            {
                throw new Exception(string.Join(", ", createUserResult.Errors.Select(x => $"Code:{x.Code}. Description:{x.Description}")));
            }
        }
        // ONLY when it's not a new user (user has already a user in the DB but not a login for this provider) 
        else
        {
            _logger.LogInformation("{name} {email} exist in the DB", info.Principal.Identity.Name, info.Principal.FindFirst(ClaimTypes.Email));
            user = _context.Users.Single(x => x.Email == email);
        }

        // --------------------THROWS HERE-------------------
        var addLoginResult = await _userManager.AddLoginAsync(user, info); // THROWS HERE
        _logger.LogInformation("add external login to {email} {provider}, {success}", info.Principal.Identity.Name, info.LoginProvider, result.Succeeded);
        if (addLoginResult.Succeeded)
        {
            await _userManager.AddClaimsAsync(user, info.Principal.Claims);
            await _signInManager.SignInAsync(user, isPersistent: true);
            _logger.LogInformation(6, "User created an account using {Name} provider.", info.LoginProvider);
            return RedirectToLocal(returnUrl);
        }

        _logger.LogInformation("Can't login external login --not implemented {email} {provider}", info.Principal.Identity.Name, info.LoginProvider);

        throw new NotImplementedException();
    }
}

Exception trace:

System.InvalidOperationException: The instance of entity type 'ApplicationUser' cannot be tracked because another instance of this type with the same key is already being tracked. When adding new entities, for most key types a unique temporary key value will be created if no key is set (i.e. if the key property is assigned the default value for its type). If you are explicitly setting key values for new entities, ensure they do not collide with existing entities or temporary values generated for other new entities. When attaching existing entities, ensure that only one entity instance with a given key value is attached to the context.
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap1.Add(TKey key, InternalEntityEntry entry) at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.StartTracking(InternalEntityEntry entry) at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState oldState, EntityState newState, Boolean acceptChanges) at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.PaintAction(EntityEntryGraphNode node) at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph(EntityEntryGraphNode node, Func2 handleNode)
at Microsoft.EntityFrameworkCore.DbContext.SetEntityState[TEntity](TEntity entity, EntityState entityState)
at Microsoft.AspNetCore.Identity.EntityFrameworkCore.UserStore8.<UpdateAsync>d__35.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.AspNetCore.Identity.UserManager1.d__159.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Identity.UserManager1.<AddLoginAsync>d__91.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) at companyName.Web.Controllers.AccountController.<ExternalLoginCallback>d__13.MoveNext() in C:\Git\companyName\src\companyName.Web\Controllers\AccountController.cs:line 227 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.<InvokeActionFilterAsync>d__28.MoveNext() --- End of stack trace from previous location where exception was thrown --- at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.<InvokeAsync>d__18.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.AspNetCore.Builder.RouterMiddleware.<Invoke>d__4.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.<Invoke>d__18.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.<Invoke>d__18.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.<Invoke>d__18.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.<Invoke>d__18.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.<Invoke>d__18.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.<Invoke>d__18.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.<Invoke>d__18.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware1.d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware`1.d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.MigrationsEndPointMiddleware.d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.DatabaseErrorPageMiddleware.d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.DatabaseErrorPageMiddleware.d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.d__6.MoveNext()

Any insights?
If you really need, I can add you as collaborate to the private GitHub repository, it's easy to reproduce.

@gdoron
Copy link
Author

gdoron commented Jul 27, 2016

@HaoK @divega Can someone please give us some guidance on this issue?
We changed DbContext to be Scoped because of this bug, but DbContext isn't thread safe, and we DO need to run multiple queries at the same time, and every here and there we get race conditions errors.

Your help is appreciated.

@HaoK
Copy link
Member

HaoK commented Jul 27, 2016

@divega might have more info, but I believe you get this error exactly because of the fact that you are using different instances of a DbContext to manipulate the same entity. That's when I've seen exceptions regarding tracking issues in the past.

In general the default identity EF store expects the DbContext to be scoped and I'm not sure what level of support EF has for modifying the same entity in different DbContex concurrently...

@gdoron
Copy link
Author

gdoron commented Jul 27, 2016

@HaoK @divega Unless I'm missing something made internally by Identity, there aren't any entities being changed in different DbContex concurrently.
All the async operations are being awaited separately.

Please provide a solution, changing the lifetime of DbContext to Scope causes EF to throw in other places we actually do have and -by design- queries that run concurrently.
And changing the lifetime of DbContext to Transient causes Identity to throw here.

We don't know how to fix it and users complain about it for more than a week. 😢

@divega
Copy link

divega commented Jul 27, 2016

@HaoK @gdoron actually EF Core will throw this error if two different object instances representing the same entity (i.e. having the same key value) are somehow brought into the same DbContext instance, (i.e. using scoped DbContext instance, within the same request).

This could happen for example if a query materializes an ApplicationUser first and then a call to Add(), Attach() or to Update() tries to bring another object instance with the same key.

From looking at the code, it seems that such situation could arise UserManager.AddLoginAsync() at https://github.com/aspnet/Identity/blob/dev/src/Microsoft.AspNetCore.Identity/UserManager.cs#L891. We call await UpdateUserAsync(user) which in turns will attach the user to the context, but that would fail if the user (a separate object instance, just the same key) was previously retrieved by a query.

@HaoK I think we just need to try this repro steps to understand what is going on. I.e. if we are able to repro we should find out where the other instance of the user is being materialized.

@gdoron
Copy link
Author

gdoron commented Jul 27, 2016

@divega Thanks!
Let me know if you need access to my repository, it's being reproduced on each call.

@HaoK
Copy link
Member

HaoK commented Jul 27, 2016

So the issue is between _context and _userManager's store context I guess since the user is fetched from the controller DbContext here.

user = _context.Users.Single(x => x.Email == email);

And then added to the userManager context here, which throws as expected based on what @divega explained:

var addLoginResult = await _userManager.AddLoginAsync(user, info); // THROWS HERE

@gdoron
Copy link
Author

gdoron commented Jul 27, 2016

@HaoK That makes sense, but what caused Identity's DbContext to already track that user?

@HaoK
Copy link
Member

HaoK commented Jul 27, 2016

Try replacing the _context.Users.Single(x => x.Email == email) with _userManager.FindByEmailAsync and that might fix things

@gdoron
Copy link
Author

gdoron commented Jul 27, 2016

@HaoK

Changed my code to:

ApplicationUser user = await _userManager.FindByEmailAsync(email);
if (user != null)
{
    _logger.LogInformation("{name} {email} exist in the DB", info.Principal.Identity.Name,
        info.Principal.FindFirst(ClaimTypes.Email));
}
else
{
    _logger.LogInformation("{name} {email} doesn't exist in the DB", info.Principal.Identity.Name,
        info.Principal.FindFirst(ClaimTypes.Email));
    user = new ApplicationUser
    {
        UserName = email,
        Email = email,
        PictureUrl = picture,
        FirstName = firstName,
        LastName = lastName,
        EmailConfirmed = true
    };
    var createUserResult = await _userManager.CreateAsync(user);
    if (!createUserResult.Succeeded)
    {
        throw new Exception(string.Join(", ",
            createUserResult.Errors.Select(x => $"Code:{x.Code}. Description:{x.Description}")));
    }
}
 var addLoginResult = await _userManager.AddLoginAsync(user, info);

Seems to fix it 🎉

I think it's still a bug, Identity should be able to handle cases where an entity is already attached, but I'm good now.

BTW, I still don't understand why Identiy's dbcontext already tracked this user, maybe

var result = await _signInManager.ExternalLoginSignInAsync(info.LoginProvider, info.ProviderKey, isPersistent: true);

is causing it?

Thanks a ton for both of you @HaoK and @divega !!!

(You can close the issue if you think this is the expected behavior.)

@HaoK
Copy link
Member

HaoK commented Jul 27, 2016

Cool, #887 is related to the question about whether this is expected behavior, @divega thoughts?

@gdoron
Copy link
Author

gdoron commented Jul 27, 2016

@HaoK His question is vague and not the same as this one.
This one is about having multiple DbContext instances, and that's not even a question since EF isn't thread safe, Identity must support it.

His question is about having multiple DbContext types.

@HaoK
Copy link
Member

HaoK commented Jul 27, 2016

In general we mostly want you to use identity APIs to deal with the user, once you start mixing DbContext operations along side what identity's default EF implementation does, well as you have already seen, there be monsters...

@HaoK
Copy link
Member

HaoK commented Jul 27, 2016

FYI you still can do arbitrary queries against the users, just use _userManager.Users instead of the DbSet from your context.

@gdoron
Copy link
Author

gdoron commented Jul 27, 2016

FYI you still can do arbitrary queries against the users, just use _userManager.Users instead of the DbSet from your context.

Yes, but in an other thread you told me that Identity's UserManager is Scoped, and so its DbContext is scoped as well, which means, you can't run multiple queries in parallel using UserManager.
Well, it's 2:00 AM in my timezone, good night sirs.

@HaoK
Copy link
Member

HaoK commented Jul 27, 2016

After chatting with @divega we expect the user to be retrieved from identity or brand new, we don't currently have the necessary APIs from EF Core to applying changes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants