Properly handling DbContexts in ASP.NET Core WebApi

Question

Got a small confusion here. I'm not sure if I am handling my DbContext throughout the WebApi properly. I do have some controllers that do some operations on my DB (Inserts/Updates with EF) and after doing these actions I do trigger an event. In my EventArgs (I have a custom class which inherits from EventArgs) I pass my DbContext and I use it in the event handler to log these operations (basically I just log authenticated user API requests).

In the event handler when I am trying to commit my changes (await SaveChangesAsync) I get an error : "Using a disposed object...etc" basically noticing me that at the first time I use await in my async void (fire and forget) I notify the caller to dispose the Dbcontext object.

Not using async works and the only workaround that I've mangaged to put out is by creating another instance of DbContext by getting the SQLConnectionString of the EventArgs passed DbContext.

Before posting I did made a small research based on my issue Entity Framework disposing with async controllers in Web api/MVC

This is how I pass parameters to my OnRequestCompletedEvent

OnRequestCompleted(dbContext: dbContext,requestJson: JsonConvert.SerializeObject);

This is the OnRequestCompleted() declaration

 protected virtual void OnRequestCompleted(int typeOfQuery,PartnerFiscalNumberContext dbContext,string requestJson,string appId)
        {
       RequestCompleted?.Invoke(this,new MiningResultEventArgs()
          {
            TypeOfQuery = typeOfQuery,
            DbContext   = dbContext,
            RequestJson = requestJson,
            AppId = appId
          });
        }

And this is how I process and use my dbContext

var appId = miningResultEventArgs.AppId;
var requestJson = miningResultEventArgs.RequestJson;
var typeOfQuery = miningResultEventArgs.TypeOfQuery;
var requestType =  miningResultEventArgs.DbContext.RequestType.FirstAsync(x => x.Id == typeOfQuery).Result;
var apiUserRequester =  miningResultEventArgs.DbContext.ApiUsers.FirstAsync(x => x.AppId == appId).Result;

var apiRequest = new ApiUserRequest()
{
    ApiUser = apiUserRequester,
    RequestJson = requestJson,
    RequestType = requestType
};

miningResultEventArgs.DbContext.ApiUserRequests.Add(apiRequest);
await miningResultEventArgs.DbContext.SaveChangesAsync();

By using SaveChanges instead of SaveChangesAsync everything works. My only idea is to create another dbContext by passing the previous DbContext's SQL connection string

var dbOptions = new DbContextOptionsBuilder<PartnerFiscalNumberContext>();
dbOptions.UseSqlServer(miningResultEventArgs.DbContext.Database.GetDbConnection().ConnectionString);

    using (var dbContext = new PartnerFiscalNumberContext(dbOptions.Options))
    {
        var appId = miningResultEventArgs.AppId;
        var requestJson = miningResultEventArgs.RequestJson;
        var typeOfQuery = miningResultEventArgs.TypeOfQuery;


        var requestType = await dbContext.RequestType.FirstAsync(x => x.Id == typeOfQuery);
        var apiUserRequester = await dbContext.ApiUsers.FirstAsync(x => x.AppId == appId);

        var apiRequest = new ApiUserRequest()
        {
            ApiUser = apiUserRequester,
            RequestJson = requestJson,
            RequestType = requestType
        };

        dbContext.ApiUserRequests.Add(apiRequest);
        await dbContext.SaveChangesAsync();
    }

The latter code excerpt is just a small test to check my supposition, basically I should pass the SQL connection string instead of the DbContext object.

I am not sure (in terms of best practice) if I should pass a connection string and create a new dbContext object (and dispose it by using a using clause) or if I should use/have another mindset for this issue.

From what I know, using a DbContext should be done for a limited set of operations and not for multiple purposes.

EDIT 01

I'm going to detail more thorough what I've been doing down below.

I think I got an idea of why this error happens.

I have 2 controllers One that receives a JSON and after de-serializing it I return a JSON to the caller and another controller that gets a JSON that encapsulates a list of objects that I iterate in an async way, returning an Ok() status.

The controllers are declared as async Task<IActionResult> and both feature an async execution of 2 similar methods.

The first one that returns a JSON executes this method

await ProcessFiscalNo(requestFiscalView.FiscalNo, dbContext);

The second one (the one that triggers this error)

foreach (string t in requestFiscalBulkView.FiscalNoList)
       await ProcessFiscalNo(t, dbContext);

Both methods (the ones defined previously) start an event OnOperationComplete() Within that method I execute the code from my post's beginning. Within the ProcessFiscalNo method I DO NOT use any using contexts nor do I dispose the dbContext variable. Within this method I only commit 2 major actions either updating an existing sql row or inserting it. For edit contexts I select the row and tag the row with the modified label by doing this

dbContext.Entry(partnerFiscalNumber).State = EntityState.Modified;

or by inserting the row

dbContext.FiscalNumbers.Add(partnerFiscalNumber);

and finally I execute an await dbContext.SaveChangesAsync();

