ninject / Ninject

the ninja of .net dependency injectors
http://ninject.org/
Other
2.67k stars 531 forks source link

Concurrent Dictionary updates on .NET Core 2.1 throws exception #275

Open ternst opened 5 years ago

ternst commented 5 years ago

It doesn't happen too often and typically happens soon after the web server spins up when the server is under load. It looks to be related to a new exception that is thrown in .NET Core 2.1 https://github.com/dotnet/corefx/issues/31186.

The stack trace below shows it occurring in the Authentication pipeline in AuthenticationHttpContextExtensions here:

public static Task<AuthenticateResult> AuthenticateAsync(this HttpContext context, string scheme) =>
            context.RequestServices.GetRequiredService<IAuthenticationService>().AuthenticateAsync(context, scheme);

Full stack trace:

System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
   at System.Collections.Generic.Dictionary`2.FindEntry(TKey key)
   at Ninject.Infrastructure.Multimap`2.get_Item(TKey key)
   at Ninject.Components.ComponentContainer.GetAll(Type component)
   at Ninject.Components.ComponentContainer.GetAll[T]()
   at Ninject.KernelBase.GetBindings(Type service)
   at Ninject.KernelBase.Resolve(IRequest request, Boolean handleMissingBindings)
   at Ninject.KernelBase.System.IServiceProvider.GetService(Type service)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider provider, Type serviceType)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider)
   at Microsoft.AspNetCore.Authentication.AuthenticationHttpContextExtensions.AuthenticateAsync(HttpContext context, String scheme)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Server.IISIntegration.IISMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Builder.Extensions.UsePathBaseMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)
BrunoJuchli commented 5 years ago

Might this be related to https://github.com/ninject/Ninject/issues/128 ?

(see https://groups.google.com/forum/#!topic/ninject/5WiOkJIImDE)

ternst commented 5 years ago

I don't think so. The implementation of IAuthenticationService that ninject is returning is Microsoft.AspNetCore.Authentication.AuthenticationService, which has 3 dependencies, none of which depend on an IEnumerable. Furthermore, the top line of the stack trace is

System.Collections.Generic.Dictionary`2.FindEntry(TKey key)

, so it's the Dictionary that's throwing the error.

BrunoJuchli commented 5 years ago

@ternst The problem occurring in AuthenticationService might be caused by another component concurrently enumerating an IEnumerable. I'd expect the path which AuthenticationService is taking to take the proper locks to safe-guard against concurrent access, but the IEnumerable access not to.

ternst commented 5 years ago

@BrunoJuchli I think you're going down the wrong path. The exception being thrown is a new exception that Dictionary and HashSet will throw as of .NET Core 2.1. In this case, it's the Dictionary that Ninject.Infrastructure.Multimap is wrapping that's throwing the exception. There's no IEnumerable touching anything going on here.

As the above linked corefx issue shows, IdentityServer and EFCore have both had to fix internal stuff relating to how they were accessing dictionaries because of this. This certainly looks like the same thing.

BrunoJuchli commented 5 years ago

@ternst It's entirely possible that it's unrelated.

I am, however, talking about a concurrency issue and therefore one should be open minded about the possibilty that there are two entirely different code paths leading to a concurrency issue on the dictionary.

To be a bit more precise: If you have ninject inject an IEnumerable<T> where the Ts are bound by ninject, enumerating the IEnumerable<T> will lead to unsynchronized access to Ninject internals - might well be the Dictionary the MultiMap contains. Hence why I believe, with the info we currently have, it is a plausible cause.

kiranbhadani commented 5 years ago

I am getting similar issue from Dictionary object, Is there any solution?

System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct. at System.Collections.Generic.Dictionary2.FindEntry(TKey key) at System.Collections.Generic.Dictionary2.TryGetValue(TKey key, TValue& value) at Microsoft.EntityFrameworkCore.Metadata.Internal.Model.GetDisplayName(Type type) at Microsoft.EntityFrameworkCore.Metadata.Internal.Model.FindEntityType(Type type) at Microsoft.EntityFrameworkCore.Internal.InternalDbSet1.get_EntityType() at Microsoft.EntityFrameworkCore.Internal.InternalDbSet1.get_EntityQueryable() at Microsoft.EntityFrameworkCore.Internal.InternalDbSet1.System.Linq.IQueryable.get_Provider() at System.Linq.Queryable.Join[TOuter,TInner,TKey,TResult](IQueryable1 outer, IEnumerable1 inner, Expression1 outerKeySelector, Expression1 innerKeySelector, Expression1 resultSelector)

BrunoJuchli commented 5 years ago

@kiranbhadani are you using ninject?