14

In my Asp.Net Core App I need a singleton service that I can reuse for the lifetime of the application. To construct it, I need a DbContext (from the EF Core), but it is a scoped service and not thread safe.

Therefore I am using the following pattern to construct my singleton service. It looks kinda hacky, therefore I was wondering whether this is an acceptable approach and won't lead to any problems?

services.AddScoped<IPersistedConfigurationDbContext, PersistedConfigurationDbContext>();
services.AddSingleton<IPersistedConfigurationService>(s =>
{
    ConfigModel currentConfig;
    using (var scope = s.CreateScope())
    {
        var dbContext = scope.ServiceProvider.GetRequiredService<IPersistedConfigurationDbContext>();
        currentConfig = dbContext.retrieveConfig();            
    }
    return new PersistedConfigurationService(currentConfig);
});

...

public class ConfigModel
{
    string configParam { get; set; }
}
eddyP23
  • 6,420
  • 7
  • 49
  • 87
  • There are many duplicates that show how to do this. So does the documentation for background services. `PersistedConfigurationService` should have an `IServiceProvider` parameter in its costructor. *It* should create the scope when needed. – Panagiotis Kanavos Apr 16 '19 at 12:47
  • A `ServiceProvider` is nothing more than a 'root' scope; therefore, you can just use it directly (i.e without the need to create scope). – Aly Elhaddad Apr 16 '19 at 12:48
  • 2
    Check [Consuming a scoped service in a background task](https://learn.microsoft.com/en-us/aspnet/core/fundamentals/host/hosted-services?view=aspnetcore-2.2#consuming-a-scoped-service-in-a-background-task). It shows that `IServiceProvider` is injected to the singleton service's constructor. The method that actually needs the scoped service, `DoWork`, creates the scope and requests the service only when it's actually needed. – Panagiotis Kanavos Apr 16 '19 at 12:48
  • @PanagiotisKanavos, thanks for sharing the link with an example. The remaining question I have is whether my approach is good then, as it is doing a similar thing to yours, but in a different place (when the construction of services happens, and not inside of a service) – eddyP23 Apr 16 '19 at 12:56
  • I don't think there is anything really wrong with this approach. You're not passing a scoped service into the `PersistedConfigurationService` - you're passing in configuration that's obtained from a scoped service, which is different. @PanagiotisKanavos Would you agree? One might argue that `PersistedConfigurationService` shouldn't know or care about `IPersistedConfigurationDbContext` here. – Kirk Larkin Apr 16 '19 at 12:59
  • @KirkLarkin Your previous comment make sense! Actually its not resolving the scoped service from singleton. Its just getting an instance of scoped service. am I correct? – TanvirArjel Apr 16 '19 at 13:02
  • 1
    @TanvirArjel Yes, that's right. It's easy to misread this and think we have a singleton attempting to consume a scoped service, but we don't. – Kirk Larkin Apr 16 '19 at 13:03
  • @KirkLarkin I edited the Q and got rid of a `var` and initialisation problem. Pls turn this into an answer, I am happy to accept it, thanks :) – eddyP23 Apr 16 '19 at 14:36
  • Here is rather comprehensive answer: [https://stackoverflow.com/questions/69179675/net-core-di-allowing-to-inject-scoped-service-into-singleton-on-local-machine/75201710#75201710](https://stackoverflow.com/questions/69179675/net-core-di-allowing-to-inject-scoped-service-into-singleton-on-local-machine/75201710#75201710) – Majid Shahabfar Jan 22 '23 at 15:51

3 Answers3

32

What you're doing is not good and can definitely lead to issues. Since this is being done in the service registration, the scoped service is going to be retrieve once when your singleton is first injected. In other words, this code here is only going to run once for the lifetime of the service you're registering, which since it's a singleton, means it's only going to happen once, period. Additionally, the context you're injecting here only exists within the scope you've created, which goes away as soon as the using statement closes. As such, by the time you actually try to use the context in your singleton, it will have been disposed, and you'll get an ObjectDisposedException.

If you need to use a scoped service inside a singleton, then you need to inject IServiceProvider into the singleton. Then, you need to create a scope and pull out your context when you need to use it, and this will need to be done every time you need to use it. For example:

public class PersistedConfigurationService : IPersistedConfigurationService
{
    private readonly IServiceProvider _serviceProvider;

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

    public async Task Foo()
    {
        using (var scope = _serviceProvider.CreateScope())
        {
             var context = scope.ServiceProvider.GetRequiredService<IPersistedConfigurationDbContext>();
             // do something with context
        }
    }
}

Just to emphasize, again, you will need to do this in each method that needs to utilize the scoped service (your context). You cannot persist this to an ivar or something. If you're put off by the code, you should be, as this is an antipattern. If you must get a scoped service in a singleton, you have no choice, but more often than not, this is a sign of bad design. If a service needs to use scoped services, it should almost invariably be scoped itself, not singleton. There's only a few cases where you truly need a singleton lifetime, and those mostly revolve around dealing with semaphores or other state that needs to be persisted throughout the life of the application. Unless there's a very good reason to make your service a singleton, you should opt for scoped in all cases; scoped should be the default lifetime unless you have a reason to do otherwise.

Chris Pratt
  • 232,153
  • 36
  • 385
  • 444
  • 1
    Thanks for such a detailed answer, but I think you misunderstood my question. My apologies, for not being clear. I have updated the question with the parts I thought are implicitly clear. – eddyP23 Apr 16 '19 at 14:53
  • And I believe that your suggestion is basically the same to what I have in my question :) – eddyP23 Apr 16 '19 at 14:56
  • 3
    No. I don't believe I misunderstood at all. If you need to access a scoped service in a singleton, the method in my answer above *is the only way to do so*. The code you have is fundamentally broken, and will not work at all. So, it's not even really an issue of is this "acceptable" or not, it won't work at all. – Chris Pratt Apr 16 '19 at 14:58
  • 1
    I am not passing a scoped service as an argument in a constructor of a singleton. I am instantiating a scoped service, using it to retrieve some data, then disposing it and never seeing the scoped object again. – eddyP23 Apr 16 '19 at 15:01
  • So `retrieveConfig` returns a static object that never changes? – Chris Pratt Apr 16 '19 at 15:02
  • @ChrisPratt if `IPersistedConfigurationDbContext` has other dependencies, and those dependencies are defined as Scoped, will it be enough to create the scope here? Will the scope be propagated to the dependencies? – Ben Jul 10 '20 at 20:12
  • Clear, Crisp and valid answer. – Nouman Qaiser Jul 15 '22 at 12:39
