Run every healthcheck in its own scope

[ThomasBergholdWieser]

Is your feature request related to a problem? Please describe.

Right now adding multiple different checks using the same DbContext type will result in some of them being failed, because the DbContext might already be disposed.

Default Lifetime of DbContext is scoped, it would be a shame to have to change that to transient for HealthChecks to work properly in this scenarop.

Describe the solution you'd like

Please change or add options to configure this behavior to run every healthcheck in its own scope,

Additional context

working repro here:
https://github.com/ThomasBergholdWieser/healthchecksrepro

Just execute the /health endpoint a few times, it should happen pretty often. Change the Lifetime to Transient to make it work.

[javiercn]

@ThomasBergholdWieser Thanks for contacting us.

Have you tried resolving an IServiceScopeFactory within your HealthCheck and using that to create your own scope and resolve the DbContext from there?

[dasMulli]

But that doesn't help that the inbox EF health check already created a dbcontext in the parent scope and the new sub-scope resolves the already created context from the parent scope?

[ThomasBergholdWieser]

Injecting the IServiceScopeFactory helps with this check, but somehow i am now getting weird issues with the default AddDbContext Check. I have updated my repro to include another version of the Custom Check with the service scope factory.

Here is an example response:

{
  "status": "Unhealthy",
  "statusComponents": [
    {
      "key": "AddDbContextCheck",
      "value": "Unhealthy",
      "desc": "An attempt was made to use the context while it is being configured. A DbContext instance cannot be used inside OnConfiguring since it is still being configured at this point. This can happen if a second operation is started on this context before a previous operation completed. Any instance members are not guaranteed to be thread safe.",
      "ex": "An attempt was made to use the context while it is being configured. A DbContext instance cannot be used inside OnConfiguring since it is still being configured at this point. This can happen if a second operation is started on this context before a previous operation completed. Any instance members are not guaranteed to be thread safe."
    },
    {
      "key": "CustomCheckInjectedScopeFactory",
      "value": "Healthy",
      "desc": null,
      "ex": null
    },
    {
      "key": "CustomCheckInjectedContext",
      "value": "Healthy",
      "desc": null,
      "ex": null
    }
  ]
}

[javiercn]

@ThomasBergholdWieser This goes beyond my knowledge. I'm looping in some folks to help here.

@rynowak @bricelam any ideas on how to integrate EF + Healthchecks??

[rynowak]

Can you share the call-stack of the exception you're hitting?

health checks already creates a scope when it runs so all of your checks will share the same scope. https://github.com/aspnet/Extensions/blob/master/src/HealthChecks/HealthChecks/src/DefaultHealthCheckService.cs#L53

I expect the problems that you'd hit are related to executing checks concurrently which DbContext doesn't support. We should probably have an option to execute checks serially instead of in parallel.

Can I ask why you end up needing multiple health checks for the same DbContext? Is this because you want to run multiple queries on the same database and report their results separately as metrics?

[dasMulli]

We wanted to create both low-level technical health checks (AddDbContextCheck) and business health checks - e.g. "a lot of imports are failing", "work in progress is stacking up".

We assumed that health checks would be a good abstraction for composing health status. But apparently the lack of scoping with some EF specifics defies that.

I'd argue that in order to be a good compositional pattern, it should allow for isolation of each component or else it would force users to create an additional layer of abstraction (scope creation + registration of checks that need scope) for it to be usable without fearing unintended side effects of using some components together.

This may be a problem once libraries that add to / extend EF contexts (asp.net core identity?, hangfire, openiddict, ...) offer health check components that would otherwise not be able to run alongside one another.

[rynowak]

But apparently the lack of scoping with some EF specifics defies that.

Sorry, I still don't get it - we create a scope for all health checks and run them in there. Can you explain how this is causing your problem?

I'll try your repro and get back to you.

[dasMulli]

Looks like the issue really mostly is that the checks run in parallel. This makes using DbContext checks hard since DbContext is not thread safe.

This is a bit interesting since:

• Most other user code is executed serially and thus doesn't create issues with scoped dependencies. I might be mistaken for that, but do other things like authorisation policies execute in parallel? This really is a bit of a surprise. Could lead other users into similar issues with other non-thread-safe libraries.
• It will be hard to isolate and thread-safe custom checks or create a collection of own checks in addition to libraries that do have dependencies on scoped dependencies.

There would be two ways to mitigate this:

1. Option to execute checks serially.
2. Option to create scopes for each check.

1 should be a good start to help fix such issues. 2. would also fix issues where we can't control 3rd party checks. Like with EF where there wold be a race if executed serially but could have issues as well when one of the checks creates an instance in the outer scope and then the inner scopes resolve this object again (e.g. EF check that doesn't create its own scope).

[tiakun]

There would be two ways to mitigate this:

1. Option to execute checks serially.
2. Option to create scopes for each check.

By default, a service scope should be created for each check by HealthCheckService.

Checking serially is effective but it is more like workaround because checks should be able to run concurrently already just by having their own scope. Using only single scope for all checks like it is right now is also not safe, as the state of the services inside the scope might be altered by prior checks and it might affect the result of latter checks.

[Peter-Optiway]

I'm also getting the A second operation started on this context before a previous operation completed. This is usually caused by different threads using the same instance of DbContext. For more information on how to avoid threading issues with DbContext, see https://go.microsoft.com/fwlink/?linkid=2097913. error.

