EN VI

C# - Storing a transient instance as a dictionary inside a singleton instance?

How to C# - Storing a transient instance as a dictionary inside a singleton instance

I'm following an existing code base trying to understand DI lifetimes, in the following code, we have a Factory class that is registered as a singleton which returns an instance from a dictionary if it exists or creates a new transient instance and returns it while storing it in the dictionary.

My question is whether the transient instances created and stored inside the factory will begin acting like a singleton? Is this a good coding approach?

My Program.cs

var builder = WebApplication.CreateBuilder(args);

builder.Services
    .AddHostedService<Consumer>()
    .AddSingleton<IPipelineFactory, PipelineFactory>()
    .AddTransient<IPipeline, Pipeline>();

var app = builder.Build();

app.MapGet("/", (IPipelineFactory pipelineFactory) =>
{
    return $"Total pipelines: {pipelineFactory.GetPipelineCount()}";
});


app.Run();

My Factory implementation,

public class PipelineFactory : IPipelineFactory
    {
        private readonly Dictionary<int, IPipeline> _pipelines = new();
        private readonly IServiceProvider _serviceProvider;

        public PipelineFactory(IServiceProvider serviceProvider)
        {
            _serviceProvider = serviceProvider;
        }

        public IPipeline GetPipeline(int pipelineId)
        {
            if (!_pipelines.ContainsKey(pipelineId))
            {
                using var scope = _serviceProvider.CreateScope();
                _pipelines.Add(pipelineId, scope.ServiceProvider.GetRequiredService<IPipeline>());                                
            }

            return _pipelines[pipelineId];
        }

        public int GetPipelineCount()
        {
            return _pipelines.Count;
        }
    }

Solution:

My question is whether the transient instances created and stored inside the factory will begin acting like a singleton?

Yes, effectively.

Is this a good coding approach?

Arguably - no. It can be somewhat tolerable (for example as quick-and-dirty fix or if the whole app lifetime is "scoped", i.e. it is some "runs ones" tool) but I would strongly recommend to reconsider the approach (especially in context of ASP.NET Core app, based on the shown code). Basically you are relying on the fact that the pipeline does not use internally any scoped services. If you don't have scoped dependencies then you don't need to create scope (_serviceProvider.CreateScope()).

If you have scoped dependencies consumed by the pipeline then your current code:

if (!_pipelines.ContainsKey(pipelineId))
{
    using var scope = _serviceProvider.CreateScope();
    _pipelines.Add(pipelineId, scope.ServiceProvider.GetRequiredService<IPipeline>());                                
}

Have at least the following problems:

The scope is disposed right after the addition to the dictionary, so all disposable dependencies created/managed by it will be disposed too
You are creating so called captive dependency which can result in unwanted/undesired behavior. For example if pipeline is using EF (with default AddDbContext setup which results in scoped context) then it can lead to big performance degradation and memory leaks (if you "handle" the disposal problem) due to the change tracking (which is used for create/update operations).