search

jbogard / MediatR.Extensions.Microsoft.DependencyInjection

MediatR extensions for Microsoft.Extensions.DependencyInjection
MIT License
327 stars 89 forks source link

Fix concurrency issue in ServiceFactory implementation #41

Closed ogaudefroy closed 5 years ago

ogaudefroy commented 5 years ago

Hey there,

This PR attempts to solve a concurrency issue we encountered using this library.

Here is the stack trace we occasionally have: Unexpected exception raised. Collection was modified; enumeration operation may not execute. System.InvalidOperationException: Collection was modified; enumeration operation may not execute. at System.Collections.Generic.List1.Enumerator.MoveNextRare() at MediatR.ServiceCollectionExtensions.<>c__DisplayClass15_1.<AddRequiredServices>b__1(Type type) at MediatR.ServiceFactoryExtensions.GetInstances[T](ServiceFactory factory) at MediatR.Internal.RequestHandlerWrapperImpl2.Handle(IRequest1 request, CancellationToken cancellationToken, ServiceFactory serviceFactory) at MediatR.Mediator.Send[TResponse](IRequest1 request, CancellationToken cancellationToken) at Procurement.PurchaseOrders.Common.Domain.QueryBus.Send[TQuery,TResponse](TQuery query) in C:\projets\procurement\purchase-orders\src\PurchaseOrders.Common\Domain\Queries\QueryBus.cs:line 24 etc...

The registered ServiceFactory implementation iterates over services and hot replaces service description ; I added a locking mechanism to ensure that services aren't modified while doing that overall process.

Olivier

jbogard commented 5 years ago

How does this happen? That method is only supposed to be called once on app startup. How can multiple threads be initializing?

ogaudefroy commented 5 years ago

Our DI container is configured like below:

services.AddMediatR(typeof(Startup).GetTypeInfo().Assembly);
services.AddSingleton<ICommandBus, CommandBus>();
services.AddSingleton<IDomainEventBus, DomainEventBus>();
services.AddSingleton<IQueryBus, QueryBus>();
services.AddScoped(typeof(IPipelineBehavior<,>, typeof(AxCommandExceptionHandler<,>) )

The concurrent replacement occurs on a registered pipeline behavior which has a per request scope:

Here is a trace with concurrent failure

[13:45:58 INF] MediatR resolving service: System.Collections.Generic.IEnumerable`1[[MediatR.IPipelineBehavior`2[[Procurement.PurchaseOrders.Queries.V1.FindPurchaseOrdersQuery, Procurement.PurchaseOrders, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null],[Procurement.PurchaseOrders.Queries.PagedResult`1[[Procurement.PurchaseOrders.Queries.V1.PurchaseOrderSearchResultItem, Procurement.PurchaseOrders, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]], Procurement.PurchaseOrders, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]], MediatR, Version=5.1.0.0, Culture=neutral, PublicKeyToken=null]]
[13:45:58 INF] MediatR resolving service: System.Collections.Generic.IEnumerable`1[[MediatR.IPipelineBehavior`2[[Procurement.PurchaseOrders.Queries.V1.FindPurchaseOrdersQuery, Procurement.PurchaseOrders, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null],[Procurement.PurchaseOrders.Queries.PagedResult`1[[Procurement.PurchaseOrders.Queries.V1.PurchaseOrderSearchResultItem, Procurement.PurchaseOrders, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]], Procurement.PurchaseOrders, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]], MediatR, Version=5.1.0.0, Culture=neutral, PublicKeyToken=null]]
[13:45:58 INF] MediatR resolving service: MediatR.IRequestHandler`2[[Procurement.PurchaseOrders.Queries.V1.FindPurchaseOrdersQuery, Procurement.PurchaseOrders, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null],[Procurement.PurchaseOrders.Queries.PagedResult`1[[Procurement.PurchaseOrders.Queries.V1.PurchaseOrderSearchResultItem, Procurement.PurchaseOrders, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]], Procurement.PurchaseOrders, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]]
[13:45:58 ERR] Unexpected exception raised. Collection was modified; enumeration operation may not execute.
System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
   at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
   at MediatR.ServiceCollectionExtensions.<>c__DisplayClass15_1.<AddRequiredServices>b__1(Type type) in C:\projets\procurement\purchase-orders\src\PurchaseOrders\Infrastructure\DependencyInjection\InternalMediatr.cs:line 304
   at MediatR.ServiceFactoryExtensions.GetInstances[T](ServiceFactory factory)
   at MediatR.Internal.RequestHandlerWrapperImpl`2.Handle(IRequest`1 request, CancellationToken cancellationToken, ServiceFactory serviceFactory)
   at MediatR.Mediator.Send[TResponse](IRequest`1 request, CancellationToken cancellationToken)
   at Procurement.PurchaseOrders.SharedKernel.QueryBus.Send[TQuery,TResponse](TQuery query) in C:\projets\procurement\purchase-orders\src\PurchaseOrders\SharedKernel\Queries\QueryBus.cs:line 24
   at Procurement.PurchaseOrders.Controllers.V1.PurchaseOrdersController.FindBySearchCriteria(FindPurchaseOrdersQuery query) in C:\projets\procurement\purchase-orders\src\PurchaseOrders\Controllers\V1\PurchaseOrdersController.cs:line 40
   at lambda_method(Closure , Object )
   at Microsoft.Extensions.Internal.ObjectMethodExecutorAwaitable.Awaiter.GetResult()
   at Microsoft.AspNetCore.Mvc.Internal.ActionMethodExecutor.AwaitableObjectResultExecutor.Execute(IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at System.Threading.Tasks.ValueTask`1.get_Result()
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.InvokeActionMethodAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.InvokeNextActionFilterAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.Rethrow(ActionExecutedContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.InvokeInnerFilterAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeNextExceptionFilterAsync()
[13:45:58 ERR] HTTP GET /api/v1/purchaseOrders/search responded 500 in 75.7205 ms
[13:45:59 INF] HTTP GET /api/v1/purchaseOrders/search responded 200 in 204.5497 ms
jbogard commented 5 years ago

Ahhhhh yes the constrained generic stuff. I think we can just call ToList or ToArray on the services before enumerating instead of locking. We're not modifying the list inside of the enumerable.

ogaudefroy commented 5 years ago

You're right your solution avoids locking and is by far better. Thx for Mediatr and taking our problem in consideration.

jbogard commented 5 years ago

No problem! Took me a while to understand the problem, sorry about that!