abpframework / abp

Open-source web application framework for ASP.NET Core! Offers an opinionated architecture to build enterprise software solutions with best practices on top of the .NET. Provides the fundamental infrastructure, cross-cutting-concern implementations, startup templates, application modules, UI themes, tooling and documentation.
https://abp.io
GNU Lesser General Public License v3.0
12.77k stars 3.41k forks source link

Concurrency issue with AbpLazyServiceProvider #7275

Closed olicooper closed 3 years ago

olicooper commented 3 years ago

Version: 4.2.0-rc.1

This happened in a background service when saving an entity to the database. I believe the EfCoreRepository instance was called concurrently and was probably sharing the same instance of AbpLazyServiceProvider. The CachedServices is not a ConcurrentDictionary so it threw an exception.

https://github.com/abpframework/abp/blob/a9bde97464d126d44516bf310292b053eb8c80dc/framework/src/Volo.Abp.Core/Volo/Abp/DependencyInjection/AbpLazyServiceProvider.cs#L7-L9

Exception details:

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.FindValue(TKey key)
   at System.Collections.Generic.Dictionary`2.TryGetValue(TKey key, TValue& value)
   at System.Collections.Generic.AbpDictionaryExtensions.GetOrAdd[TKey,TValue](IDictionary`2 dictionary, TKey key, Func`2 factory)
   at Volo.Abp.DependencyInjection.AbpLazyServiceProvider.LazyGetService(Type serviceType, Object defaultValue)
   at Volo.Abp.DependencyInjection.AbpLazyServiceProvider.LazyGetService[T](T defaultValue)
   at Volo.Abp.Domain.Repositories.BasicRepositoryBase`1.get_CancellationTokenProvider()
   at Volo.Abp.Domain.Repositories.BasicRepositoryBase`1.GetCancellationToken(CancellationToken preferredValue)
   at Volo.Abp.Domain.Repositories.EntityFrameworkCore.EfCoreRepository`2.InsertAsync(TEntity entity, Boolean autoSave, CancellationToken cancellationToken)
   at Castle.DynamicProxy.AsyncInterceptorBase.ProceedAsynchronous[TResult](IInvocation invocation, IInvocationProceedInfo proceedInfo)
   at Volo.Abp.Castle.DynamicProxy.CastleAbpMethodInvocationAdapterWithReturnValue`1.ProceedAsync()
   at Volo.Abp.Uow.UnitOfWorkInterceptor.InterceptAsync(IAbpMethodInvocation invocation)
   at Volo.Abp.Castle.DynamicProxy.CastleAsyncAbpInterceptorAdapter`1.InterceptAsync[TResult](IInvocation invocation, IInvocationProceedInfo proceedInfo, Func`3 proceed)

Related: https://github.com/abpframework/abp/issues/7227

maliming commented 3 years ago

hi

Can you share the steps to reproduce the problem?

I used Dictionary for performance. You can override AbpLazyServiceProvider to use ConcurrentDictionary.

Because I consider that there is no concurrency in a single http request.

olicooper commented 3 years ago

I will see if I can create a reproducible solution for you but my project is complicated so it might be hard to replicate. This is happening in a background service which executes an async 'processing' method when an email arrives in a monitored inbox, so it is likely that the code is executed concurrently.

There is another issue which I need to understand too, which is that the LazyServiceProvider is not being automatically injected into a nested DomainService, but I can use ServiceProvider.GetService to manually get an instance in the constructor. I don't know if this is a bug in my code yet but it is really strange because other DomainServices in the same call stack are able to get an instance of the LazyServiceProvider.

olicooper commented 3 years ago

I haven't made much progress so far. I can't reliably reproduce the issue because 2+ threads have to request the ICancellationTokenProvider from a shared instance of LazyServiceProvider at the same time. I think both threads are calling Dictionary.FindValue which returns false, so both threads try to add the ICancellationTokenProvider.

My debugging has included initializing the LazyServiceProvider before calling the IRepository.InsertAsync() method (see below) and not specifying the cancellationToken e.g. await _myRepository.InsertAsync(entity, false); but these haven't yielded any reliable results.

((IAbpLazyServiceProvider)ServiceProvider.GetService(typeof(IAbpLazyServiceProvider)));
await _myRepository.InsertAsync(entity, false, cancellationToken);

I have noticed that the token provider type is HttpContextCancellationTokenProvider despite the service running in the background (so not having a httpContext) and the service being initialised from the Application module not the HttpApi.Host module.

var tokenProviderType = ((IAbpLazyServiceProvider)ServiceProvider
  .GetService(typeof(IAbpLazyServiceProvider)))
  .LazyGetService<ICancellationTokenProvider>(NullCancellationTokenProvider.Instance)
  .GetType();
maliming commented 3 years ago

You can override AbpLazyServiceProvider to use ConcurrentDictionary or use lock at some place.

olicooper commented 3 years ago

I had an asynchronous event handler which must have been sharing a service scope. I created a new scope inside the event handler method using IServiceScopeFactory and then instantiated the required services inside that scope.

The other issue with LazyServiceProvider being null inside a DomainService was because Autofac was struggling to correctly inject required services when HttpClient was part of the class. I had to specify a concrete class instead of an interface when using AddHttpClient in ConfigureServices - I am not sure why this fixed the issue.

I am hoping that the issues are now resolved with these fixes. Thank you for helping.

booksir commented 2 years ago
2021-12-28 07:39:15.427 +08:00 [Error] Connection ID ""16501189066360887017"", Request ID ""800016ea-0007-e500-b63f-84710c7967bb"": An unhandled exception was thrown by the application.
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.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.Collections.Generic.Dictionary`2.set_Item(TKey key, TValue value)
   at System.Collections.Generic.AbpDictionaryExtensions.GetOrAdd[TKey,TValue](IDictionary`2 dictionary, TKey key, Func`2 factory)
   at Volo.Abp.DependencyInjection.AbpLazyServiceProvider.LazyGetService(Type serviceType, Object defaultValue)
   at Volo.Abp.DependencyInjection.AbpLazyServiceProvider.LazyGetService[T](T defaultValue)
   at Volo.Abp.Domain.Repositories.BasicRepositoryBase`1.GetCancellationToken(CancellationToken preferredValue)
   at Volo.Abp.PermissionManagement.EntityFrameworkCore.EfCorePermissionGrantRepository.GetListAsync(String providerName, String providerKey, CancellationToken cancellationToken)
   at Castle.DynamicProxy.AsyncInterceptorBase.ProceedAsynchronous[TResult](IInvocation invocation, IInvocationProceedInfo proceedInfo)
   at Volo.Abp.Castle.DynamicProxy.CastleAbpMethodInvocationAdapterWithReturnValue`1.ProceedAsync()
   at Volo.Abp.Uow.UnitOfWorkInterceptor.InterceptAsync(IAbpMethodInvocation invocation)
   at Volo.Abp.Castle.DynamicProxy.CastleAsyncAbpInterceptorAdapter`1.InterceptAsync[TResult](IInvocation invocation, IInvocationProceedInfo proceedInfo, Func`3 proceed)
   at Volo.Abp.PermissionManagement.PermissionStore.SetCacheItemsAsync(String providerName, String providerKey, String currentName, PermissionGrantCacheItem currentCacheItem)
   at Volo.Abp.PermissionManagement.PermissionStore.GetCacheItemAsync(String name, String providerName, String providerKey)
   at Volo.Abp.PermissionManagement.PermissionStore.IsGrantedAsync(String name, String providerName, String providerKey)
   at Volo.Abp.Authorization.Permissions.RolePermissionValueProvider.CheckAsync(PermissionValueCheckContext context)
   at Volo.Abp.Authorization.Permissions.PermissionChecker.IsGrantedAsync(ClaimsPrincipal claimsPrincipal, String name)
   at Volo.Abp.Authorization.PermissionRequirementHandler.HandleRequirementAsync(AuthorizationHandlerContext context, PermissionRequirement requirement)
   at Microsoft.AspNetCore.Authorization.AuthorizationHandler`1.HandleAsync(AuthorizationHandlerContext context)
   at Microsoft.AspNetCore.Authorization.DefaultAuthorizationService.AuthorizeAsync(ClaimsPrincipal user, Object resource, IEnumerable`1 requirements)
   at Microsoft.AspNetCore.Authorization.Policy.PolicyEvaluator.AuthorizeAsync(AuthorizationPolicy policy, AuthenticateResult authenticationResult, HttpContext context, Object resource)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Volo.Abp.AspNetCore.MultiTenancy.MultiTenancyMiddleware.InvokeAsync(HttpContext context, RequestDelegate next)
   at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.<>c__DisplayClass6_1.<<UseMiddlewareInterface>b__1>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Volo.Abp.AspNetCore.Tracing.AbpCorrelationIdMiddleware.InvokeAsync(HttpContext context, RequestDelegate next)
   at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.<>c__DisplayClass6_1.<<UseMiddlewareInterface>b__1>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.ResponseCompression.ResponseCompressionMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Localization.RequestLocalizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.RequestLocalization.AbpRequestLocalizationMiddleware.InvokeAsync(HttpContext context, RequestDelegate next)
   at Microsoft.AspNetCore.Builder.UseMiddlewareExtensions.<>c__DisplayClass6_1.<<UseMiddlewareInterface>b__1>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Server.IIS.Core.IISHttpContextOfT`1.ProcessRequestAsync()

ABP Version:4.3.3

volethanh commented 2 years ago

I had an asynchronous event handler which must have been sharing a service scope. I created a new scope inside the event handler method using IServiceScopeFactory and then instantiated the required services inside that scope.

The other issue with LazyServiceProvider being null inside a DomainService was because Autofac was struggling to correctly inject required services when HttpClient was part of the class. I had to specify a concrete class instead of an interface when using AddHttpClient in ConfigureServices - I am not sure why this fixed the issue.

I am hoping that the issues are now resolved with these fixes. Thank you for helping.

Hi @olicooper ,

Can you explain more detail how to solve this issue. I've got this problem when I run Background Service

thanks

olicooper commented 2 years ago

@volethanh you should check out the background workers in the docs, they use IServiceScopeFactory which you'll need to create a new scope, then use that scope to get the services you need with GetRequiredService