simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.22k stars 152 forks source link

How to integrate ASP.NET MVC (net472) and Async Methods #940

Closed pfeigl closed 2 years ago

pfeigl commented 2 years ago

We are still running a rather large ASP.NET MVC code based based on net472 and are using SimpleInjector as an IoC.

While we migrated alot of our applications to .NET Core, it will take us quite some more time to migrate this specific application. We still would like to use async/await in many situations. However this is where we struggle a little with the integration. We successfully integrated SimpleInjector in some WebAPI (Owin based) stacks with AsyncScopedLifestyle and everything is working nice there.

In this specific project however we struggle a little with the combination of MVC + async/await, as there a two different integration packages (mvc vs webapi). What we tried to do sofar is to use the MVC package for the controller registration and than use the AsyncScopedLifestyle which we BeginScope and Dispose manually using Application_(Pre|Post)RequestHandlerExecute in Global.asax.

While this seems to work reasonable well, we have various occasions where we run into exceptions, where the construction of the controller runs into an instance requested out of a active scope exception.

I guess our question is: Is this setup in any way supported and if yes, what would be the best way to get it working? Is our approach reasonable?

dotnetjunkie commented 2 years ago

Using application pre and post request handler method can probably a bit tricky, but you seem to have already noticed this. The Simple Injector docs do explain the difference between WebRequestLifestyle and AsyncScopedLifestyle, but it unfortunately, won't give you a solution to your problem.

I think the trick is to create a Scope as close to the creation of the controller as possible. The most likely candidate is the IControllerFactory. The difficulty is to find a good way to be able to end the scope. This should likely be done in its ReleaseController method. You only have to find a way to store the Scope instance, and I think a safe way to do this (within the context of MVC) is with the HttpContext.Items dictionary. Try this implementation:

public class SimpleInjectorAsyncScopedControllerFactory : DefaultControllerFactory
{
    private readonly Container container;

    public SimpleInjectorAsyncScopedControllerFactory(Container container)
    {
        this.container = container;
    }

    public override IController CreateController(RequestContext c, string name)
    {
        HttpContext.Current.Items[nameof(AsyncScopedLifestyle)] =
            AsyncScopedLifestyle.BeginScope(this.container);

        return base.CreateController(c, name);
    }

    public override void ReleaseController(IController controller)
    {
        base.ReleaseController(controller);

        (HttpContext.Current.Items[nameof(AsyncScopedLifestyle)] as IDisposable)?.Dispose();
    }
}

Please note that I haven't tested this code, so don't push it to production right away ;-).

I believe that if you register this class in Simple Injector, MVC will start using it right away:

container.RegisterSingleton<IControllerFactory>(
    new SimpleInjectorAsyncScopedControllerFactory(container));

Alternatively, you can also set it using:

ControllerBuilder.Current.SetControllerFactory(new SimpleInjectorAsyncScopedControllerFactory(container));

Let me know whether that works. If the HttpContext.Current is not available when ReleaseController is called, we need a slightly more complex solution that involves using a static list and a weak reference.

pfeigl commented 2 years ago

Hi @dotnetjunkie , thanks for the answer and sorry for coming back on this that late. While the approach looked very promising at first, it doesn't really solve our problems, because there is various things outside of the actual controller action which might access the DI container earlier (or later). Things like ActionFilter or ModelBinder come into mind here.

What I'm currently experimenting with is a combination of the current AsyncScopedLifestyle and the WebRequestLifestyle and I try to integrate them against each other in a way that they both keep the same scope and are able to kind of resurrect the other one whenever of them loses the scope due to Thread / Async natures of different code areas.

I'm trying to do this within the OnExecuteRequestStep handler. The idea is from this SO https://stackoverflow.com/questions/43391498/asynclocal-value-is-null-after-being-set-from-within-application-beginrequest

pfeigl commented 2 years ago

Our plan seems that it worked out pretty nicely (Thanks to #880) . This is pretty much what we do now and sofar could resolve all of our problems

        public override void Init()
        {
            base.Init();

            this.OnExecuteRequestStep((context, step) =>
            {
                var lifestyle = _container.Options.DefaultScopedLifestyle;
                var currentScope = lifestyle.GetCurrentScope(_container);
                var httpContextScope = context.Items["scope"] as Scope;

                if (currentScope == null && httpContextScope != null)
                {
                    lifestyle.SetCurrentScope(httpContextScope);
                }

                step();
            });
        }

        protected void Application_PreRequestHandlerExecute(object sender, EventArgs e)
        {
            var scope = AsyncScopedLifestyle.BeginScope(_container);
            HttpContext.Current.Items["scope"] = scope;
        }

        protected void Application_PostRequestHandlerExecute(object sender, EventArgs e)
        {
            var scope = HttpContext.Current.Items["scope"] as Scope;
            scope?.Dispose();
        }

We could probably implement this a little better by defining our own hybrid-like lifestyle which does the heavy lifting internally, by hooking up to the OnExecuteRequestStep event itself, but this was easy enough for now and solves our problems.