The error always gets triggered within the EventHandler ( the one detailed @ the beginning of the thread) during the await dbContext.SaveChangedAsync() which is pretty weird since 2 lines before that I do await reads on my DB with EF.

 var requestType = await dbContext.RequestType.FirstAsync(x => x.Id == typeOfQuery);
 var apiUserRequester = await dbContext.ApiUsers.FirstAsync(x => x.AppId == appId);

 dbContext.ApiUserRequests.Add(new ApiUserRequest() { ApiUser = apiUserRequester, RequestJson = requestJson, RequestType = requestType });

  //this throws the error
 await dbContext.SaveChangesAsync();

For some reason calling await within the Event Handler notifies the caller to dispose the DbContext object. Also by re-creating the DbContext and not re-using the old one I see a huge improvement on access. Somehow when I use the first controller and return the info the DbContext object appears to get flagged by the CLR for disposal but for some unknown reason it still functions.

EDIT 02 Sorry for the bulk-ish content that follows, but I've placed all of the areas where I do use dbContext.

This is how I'm propagating my dbContext to all my controllers that request it.

 public void ConfigureServices(IServiceCollection services)
        {

         // Add framework services.
        services.AddMemoryCache();

        // Add framework services.
        services.AddOptions();
        var connection = @"Server=.;Database=CrawlerSbDb;Trusted_Connection=True;";
        services.AddDbContext<PartnerFiscalNumberContext>(options => options.UseSqlServer(connection));

        services.AddMvc();
        services.AddAuthorization(options =>
        {
            options.AddPolicy("PowerUser",
                              policy => policy.Requirements.Add(new UserRequirement(isPowerUser: true)));
        });

        services.TryAddSingleton<IHttpContextAccessor, HttpContextAccessor>();
        services.AddSingleton<IAuthorizationHandler, UserTypeHandler>();
    }

In Configure I'm using the dbContext for my custom MiddleWare

 public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory)
        {
            loggerFactory.AddConsole(Configuration.GetSection("Logging"));
            loggerFactory.AddDebug();

            var context = app.ApplicationServices.GetService<PartnerFiscalNumberContext>();
            app.UseHmacAuthentication(new HmacOptions(),context);

            app.UseMvc();
        }

In the custom MiddleWare I'm only using it for a query.

public HmacHandler(IHttpContextAccessor httpContextAccessor, IMemoryCache memoryCache, PartnerFiscalNumberContext partnerFiscalNumberContext)
        {
            _httpContextAccessor = httpContextAccessor;
            _memoryCache = memoryCache;
            _partnerFiscalNumberContext = partnerFiscalNumberContext;

            AllowedApps.AddRange(
                    _partnerFiscalNumberContext.ApiUsers
                        .Where(x => x.Blocked == false)
                        .Where(x => !AllowedApps.ContainsKey(x.AppId))
                        .Select(x => new KeyValuePair<string, string>(x.AppId, x.ApiHash)));
        }

In my controller's CTOR I'm passing the dbContext

public FiscalNumberController(PartnerFiscalNumberContext partnerContext)
        {
            _partnerContext = partnerContext;
        }

This is my Post

        [HttpPost]
        [Produces("application/json", Type = typeof(PartnerFiscalNumber))]
        [Consumes("application/json")]
        public async Task<IActionResult> Post([FromBody]RequestFiscalView value)
        {
            if (!ModelState.IsValid)
                return BadRequest(ModelState);

            var partnerFiscalNo = await _fiscalNoProcessor.ProcessFiscalNoSingle(value, _partnerContext);
        }

Within the ProcessFiscalNoSingle method I have the following usage, If that partner exists then I'll grab him, if not, create and return him.

internal async Task<PartnerFiscalNumber> ProcessFiscalNoSingle(RequestFiscalView requestFiscalView, PartnerFiscalNumberContext dbContext)
        {
            var queriedFiscalNumber =  await dbContext.FiscalNumbers.FirstOrDefaultAsync(x => x.FiscalNo == requestFiscalView.FiscalNo && requestFiscalView.ForceRefresh == false) ??
                                       await ProcessFiscalNo(requestFiscalView.FiscalNo, dbContext, TypeOfQuery.Single);

            OnRequestCompleted(typeOfQuery: (int)TypeOfQuery.Single, dbContextConnString: dbContext.Database.GetDbConnection().ConnectionString, requestJson: JsonConvert.SerializeObject(requestFiscalView), appId: requestFiscalView.RequesterAppId);

            return queriedFiscalNumber;
        }

Further down in the code, there's the ProcessFiscalNo method where I use the dbContext

 var existingItem =
        dbContext.FiscalNumbers.FirstOrDefault(x => x.FiscalNo == partnerFiscalNumber.FiscalNo);

    if (existingItem != null)
    {
        var existingGuid = existingItem.Id;
        partnerFiscalNumber = existingItem;

        partnerFiscalNumber.Id = existingGuid;
        partnerFiscalNumber.ChangeDate = DateTime.Now;

        dbContext.Entry(partnerFiscalNumber).State = EntityState.Modified;
    }
    else
        dbContext.FiscalNumbers.Add(partnerFiscalNumber);

    //this gets always executed at the end of this method
    await dbContext.SaveChangesAsync();

