René
René

Reputation: 108

Entity Framework "There is already an open DataReader associated with this Command which must be closed first"

I hope I can summarize this issue well enough. Let's start with the setup.

Setup

We have an ASP.NET Core Application and use Entity Framework (without Core). We use a general AuthorizeFilter for all controllers:

services.AddMvc(options =>
{
    var policy = new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build();
        options.Filters.Add(new AuthorizeFilter(policy));
})

And we have additional authorize policies applied to some controllers. e.g.:

[ApiController]
[Authorize("UserHasActivatedFeature")]
[Authorize("IsPremiumUser")]
public class PremiumFeatureController : ControllerBase

These policies are handled both with a single IAuthorizationHandler implementation. Looking like this:

public class UserAuthorizationHandler : IAuthorizationHandler
{
    private readonly IUserRepository _userRepository;
    public UserAuthorizationHandler(IUserRepository userRepository)
    {
        _userRepository = userRepository;
    }
    public Task HandleAsync(AuthorizationHandlerContext context)
    {
        var pendingRequirements = context.PendingRequirements.ToList();

        foreach (var requirement in pendingRequirements)
        {
            switch (requirement)
            {
                case PremiumRequirement:
                    if (_userRepository.IsPremium(userId))
                    {
                        context.Succeed(requirement);
                    }

                    break;
                case FeatureActivatedRequirement:
                    if (_userRepository.IsFeatureActivated(userId))
                    {
                        context.Succeed(requirement);
                    }

                    break;
            }
        }

        return Task.CompletedTask;
    }
}

and the UserRepository looks like this:

public class UserRepository : IRepository<User>, IUserRepository
{
    private DbSet<User> DbSet { get; private set; }
    public UserRepository(UnitOfWork unitOfWork)
    {
        DbSet = unitOfWork.DbContext.Set<User>();
    }
    public bool IsPremium(int userId)
    {
        return DbSet.AsNoTracking().Any(user => user.Id == userId && user.IsPremium);
    }
    public bool IsFeatureActivated(int userId)
    {
        return DbSet.AsNoTracking().Any(user => user.Id == userId && user.IsFeatureActivated);
    }
}

Problem

On our test environment, where about 20 people are testing our application. There are sometimes exceptions thrown from UserAuthorizationHandler saying: "There is already an open DataReader associated with this Command which must be closed first". When this exception is thrown it is thrown twice once from IsPremium and once from IsFeatureActivated.

Stacktrace (only 1 of the 2)

System.Data.Entity.Core.EntityCommandExecutionException: An error occurred while executing the command definition. See the inner exception for details. ---> System.InvalidOperationException: There is already an open DataReader associated with this Command which must be closed first.
at System.Data.SqlClient.SqlInternalConnectionTds.ValidateConnectionForExecute(SqlCommand command)
at System.Data.SqlClient.SqlCommand.ValidateCommand(String method, Boolean async)
at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry)
at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method)
at System.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior, String method)
at System.Data.Entity.Infrastructure.Interception.InternalDispatcher`1.Dispatch[TTarget,TInterceptionContext,TResult](TTarget target, Func`3 operation, TInterceptionContext interceptionContext, Action`3 executing, Action`3 executed)
at System.Data.Entity.Infrastructure.Interception.DbCommandDispatcher.Reader(DbCommand command, DbCommandInterceptionContext interceptionContext)
at System.Data.Entity.Core.EntityClient.Internal.EntityCommandDefinition.ExecuteStoreCommands(EntityCommand entityCommand, CommandBehavior behavior)
--- End of inner exception stack trace ---
at System.Data.Entity.Core.EntityClient.Internal.EntityCommandDefinition.ExecuteStoreCommands(EntityCommand entityCommand, CommandBehavior behavior)
at System.Data.Entity.Core.Objects.Internal.ObjectQueryExecutionPlan.Execute[TResultType](ObjectContext context, ObjectParameterCollection parameterValues)
at System.Data.Entity.Core.Objects.ObjectContext.ExecuteInTransaction[T](Func`1 func, IDbExecutionStrategy executionStrategy, Boolean startLocalTransaction, Boolean releaseConnectionOnSuccess)
at System.Data.Entity.Core.Objects.ObjectQuery`1.<>c__DisplayClass41_0.<GetResults>b__0()
at System.Data.Entity.SqlServer.DefaultSqlExecutionStrategy.Execute[TResult](Func`1 operation)
at System.Data.Entity.Core.Objects.ObjectQuery`1.GetResults(Nullable`1 forMergeOption)
at System.Data.Entity.Core.Objects.ObjectQuery`1.<System.Collections.Generic.IEnumerable<T>.GetEnumerator>b__31_0()
at System.Data.Entity.Internal.LazyEnumerator`1.MoveNext()
at System.Linq.Enumerable.Single[TSource](IEnumerable`1 source)
at awesomeCompany.Repositories.UserRepository.IsPremium(Int32 userId)
at awesomeCompany.API.Handlers.UserAuthorizationHandler.HandleAsync(AuthorizationHandlerContext context)
Inner Exception:
System.InvalidOperationException: There is already an open DataReader associated with this Command which must be closed first.
at System.Data.SqlClient.SqlInternalConnectionTds.ValidateConnectionForExecute(SqlCommand command)
at System.Data.SqlClient.SqlCommand.ValidateCommand(String method, Boolean async)
at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry)
at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method)
at System.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior, String method)
at System.Data.Entity.Infrastructure.Interception.InternalDispatcher`1.Dispatch[TTarget,TInterceptionContext,TResult](TTarget target, Func`3 operation, TInterceptionContext interceptionContext, Action`3 executing, Action`3 executed)
at System.Data.Entity.Infrastructure.Interception.DbCommandDispatcher.Reader(DbCommand command, DbCommandInterceptionContext interceptionContext)
at System.Data.Entity.Core.EntityClient.Internal.EntityCommandDefinition.ExecuteStoreCommands(EntityCommand entityCommand, CommandBehavior behavior)

