Do DI Factories need their own scope?

[OskarKlintrot]

When creating a di factory that resolves instances from the service provider, do I need to create a new scope before resolving the instance or is the service provider provided already in a scope so that I don't risk resolving instances from the root?

For example, I want to register JsonSerializerOptions to 1) avoid making the mistake of creating more instances than necessary and 2) avoid having to register the converters that I already have registered in AddJsonOptions. Should I then resolve IOptions<JsonOptions> directly from the service provider or should I create a new scope first?

builder.Services
    .AddSingleton(sp =>
    {
        using var scope = sp.CreateScope(); // Do I need this?

        return scope.ServiceProvider.GetRequiredService<IOptions<JsonOptions>>().Value.JsonSerializerOptions;
    })
    .AddControllers(options =>
    {
        options.Filters.Add(new AuthorizeFilter());
    })
    .AddJsonOptions(options => options.JsonSerializerOptions.Converters.Add(new MyJsonFactory()));

[OskarKlintrot]

I tried to look in the source code and bumped into this issue when trying to find ServiceProviderEngineScope. It looks like it's correct to assume that when using the di factories the ServiceProvider provided will already be correctly scoped and hence I don't need to create a new scope?

[KalleOlaviNiemitalo]

Because your code calls AddControllers, I presume you are using IMvcBuilder. Then:

• MvcCoreMvcBuilderExtensions.AddJsonOptions(this IMvcBuilder builder, Action<JsonOptions> configure) calls builder.Services.Configure(configure).
• OptionsServiceCollectionExtensions.Configure<TOptions>(this IServiceCollection services, Action<TOptions> configureOptions) calls services.Configure(Options.Options.DefaultName, configureOptions).
• OptionsServiceCollectionExtensions.Configure<TOptions>(this IServiceCollection services, string? name, Action<TOptions> configureOptions), calls services.AddOptions().
• OptionsServiceCollectionExtensions.AddOptions(this IServiceCollection services) calls services.TryAdd(ServiceDescriptor.Singleton(typeof(IOptions<>), typeof(UnnamedOptionsManager<>))).

Unless IOptions<JsonOptions> is also registered separately, it uses the singleton IOptions<> registration. Even if you try to resolve it from a service scope, the singleton is looked up and saved to the root scope.

In contrast, IOptionsSnapshot<> is registered as a scoped service. So if you requested the IOptionsSnapshot<JsonOptions> service, then that would be resolved in the service scope that you create. This would however be slower because it would do configuration binding from scratch and not return the same JsonOptions instance as what ASP.NET Core gets from IOptions<JsonOptions>.Value. And because you register JsonSerializerOptions itself as a singleton, the factory delegate is only called once; I don't see any benefit from using a scoped service there.

[OskarKlintrot]

I think you missed the actual question. A IDisposable should never be resolved from the root instance of the service provider. Now, IOptions<T> isn't disposable so my code isn't the best example (just happened to be what I had in front of me in terms of demonstrating resolving an instance from the service provider in a di factory). But let's say it was, then my question is whether the service provider given to Func<IServiceProvider, T> is already scoped correctly or if I should create a new scope. From what I can tell from the example here the service provider given to the factory is never the root and hence, there's no need to manually create a new scope to resolve instances from, even if the instance would happen to be disposable.

[KalleOlaviNiemitalo]

A IDisposable should never be resolved from the root instance of the service provider.

If a transient service is resolved multiple times from the root scope, and its implementation is disposable, then that can be a memory leak: the IServiceProvider creates a new instance on each call and has to preserve all of those instances until the IServiceProvider is disposed. However, it's no problem for a singleton service. For example, Microsoft.Extensions.Logging.ILoggerFactory is disposable and often resolved from the root scope.

In #3127, the IServiceProvider that is passed to the factory delegate of Ibbb is not ReferenceEqual to the IServiceProvider that BuildServiceProvider returned, but it still uses the root scope because Ibbb is registered as singleton.

[OskarKlintrot]

My example isn't the best and it clearly seems to be in the way of the actual question. So consider this instead:

public static class ServiceCollectionExtensions
{
    public static IServiceCollection AddBehaviour(this IServiceCollection services)
        => services
            .AddTransient<DisposableService>()
            .AddTransient<IBehaviour>(sp => sp.GetRequiredService<DisposableService>());
}

public interface IBehaviour { }

public class DisposableService : IBehaviour, IDisposable { }

[OskarKlintrot]

If we ignore all code, the question is do I ever need to create a new scope in a di factory?

[KalleOlaviNiemitalo]

A temporary service scope in a DI factory delegate could make sense if you wanted to create instances of services and dispose them before the factory returns. In which case, the service instance that the factory returns should not keep any references to the disposed services.