7

Although Dependency injection: Service lifetimes documentation in ASP.NET Core says:

It's dangerous to resolve a scoped service from a singleton. It may cause the service to have incorrect state when processing subsequent requests.

But in your case this is not the issue. Actually you are not resolving the scoped service from singleton. Its just getting an instance of scoped service from singleton whenever it requires. So your code should work properly without any disposed context error!

But another potential solution can be using IHostedService. Here is the details about it:

Consuming a scoped service in a background task (IHostedService)

TanvirArjel
  • 30,049
  • 14
  • 78
  • 114
1

Looking at the name of this service - I think what you need is a custom configuration provider that loads configuration from database at startup (once only). Why don't you do something like following instead? It is a better design, more of a framework compliant approach and also something that you can build as a shared library that other people can also benefit from (or you can benefit from in multiple projects).


public class Program
{
    public static void Main(string[] args)
    {
        CreateWebHostBuilder(args).Build().Run();
    }

    public static IWebHostBuilder CreateWebHostBuilder(string[] args) =>
        WebHost.CreateDefaultBuilder(args)
                .UseStartup<Startup>()
                .ConfigureAppConfiguration((context, config) =>
                {
                    var builtConfig = config.Build();
                    var persistentConfigBuilder = new ConfigurationBuilder();
                    var connectionString = builtConfig["ConnectionString"];
                    persistentStorageBuilder.AddPersistentConfig(connectionString);
                    var persistentConfig = persistentConfigBuilder.Build();
                    config.AddConfiguration(persistentConfig);
                });
}

Here - AddPersistentConfig is an extension method built as a library that looks like this.


public static class ConfigurationBuilderExtensions
{
    public static IConfigurationBuilder AddPersistentConfig(this IConfigurationBuilder configurationBuilder, string connectionString)
    {
          return configurationBuilder.Add(new PersistentConfigurationSource(connectionString));
    }

}

class PersistentConfigurationSource : IConfigurationSource
{
    public string ConnectionString { get; set; }

    public PersistentConfigurationSource(string connectionString)    
    {
           ConnectionString = connectionString;
    }

    public IConfigurationProvider Build(IConfigurationBuilder builder)
    {
         return new PersistentConfigurationProvider(new DbContext(ConnectionString));
    }

}

class PersistentConfigurationProvider : ConfigurationProvider
{
    private readonly DbContext _context;
    public PersistentConfigurationProvider(DbContext context)
    {
        _context = context;
    }


    public override void Load() 
    {
           // Using _dbContext
           // Load Configuration as valuesFromDb
           // Set Data
           // Data = valuesFromDb.ToDictionary<string, string>...
    }

}
Hiral Desai
  • 1,062
  • 1
  • 9
  • 24