What did I try?

As I said above this exception only appears from time to time (like 2 or 3 times a day) so I tried to isolate the issue locally and created unit tests to test the repository with multiple parallel calls at once, but I could not reproduce the issue. Calling the methods 100 times at the same time seams to be completely fine.

I read many similar questions here on stackoverflow. Many seam to have this issue when using a query inside another query or inside a IEnumerable loop, but since I am only using an Any() method on the database entities inside a normal foreach-loop iterating through a list that is completely unrelated from the userRepository I am not sure if any of the solutions apply here.

Also applying the Setting MultipleActiveResultSets=true to the connection string seams not to be a good solution and I would only consider it as a last option.

Upvotes: 0

Views: 2908

Answers (1)

Panagiotis Kanavos
Panagiotis Kanavos

Reputation: 131180

The problem is using a singleton in the first place. A DbContext is neither thread-safe nor meant to be long lived. It's a Unit-of-Work that caches any pending changes and either commits them all atomically when SaveChanges is called, or discards them when disposed. This means a DbContext should always be disposed once the unit of work is done.

Nothing is gained by using a singleton DbContext either. It's not a connection, so no time is saved by keeping it around for long. A DbContext opens and closes connections itself when it has to read data or persist changes. It doesn't keep them alive.

By making IAuthorizationHandler you're also keeping a DbContext instance as a singleton and end up using it concurrently from multiple requests. You may not get any exceptions until two requests try to read from the DbContext instance simultaneously.

Solution

Make UserAuthorizationHandler scoped

To fix this problem, the easiest solution seems to be to make UserAuthorizationHandler a scoped instance. The code posted here doesn't seem to use any other singleton services so it doesn't need to be a singleton itself.

Use scopes explicitly

If it has to remain a singleton, the DbContext and by extension, the UserRepository will have to be created inside a scope as needed. In this case, inside HandleAsync. To do that you need to inject IServiceProvider instead of IUserRepository. This is explained in Consuming a scoped service in a background task :

public class UserAuthorizationHandler : IAuthorizationHandler
{
    private readonly IServiceProvider _services;
    public UserAuthorizationHandler(IServiceProvider services)
    {
        _services = services;
    }
    public Task HandleAsync(AuthorizationHandlerContext context)
    {
        using (var scope = Services.CreateScope())
        {
            using(var userRepository=scope.GetRequiredService<IUserRepository>())
            {
                var pendingRequirements = context.PendingRequirements.ToList();
                ...
            }
        }
    }

BTW don't use blocking IO/database calls in an async method. This blocks a thread that could be used to serve other requests and increasing CPU usage due to spinwaits. A lot of the performance gains in ASP.NET Core come from avoiding unnecessary blocks

This means that either IsPremium and IsFeatureActivated should become asynchronous, or, if the synchronous methods are really needed, that async versions should be added. In this particular case both methods are used, so a good idea would be to use a single query to return both values :

public async Task<(bool isPremium,isFeatureActivated)> FeaturesForAsync(int userId)
{
    var results=await _context.Users
                        .Where(u=>u.Id==userId)
                        .Select(u=>new {u.IsPremium,u.IsFeatureActivated})
                        .SingleOrDefaultAsync();

    if (results==null) return default;
    return (results.IsPremium,results.IsFeatureActivated);
}

Or, using records instead of tuples :

record UserFeatures(bool IsPremium,bool IsFeatureActivated);

public async Task<(bool isPremium,isFeatureActivated)> FeaturesForAsync(int userId)
{
    var results=await _context.Users
                        .Where(u=>u.Id==userId)
                        .Select(u=>new UserFeatures( 
                                         u.IsPremium,
                                         u.IsFeatureActivated))
                        .SingleOrDefaultAsync();

    return results ?? new UserFeatures(false,false);
}

Upvotes: 2

Related Questions