Also I've got an Event called OnRequestCompleted() where I pass my actual dbContext (after it ends up with SaveChangesAsync() if I update/create it)

The way I initiate the event args.

 RequestCompleted?.Invoke(this, new MiningResultEventArgs()
            {
                TypeOfQuery = typeOfQuery,
                DbContextConnStr = dbContextConnString,
                RequestJson = requestJson,
                AppId = appId
            });

This is the notifier class (where the error occurs)

internal class RequestNotifier : ISbMineCompletionNotify
    {
        public async void UploadRequestStatus(object source, MiningResultEventArgs miningResultArgs)
        {
            await RequestUploader(miningResultArgs);
        }

        /// <summary>
        /// API Request Results to DB
        /// </summary>
        /// <param name="miningResultEventArgs">EventArgs type of a class that contains requester info (check MiningResultEventArgs class)</param>
        /// <returns></returns>
        private async Task RequestUploader(MiningResultEventArgs miningResultEventArgs)
        {
            //ToDo - fix the following bug : Not being able to re-use the initial DbContext (that's being used in the pipeline middleware and controller area), 
            //ToDo - basically I am forced by the bug to re-create the DbContext object

            var dbOptions = new DbContextOptionsBuilder<PartnerFiscalNumberContext>();
            dbOptions.UseSqlServer(miningResultEventArgs.DbContextConnStr);

            using (var dbContext = new PartnerFiscalNumberContext(dbOptions.Options))
            {
                var appId = miningResultEventArgs.AppId;
                var requestJson = miningResultEventArgs.RequestJson;
                var typeOfQuery = miningResultEventArgs.TypeOfQuery;

                var requestType = await dbContext.RequestType.FirstAsync(x => x.Id == typeOfQuery);
                var apiUserRequester = await dbContext.ApiUsers.FirstAsync(x => x.AppId == appId);

                var apiRequest = new ApiUserRequest()
                {
                    ApiUser = apiUserRequester,
                    RequestJson = requestJson,
                    RequestType = requestType
                };

                dbContext.ApiUserRequests.Add(apiRequest);
                await dbContext.SaveChangesAsync();
            }
        }
    }

Somehow when the dbContext reaches the Event Handler CLR gets notified to dispose the dbContext object (because I'm using await?) Without recreating the object I was having huge lag when I wanted to use it.

While writing this I have an idea, I did upgrade my solution to 1.1.0 and I'm gonna try to see if it behaves similarly.

Answer 1

Concerning Why you get the error

As pointed out at the Comments by @set-fu DbContext is not thread safe.

In addition to that, since there is no explicit lifetime management of your DbContext your DbContext is going to get disposed when the garbage collector sees fit.

Judging from your context, and your mention about Request scoped DbContext I suppose you DI your DbContext in your controller's constructor. And since your DbContext is request scoped it is going to be disposed as soon as your Request is over,

BUT since you have already fired and forgot your OnRequestCompleted events there is no guarantee that your DbContext won't be disposed.

From there on , the fact that one of our methods succeeds and the other fails i think is seer "Luck". One method might be faster than the other and completes before the Garbage collector disposes the DbContext.

What you can do about this is to change the return type of your Events from

async void

To

async Task<T>

This way you can wait your RequestCompleted Task within your controller to finish and that will guarantee you that your Controller/DbContext will not get Disposed until your RequestCompleted task is finished.

Concerning Properly handling DbContexts

There are two contradicting recommendations here by microsoft and many people use DbContexts in a completely divergent manner.

1. One recommendation is to "Dispose DbContexts as soon as posible" because having a DbContext Alive occupies valuable resources like db connections etc....
2. The other states that One DbContext per request is highly reccomended

Those contradict to each other because if your Request is doing a lot of unrelated to the Db stuff , then your DbContext is kept for no reason. Thus it is waste to keep your DbContext alive while your request is just waiting for random stuff to get done...

So many people who follow rule 1 have their DbContexts inside their "Repository pattern" and create a new Instance per Database Query

        public User GetUser(int id)
        {
          User usr = null;
          using (Context db = new Context())
          {
              usr = db.Users.Find(id);
          }
          return usr;
         }

They just get their data and dispose the context ASAP. This is considered by MANY people an acceptable practice. While this has the benefits of occupying your db resources for the minimum time it clearly sacrifices all the UnitOfWork and "Caching" candy EF has to offer.

So Microsoft's recommendation about using 1 Db Context per request it's clearly based on the fact that your UnitOfWork is scoped within 1 request.

But in many cases and i believe your case also this is not true. I consider Logging a separate UnitOfWork thus having a new DbContext for your Post-Request Logging is completely acceptable (And that's the practice i also use).

An Example from my project i have 3 DbContexts in 1 Request for 3 Units Of Work.

1. Do Work
2. Write Logs
3. Send Emails to administrators.