Race condition (concurrency issue) in dependency resolver

[serialseb]

Moved from openrasta/openrasta-core#51

In DependencyManager.cs we have this little "test and set" code:

    public static object GetService(Type dependencyType)
    {
      ...
        if (AutoRegisterDependencies && !dependencyType.IsAbstract)
        {
            if (!_resolver.HasDependency(dependencyType))
                _resolver.AddDependency(dependencyType, DependencyLifetime.Transient);
        }
    }

If two threads does the HasDependency() check simultaneous and both of them get "false" then both will try to AddDependency - and one of them fails (depending on the chosen DI container). The Castle container throws an exception when inserting something that exists already.

Here is a stack trace "proof":

DEBUG 2013-02-22 09:50:36,481  5943ms  [22] Log4NetTraceListener   WriteLine          - Incoming host request for http://localhost/f2-restservices-main/parties/645585
DEBUG 2013-02-22 09:50:36,482  5944ms  [35] Log4NetTraceListener   WriteLine          - Incoming host request for http://localhost/f2-restservices-main/parties/645585
DEBUG 2013-02-22 09:50:36,484  5946ms  [19] Log4NetTraceListener   WriteLine          - Incoming host request for http://localhost/f2-restservices-main/parties/645585
DEBUG 2013-02-22 09:50:36,485  5947ms  [30] Log4NetTraceListener   Write              - openrasta Error: 0 : 
DEBUG 2013-02-22 09:50:36,486  5948ms  [30] Log4NetTraceListener   WriteLine          - An error has occurred and the processing of the request has stopped.

Exception:
Castle.MicroKernel.ComponentRegistrationException: Component CBrain.F2.Rest.Server.Modules.Parties.Codecs.RestPartyCodec could not be registered. There is already a component with that name. Did you want to modify the existing component instead? If not, make sure you specify a unique name.
   at Castle.MicroKernel.SubSystems.Naming.DefaultNamingSubSystem.Register(IHandler handler) in c:\BuildAgent\work\9834359f44c23fee\src\Castle.Windsor\MicroKernel\SubSystems\Naming\DefaultNamingSubSystem.cs:line 237
   at Castle.MicroKernel.DefaultKernel.Castle.MicroKernel.IKernelInternal.RegisterHandler(String name, IHandler handler, Boolean skipRegistration) in c:\BuildAgent\work\9834359f44c23fee\src\Castle.Windsor\MicroKernel\DefaultKernel.cs:line 718
   at Castle.MicroKernel.Handlers.DefaultHandlerFactory.Create(ComponentModel model, Boolean isMetaHandler) in c:\BuildAgent\work\9834359f44c23fee\src\Castle.Windsor\MicroKernel\Handlers\DefaultHandlerFactory.cs:line 41
   at Castle.MicroKernel.DefaultKernel.AddCustomComponent(ComponentModel model, Boolean isMetaHandler) in c:\BuildAgent\work\9834359f44c23fee\src\Castle.Windsor\MicroKernel\DefaultKernel.cs:line 265
   at Castle.MicroKernel.DefaultKernel.AddCustomComponent(ComponentModel model) in c:\BuildAgent\work\9834359f44c23fee\src\Castle.Windsor\MicroKernel\DefaultKernel.cs:line 272
   at Castle.MicroKernel.Registration.ComponentRegistration`1.Castle.MicroKernel.Registration.IRegistration.Register(IKernelInternal kernel) in c:\BuildAgent\work\9834359f44c23fee\src\Castle.Windsor\MicroKernel\Registration\ComponentRegistration.cs:line 1067
   at Castle.MicroKernel.DefaultKernel.Register(IRegistration[] registrations) in c:\BuildAgent\work\9834359f44c23fee\src\Castle.Windsor\MicroKernel\DefaultKernel.cs:line 519
   at Castle.Windsor.WindsorContainer.Register(IRegistration[] registrations) in c:\BuildAgent\work\9834359f44c23fee\src\Castle.Windsor\Windsor\WindsorContainer.cs:line 483
   at OpenRasta.DI.Windsor.WindsorDependencyResolver.AddDependencyCore(Type dependent, Type concrete, DependencyLifetime lifetime) in C:\Projects\F2-main\src\CBrain.OpenRasta\CBrain.OpenRasta\OpenRasta-Windsor-3\src\OpenRasta.DI.Windsor\WindsorDependencyResolver.cs:line 91
   at OpenRasta.DI.Windsor.WindsorDependencyResolver.AddDependencyCore(Type handlerType, DependencyLifetime lifetime) in C:\Projects\F2-main\src\CBrain.OpenRasta\CBrain.OpenRasta\OpenRasta-Windsor-3\src\OpenRasta.DI.Windsor\WindsorDependencyResolver.cs:line 143
   at OpenRasta.DI.DependencyResolverCore.AddDependency(Type concreteType, DependencyLifetime lifetime) in c:\Projects\OpenRasta\openrasta-stable\src\core\OpenRasta\DI\DependencyResolverCore.cs:line 31
   at OpenRasta.DI.DependencyManager.GetService(Type dependencyType) in c:\Projects\OpenRasta\openrasta-stable\src\core\OpenRasta\DI\DependencyManager.cs:line 87
   at OpenRasta.Pipeline.Contributors.ResponseEntityWriterContributor.WriteResponse(ICommunicationContext context) in c:\Projects\OpenRasta\openrasta-stable\src\core\OpenRasta\Pipeline\Contributors\ResponseEntityWriterContributor.cs:line 48
   at OpenRasta.Pipeline.PipelineRunner.ExecuteContributor(ICommunicationContext context, ContributorCall call) in c:\Projects\OpenRasta\openrasta-stable\src\core\OpenRasta\Pipeline\PipelineRunner.cs:line 187

[holytshirt]

Thanks @serialseb, we should think about moving way from checking the container and just not fail if a duplicate is added. Newer fast DI frameworks don't allow you to add to the container once have read from it, an example is Simple Injector, I also think the new asp.net DI the same.

[serialseb]

Well, we have two ways. For containers that support scoping, it doesn't matter cause you register the per-request dependency instances as singletons in the hosting api, for IComm, IRequest etc. For those not supporting it, I don't know what we can do. We can flow per request dependencies in an asynclocal, and pass the containers a Func for it to wireup at start time? I dunno how mvc injects this kind of stuff. May be worth looking into