`IServiceScopeFactory` is Documented Assumed to be a Singleton

[tillig]

Description

In the performance documentation for ASP.NET Core there is an example showing the use of IServiceScopeFactory from request services but making use of that factory after the request services set has been disposed.

[HttpGet("/fire-and-forget-3")]
public IActionResult FireAndForget3([FromServices]IServiceScopeFactory 
                                    serviceScopeFactory)
{
    _ = Task.Run(async () =>
    {
        await Task.Delay(1000);

        using (var scope = serviceScopeFactory.CreateScope())
        {
            var context = scope.ServiceProvider.GetRequiredService<ContosoDbContext>();

            context.Contoso.Add(new Contoso());

            await context.SaveChangesAsync();                                        
        }
    });

    return Accepted();
}

This documentation implies that the IServiceScopeFactory that is provided by request services is a singleton, or at least that it always provides service scopes that come from the root application container rather than being hierarchical. If this wasn't the case, then the parent scope that the IServiceScopeFactory came from would already have been disposed and you wouldn't be able to create a nested scope.

Basically, the documentation shows this:

• Root IServiceProvider
  • Request services scope
  • Scope created by IServiceScopeFactory

When, if scopes are hierarchical, it'd be more like:

• Root IServiceProvider
  • Request services scope
    • Scope created by IServiceScopeFactory (which couldn't happen, because request services has already been disposed)

There is no DI specification test indicating this all-scopes-must-come-from-the-root behavior, so either there's a documentation problem or there's a breaking change that will end up having to get introduced into the specification tests to make sure every conforming container implementation adheres to this two-level-only scope mechanism.

While I recognize that the context here is ASP.NET, since the DI bits are in this repo it seemed reasonable to start here. The behavior is more about what's expected from the DI implementation than it is about how ASP.NET consumes it.

Possible overlap with dotnet/aspnetcore#31478 where there is some discussion about the assumption IServiceScopeFactory is a singleton, but again, without any spec tests to enforce that and without discussing that decision with the community. This is only how the Microsoft container works, not how every conforming container works.

Reproduction Steps

Implement an ASP.NET app (or, I guess, a Console app) that performs effectively this sort of multi-threaded handling of scopes:

[HttpGet("/fire-and-forget-3")]
public IActionResult FireAndForget3([FromServices]IServiceScopeFactory 
                                    serviceScopeFactory)
{
    _ = Task.Run(async () =>
    {
        await Task.Delay(1000);

        using (var scope = serviceScopeFactory.CreateScope())
        {
            var context = scope.ServiceProvider.GetRequiredService<ContosoDbContext>();

            context.Contoso.Add(new Contoso());

            await context.SaveChangesAsync();                                        
        }
    });

    return Accepted();
}

Under the Microsoft container, this passes. Now back it with Autofac, which enforces hierarchical scopes. You'll fail because the request services scope has been disposed and you can't create a new child scope from a scope that's disposed.

Expected behavior

I expect the documentation to show an example that works with the default container and with conforming containers that adhere to the specification tests.

Actual behavior

Microsoft DI works, other conforming containers may or may not work despite passing the spec tests.

Regression?

Not a regression that I'm aware of.

Known Workarounds

Folks using other containers will have to use backing-container-specific workarounds. For example, instead of injecting an IServiceScopeFactory, folks wanting to do this with Autofac will need to hold a global static reference to the root container and manually create a child scope from that root container.

Configuration

The configuration, in this case, doesn't matter, though I'm on .NET 6 and the documentation is for .NET 6 that I linked.

Other information

The documentation should show something that works with both Microsoft and other conforming container providers. If that means that documentation needs to be removed because it's wrong, remove it. If it means introducing new requirements for conforming containers, that's definitely something to get the rest of the community involved in because it's going to break some folks who don't make this assumption.

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

In the performance documentation for ASP.NET Core there is an example showing the use of IServiceScopeFactory from request services but making use of that factory after the request services set has been disposed.

[HttpGet("/fire-and-forget-3")]
public IActionResult FireAndForget3([FromServices]IServiceScopeFactory 
                                    serviceScopeFactory)
{
    _ = Task.Run(async () =>
    {
        await Task.Delay(1000);

        using (var scope = serviceScopeFactory.CreateScope())
        {
            var context = scope.ServiceProvider.GetRequiredService<ContosoDbContext>();

            context.Contoso.Add(new Contoso());

            await context.SaveChangesAsync();                                        
        }
    });

    return Accepted();
}

This documentation implies that the IServiceScopeFactory that is provided by request services is a singleton, or at least that it always provides service scopes that come from the root application container rather than being hierarchical. If this wasn't the case, then the parent scope that the IServiceScopeFactory came from would already have been disposed and you wouldn't be able to create a nested scope.

Basically, the documentation shows this:

• Root IServiceProvider
  • Request services scope
  • Scope created by IServiceScopeFactory

When, if scopes are hierarchical, it'd be more like:

• Root IServiceProvider
  • Request services scope
    • Scope created by IServiceScopeFactory (which couldn't happen, because request services has already been disposed)

There is no DI specification test indicating this all-scopes-must-come-from-the-root behavior, so either there's a documentation problem or there's a breaking change that will end up having to get introduced into the specification tests to make sure every conforming container implementation adheres to this two-level-only scope mechanism.

While I recognize that the context here is ASP.NET, since the DI bits are in this repo it seemed reasonable to start here. The behavior is more about what's expected from the DI implementation than it is about how ASP.NET consumes it.

Possible overlap with dotnet/aspnetcore#31478 where there is some discussion about the assumption IServiceScopeFactory is a singleton, but again, without any spec tests to enforce that and without discussing that decision with the community. This is only how the Microsoft container works, not how every conforming container works.

Reproduction Steps

Implement an ASP.NET app (or, I guess, a Console app) that performs effectively this sort of multi-threaded handling of scopes:

[HttpGet("/fire-and-forget-3")]
public IActionResult FireAndForget3([FromServices]IServiceScopeFactory 
                                    serviceScopeFactory)
{
    _ = Task.Run(async () =>
    {
        await Task.Delay(1000);

        using (var scope = serviceScopeFactory.CreateScope())
        {
            var context = scope.ServiceProvider.GetRequiredService<ContosoDbContext>();

            context.Contoso.Add(new Contoso());

            await context.SaveChangesAsync();                                        
        }
    });

    return Accepted();
}

Under the Microsoft container, this passes. Now back it with Autofac, which enforces hierarchical scopes. You'll fail because the request services scope has been disposed and you can't create a new child scope from a scope that's disposed.

Expected behavior

I expect the documentation to show an example that works with the default container and with conforming containers that adhere to the specification tests.

Actual behavior

Microsoft DI works, other conforming containers may or may not work despite passing the spec tests.

Regression?

Not a regression that I'm aware of.

Known Workarounds

Folks using other containers will have to use backing-container-specific workarounds. For example, instead of injecting an IServiceScopeFactory, folks wanting to do this with Autofac will need to hold a global static reference to the root container and manually create a child scope from that root container.

Configuration

The configuration, in this case, doesn't matter, though I'm on .NET 6 and the documentation is for .NET 6 that I linked.

Other information

The documentation should show something that works with both Microsoft and other conforming container providers. If that means that documentation needs to be removed because it's wrong, remove it. If it means introducing new requirements for conforming containers, that's definitely something to get the rest of the community involved in because it's going to break some folks who don't make this assumption.

Author:	tillig
Assignees:	-
Labels:	untriaged, area-Extensions-DependencyInjection
Milestone:	-

[tillig]

/cc @davidfowl - this may be something for the DI Council.

[ohroy]

It was by following this document and using autofac that I discovered this problem. In fact, this problem doesn't replicate every time, it depends on whether Task.Run executes fast enough. It is precisely because of this that this has caused me great trouble and caused me a headache for a long time.
Hopefully, this time I'll have a chance to figure out what's going on

[davidfowl]

Oh this is a scary one, as there's assumptions baked in about this likely in a bunch of places. It's similar to how resolving a singleton from a scoped IServiceProvider returns a singleton instance. We'll need to add a spec test for this. If it was required to be a singleton, what part of autofac would break with that assumption?

[tillig]

Well, for Autofac, right now there's an implicit assumption that you get hierarchical scopes every time you create a new scope. There's a notion of being able to register things that only exist in a child scope, or that you can use child scopes in a sort of "disposable hierarchy." Folks use that as a sort of "unit of work" within the context of a controller action, creating child scopes to isolate resolved dependencies in atomic entities that can be disposed with that lifetime scope. The key here is that there's an assumption that scoped dependencies are shared down the tree, so

• Root container holds singletons
  • Request services hold request scoped stuff
    • Unit of work scopes can be created and disposed at will but will share the request scoped stuff without accidentally disposing it when the unit of work scope is disposed.

Thus, when the IServiceScopeFactory is injected, it's intentionally something that is based on the request services scope, not the root container, since this sort of hierarchical usage pattern is expected (at least in the context of Autofac).

So, back to the original question of what would break - I can think of a couple possible challenges.

First, consumers who are assuming that hierarchical scopes are a thing will probably be affected in hard to detect/insidious ways. "My controller action was using child scopes as units of work and instead of getting a sort of instance-per-request and sharing that. Now my unit of work is invoking the constructor again and my database logic is getting called twice. My perf is horrible! Dammit, Autofac!" Not everyone is doing this, but StackOverflow search for 'autofac unit of work' shows it's definitely non-zero, and I know we've seen questions via other mediums about the same thing.

Second, the multitenant DI mechanism we have effectively replaces the root container with a "multitenant container" and runs tenant ID detection one time - at request services creation. At that point, the app container is still the agnostic multitenant container, but request services have been scoped specifically to the detected tenant. Child scopes from that will automatically be associated with that tenant so the logic of detecting the tenant doesn't have to run every time. Think of it like this:

• Root container (tenant agnostic registrations)
  • Service scope for tenant 1 (tenant 1 registrations - this scope is the "tenant root")
    • Request services for tenant 1 (tenant is already identified, child scope from the "tenant root" is provided)
      • Unit of work in controller activated for tenant 1 (no need to re-identify the tenant, the scope has it inherited)

That's going to be an interesting challenge to make work with the "singleton scope factory" because it somewhat implies that every. single. time. you create a scope, no matter what, you're going to have to re-identify the tenant, which may not actually be something you can do depending on whether the current request context is now gone (fire-and-forget, for example, will have HttpContext and headers and tokens available on request, but not while that background task is running). Any perf optimization we could make by caching the hierarchy of shared root => tenant => request => unit of work scope instances is gone.

I haven't done any assessment on how other conforming containers have implemented theirs but this is the first time in all these years that I've heard the scope factory is supposed to be a singleton so I'd wager at least one other than Autofac isn't doing this.

@alistairjevans Can you think of anything I missed?

[alistairjevans]

Nothing I can see; I'll just second @tillig's statement that if you are an Autofac user, breaking default Autofac functionality by generating new scopes from the root each time will be extremely breaking.

Relatively intro-level users just moving from MSDI to Autofac for limited scenarios, less so, but a huge swathe of users depend on Autofac nested-unit-of-work.

It seems like there's a clash of assumptions here, with Autofac users coming to ASP.NET Core from existing MVC4 Autofac experience expecting nested-unit-of-work, and people coming MSDI expecting a singleton root container.

[davidfowl]

It seems to me like it has to be a singleton, otherwise there's no workaround for this outside of being container specific. We'll need to introduce a new interface here that has the nested semantics. I don't think we can walk this one back.

[tillig]

I mean, I get it, but at the same time this kinda sucks. I suppose if there's something else that can be added to support this notion of nested scopes, that's better than nothing, but showing up this far in with more relatively-undocumented assumptions is... well, let's say sub-optimal. It is... hmmm... frustrating. Yes, let's call it frustrating.

I guess it would be good to use this as a learning experience and think about how the definition of an interface needs to express behavioral intent fairly explicitly if there are expectations around that.

For example, when the challenge around returning resolved objects in the same order they were registered came up, using something like IOrderedEnumerable<T> instead of just IEnumerable<T> might have been a better option to express the intent of the behavior because plain IEnumerable<T> doesn't have any guarantee documented or otherwise about the order of the returned elements. Or, better, any time something needs to be returned in an order, it should express that order so it can be consistently sorted rather than assume the order for an effectively arbitrary bag of objects will always be maintained.

As new interfaces and integrations are added to ASP.NET Core or the .NET runtime, especially when there's a default implementation that is expected to be replaced by external providers (DI, logging, that sort of thing), it'd be a good idea to have more explicit expression about those sorts of "assumed behaviors" in the interface (names, return types, etc.) and documentation.

In the case of the conforming container, it feels like we're playing catch-up in these areas more often than not, and it's pretty frustrating to be caught up in.

[davidfowl]

We’ve been here before (and you’ve called out the example(s)) and I’m reminded once more about the reason we avoid changes to the core spec. We generally try it define all of these assumptions in the tests, this is just another one we missed. Not much we can do besides add it.

I’m not sure I would describe this as catch up though, it’s been 6 years that this ambiguity has been there.

[tillig]

I describe it as catch-up because, invariably, this seems to lead to every conforming container "catching up" to the assumptions made by the default container and having to make changes rather than considering it a bug in the core framework to fix the docs or default to something that assumes less. And every time, there's no PR coming to get that fixed; it's always on every container individually. So it may not be catch up from your end, but out here it sure feels like it. I mean, it is what it is, the default container has far more direct customers now since it's been required in ASP.NET Core (though it hasn't always been that way!) so there really is no other choice than to move forward and figure out how to fix it.

It would probably be good to decide if nested lifetime scopes is something that should ever be supported or if that's Autofac specific. Likely we'll end up recommending folks inject ILifetimeScope instead of IServiceScopeFactory if they want nested scopes today, but if there's some notion of that in the default container that hasn't been documented or decided, now would be a good time to figure it out so we can also get on implementing that.

[davidfowl]

It would probably be good to decide if nested lifetime scopes is something that should ever be supported or if that's Autofac specific

This has come up when discussing disposable transient services in blazor server.

Likely we'll end up recommending folks inject ILifetimeScope instead of IServiceScopeFactory if they want nested scopes today, but if there's some notion of that in the default container that hasn't been documented or decided, now would be a good time to figure it out so we can also get on implementing that.

Right, I think we're all discovering this together. Mostly because the default container doesn't have a notion of nested scopes.