simpleinjector / SimpleInjector.Integration.AspNetCore

MIT License
2 stars 3 forks source link

Scoped lifetime is not working as expected when using Cross wiring with ASP.NET Core web applications #26

Open royvandertuuk opened 3 years ago

royvandertuuk commented 3 years ago

Creating a AsyncScopedLifestyle scope inside of a ASP.NET Core web request does not result in a new scoped service when this service is resolved via Simple Injector ASP.NET Core cross wiring. Instead, it is resolving the same instance as in the other scope.

I could only reproduce this issue in a ASP.NET Core app, I have attached a solution (Cross wiring issue.zip) that demonstrates the issue.

Below a snippet that produces the issue:

Resolve using cross wiring The CoreScopedService is registered in the ServiceCollection and cross wired using Simple Injector. Throws because scopedService1 and scopedService2 are the same instance.

CoreScopedService scopedService1;
CoreScopedService scopedService2;

using (var scope = AsyncScopedLifestyle.BeginScope(Startup.Container))
{
    scopedService1 = scope.Container.GetInstance<CoreScopedService>();
}

using (var scope = AsyncScopedLifestyle.BeginScope(Startup.Container))
{
    scopedService2 = scope.Container.GetInstance<CoreScopedService>();
}

if (object.ReferenceEquals(scopedService1, scopedService2))
{
    throw new InvalidOperationException("Same scoped service instance");
}

The same scenario is working in the following cases:

Defer resolving the instances until after the web request Does not throw, scopedService1 and scopedService 2 are separate instances.

 Task.Run(() =>
{
    // Sleep so resolving is done after the request has been completed.
    Thread.Sleep(1000);

    CoreScopedService scopedService1;
    CoreScopedService scopedService2;

    using (var scope = AsyncScopedLifestyle.BeginScope(Startup.Container))
    {
        scopedService1 = scope.Container.GetInstance<CoreScopedService>();
    }

    using (var scope = AsyncScopedLifestyle.BeginScope(Startup.Container))
    {
        scopedService2 = scope.Container.GetInstance<CoreScopedService>();
    }

    if (object.ReferenceEquals(scopedService1, scopedService2))
    {
        throw new InvalidOperationException("Same scoped service instance");
    }
});

Using SimpleInjector only Does not throw, scopedService1 and scopedService 2 are separate instances.

SimpleInjectorScopedService scopedService1;
SimpleInjectorScopedService scopedService2;

using (var scope = AsyncScopedLifestyle.BeginScope(Startup.Container))
{
    scopedService1 = scope.Container.GetInstance<SimpleInjectorScopedService>();
}

using (var scope = AsyncScopedLifestyle.BeginScope(Startup.Container))
{
    scopedService2 = scope.Container.GetInstance<SimpleInjectorScopedService>();
}

if (object.ReferenceEquals(scopedService1, scopedService2))
{
    throw new InvalidOperationException("Same scoped service instance");
}

Using only ASP.NET Core DI Does not throw, scopedService1 and scopedService 2 are separate instances.

CoreScopedService scopedService1;
CoreScopedService scopedService2;

using (var scope = _serviceScopeFactory.CreateScope())
{
    scopedService1 = scope.ServiceProvider.GetRequiredService<CoreScopedService>();
}

using (var scope = _serviceScopeFactory.CreateScope())
{
    scopedService2 = scope.ServiceProvider.GetRequiredService<CoreScopedService>();
}

if (object.ReferenceEquals(scopedService1, scopedService2))
{
    throw new InvalidOperationException("Same scoped service instance");
}
dotnetjunkie commented 3 years ago

What you are seeing is the default service-scope-reuse behavior. For more information see this.

royvandertuuk commented 3 years ago

Thank you, this resolved the issue. Why is the default behavior not ServiceScopeReuseBehavior.OnePerNestedScope? As this is how ASP.NET Core DI is behaving by default? I read that it is to maintain the state of the request, but I assume you only lose request state when you explicitly start a new SimpleInjector scope within a request.

dotnetjunkie commented 3 years ago

Why is the default behavior not ServiceScopeReuseBehavior.OnePerNestedScope? As this is how ASP.NET Core DI is behaving by default? I read that it is to maintain the state of the request, but I assume you only lose request state when you explicitly start a new SimpleInjector scope within a request.

That's correct. You'll only lose request state when you change the reuse behavior inside your manually created nested scope. As long as you don't create a nested scope within a request the OnPerRequest and OnePerNestedScope options behave identical.

OnPerRequest is the default behavior because "because it would otherwise force you to add quite some infrastructural code to get this data back". OnPerRequest is, therefore, the behavior that would works best for most developers, because most developers would expect their used framework components to still work, even if they create a nested scope.