DbContext instances shouldn't be disposed

[sergezhigunov]

DbContext instances shouldn't be disposed

[abdusco]

What to do when the consumer is using IDbContextFactory<T>, though? If the library doesn't dispose the instances, it leaves unused instances around and puts pressure on the garbage collector.

IDbContextFactory caches the constructor. Is the model also cached? Is it compiled just once per application?
https://github.com/dotnet/efcore/blob/9ac01d6035c76626d89aa1a3cd8d200db2c3c0e1/src/EFCore/Internal/DbContextFactorySource.cs#L38-L73

When we dispose a dbcontext, we also can't utilize its internal entity cache, therefore every entity must be fetched from the database again if it's disposed.

But also, I can't dispose the dbcontext if I create it outside the contextBuilder delegate and use it inside the closure:

services.AddHangfire((provider, configuration) =>
    {
        var serviceScope = provider.CreateScope(); // share this scope (and dbcontext) among all calls to `contextBuilder` factory.
        configuration.UseEFCoreStorage(() => serviceScope.ServiceProvider.GetRequiredService<BlitzDbContext>(),
            new EFCoreStorageOptions());
       // but when to dispose the serviceScope?
    }
);

Because serviceScope stays alive until the app stops, the DbContext resolved from it also stays alive. And unlike in ASP.NET Core where the service scope is disposed at every request, it stays around until the root service provider is disposed (at the app exit).

Using IDbContextFactory<T> seems to be a better option. This means the library could keep using using statements, and only accept an IDbContextFactory instance in UseEFCoreStorage.
Or it could switch to using IPooledDbContextFactory and let the pool control the lifetime of dbcontext instances.

The problem is then reduced to the overhead of calling IDbContextFactory<T>.CreateDbContext(). Because when I look at the logs, there are many entries indicating it's initialized repeatedly:

info: Microsoft.EntityFrameworkCore.Infrastructure[10403]
      Entity Framework Core 5.0.7 initialized 'BlitzDbContext' using provider 'Npgsql.EntityFrameworkCore.PostgreSQL' with options: SensitiveDataLoggingEnabled DetailedErrorsEnabled MigrationsHistoryTable=__ef_migrations

So if it's not introducing a noticeable overhead, this issue is a noop (perhaps needs some docs/interface tweaks), otherwise I'm open to discuss this further.

[abdusco]

Actually, I can dispose of the serviceScope (and the DbContext instance it created) when the app is stopping:

services.AddHangfire((provider, configuration) =>
    {
        var serviceScope = provider.CreateScope();
        configuration.UseEFCoreStorage(() => serviceScope.ServiceProvider.GetRequiredService<BlitzDbContext>(),
            new EFCoreStorageOptions());
        
        // dispose the scope when the app is shutting down
        var lifetime = serviceScope.ServiceProvider.GetRequiredService<IHostApplicationLifetime>();
        lifetime.ApplicationStopping.Register(() => serviceScope.Dispose());
    }
);

So this actually is a viable option.

[jack775544]

The way that I do this normally and the way that it is implemented in the sample code is to instead inject a IDbContextFactory instead of of the db context. The contexts created by the factory are not bound to the scope and will work great in this case.

https://github.com/sergezhigunov/Hangfire.EntityFrameworkCore/blob/main/samples/Hangfire.EntityFrameworkCore.AspNetCoreExternalDbContext/Startup.cs#L25

The overhead of making a new db context every time is not that bad, in the end creating a new scope to make a db context is roughly the same as making a new context from the factory each time. The recommendation from the EF Core devs is that a DbContext is a short lived class that is to be used for a single set of operations, so making a new context from the factory for each operation seems to fit with those recommendations.