We have HealthChecks that take the DbContext as a constructor parameter.

How do we solve this?

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[bricelam]

cc @ajcvickers

[rduman]

Hi All,

I had same issue after upgrading project to .net core 3.1. I fixed this problem

public virtual async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default(CancellationToken))
        {
            using (var scope = serviceProvider.CreateScope())
            {

                this.healthcheckService = scope.ServiceProvider.GetRequiredService<IHealthcheckService>(); // this calls my repository
                var doProcess = await DoProcess();
                return doProcess;
            }
        }

Healthcheck call CheckHealthAsync method for every implemented healthcheck simultaneously. I created scope for each task and it fixed the problem.

[mhmd-azeez]

This issue still happens in .net 5

This exception is thrown if you call /health endpoint concurrently:

System.InvalidOperationException
An attempt was made to use the context while it is being configured. A DbContext instance cannot be used inside 'OnConfiguring' since it is still being configured at this point. This can happen if a second operation is started on this context before a previous operation completed. Any instance members are not guaranteed to be thread safe.

IServiceProvider DbContext.get_InternalServiceProvider()

Assembly:Microsoft.EntityFrameworkCore
Version:5.0.5.0
Culture:neutral
PublicKeyToken:adb9793829ddae60
TService InfrastructureExtensions.GetService<TService>(IInfrastructure<IServiceProvider> accessor)

IDatabaseFacadeDependencies DatabaseFacade.get_Dependencies()

Task<bool> DatabaseFacade.CanConnectAsync(CancellationToken cancellationToken)

static DbContextHealthCheck<TContext>()+(TContext dbContext, CancellationToken cancellationToken) => { }

async Task<HealthCheckResult> DbContextHealthCheck<TContext>.CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken)

async Task<HealthReportEntry> DefaultHealthCheckService.RunCheckAsync(IServiceScope scope, HealthCheckRegistration registration, CancellationToken cancellationToken)

GET/health

Here is my workaround:

public class NpgsqlHealthCheck : IHealthCheck
{
    private string _connectionString;

    public NpgsqlHealthCheck(IConfiguration configuration)
    {
        _connectionString = configuration.GetConnectionString("DefaultConnection");
    }

    public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default)
    {
        try
        {
            using var connection = new NpgsqlConnection(_connectionString);

            await connection.OpenAsync();
            await connection.ExecuteScalarAsync("select 1;");

            return HealthCheckResult.Healthy();
        }
        catch (Exception ex)
        {
            return new HealthCheckResult(context.Registration.FailureStatus, exception: ex);
        }
    }
}

Note: For some reason, using DbContext using the health check was causing Connection leak. See history for the previous implementation.

[projectje]

I notice this problem also in .Net5.

I have around 50 projects in my solution each with their own contexts.

Each project has several healthchecks which simply injects the services from the specific project they need since it simply calls methods from these services that on their turn rely on connection being configured.

But I think, reading this, that this will never work.

But I think it is not really handy to "get connectionstring" in every healthcheck. since the methods they call are in the services in the projects. But I think this means that we have to duplicate these methods in the healthchecks?

[Sebbl22]

Same problem in our solution

[projectje]

Summary of what i now use :

• the semaphore prevents a gazillion callers all executing it
• the precheck retrieves results from cache since results are only needed to be returned live sometimes and also performed some other checks like if the db is upgrading e.g. does the table it queries exists at all at that moment
• its own scope because to prevent database parallel stuff exceptions

public class Acme_Identity_AdDc_LogCheck : IHealthCheck
{
    public IServiceProvider _serviceProvider;
    private static readonly SemaphoreSlim _mutex = new SemaphoreSlim(1, 1);
    private readonly AcmeIdentityAdDcOptions _options;

    public Acme_Identity_AdDc_LogCheck(IServiceProvider serviceProvider, IOptionsMonitor<AcmeIdentityAdDcOptions> options)
    {
        _serviceProvider = serviceProvider;
        _options = options.CurrentValue;
    }

    public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context,
      CancellationToken cancellationToken = default)
    {
        await _mutex.WaitAsync(cancellationToken);
        try
        {
            HealthCheckResult? preCheck = await _serviceProvider.IsDbUpgradingAsync(context);
            if (preCheck.HasValue) return preCheck.Value;
            await using (AsyncServiceScope scope = _serviceProvider.CreateAsyncScope())
            {
                ILogDatabaseService _logDatabaseService = scope.ServiceProvider.GetRequiredService<ILogDatabaseService>();
                string errorMessage = await _logDatabaseService.ErrorEntriesInSharedLogTable(IdentityAdDcExtensions.AssemblyName);
                return (errorMessage != string.Empty) ? await context.CacheResultAsync((errorMessage, false), _serviceProvider) :
                    await context.CacheResultAsync(("OK", true), _serviceProvider);
            }
        }
        catch (Exception ex)
        {
            return await context.CacheResultAsync((ex.Message, false), _serviceProvider, _options.HealthCheckCacheLogCheck.Hours,
                _options.HealthCheckCacheLogCheck.Minutes, _options.HealthCheckCacheLogCheck.Seconds);
        }
        finally
        {
            _mutex.Release();
        }
    }
}