Using Task.Run causes "object disposed" exception in my application if using DbContext
.
The code looks like (see the whole chain):
UserController.cs
[Route("api/[controller]")]
public class UsersController : ControllerBase
{
private readonly UserService userService;
/// <summary>
/// </summary>
/// <param name="userService"></param>
public UsersController(UserService userService)
{
this.userService = userService;
}
public async Task<ActionResult> DoSomething()
{
await this.userService.MyMethod();
return this.Ok();
}
}
UserService.cs
public class UserService
{
private readonly UserRepository userRepository;
public UserService(UserRepository userRepository)
{
this.userRepository = userRepository;
}
public async Task MyMethod()
{
// some logic
Task.Run(() => MethodCallAsync());
}
void MethodCallAsync()
{
// some logic
// calls UserRepository, which uses the DbContext by DI
}
}
UserRepository:
public class UserRepository
{
private MyDbContext dbContext;
public UserRepository(MyDbContext dbContext)
{
this.dbContext = dbContext;
}
public async Task DoSomethingToo(string username)
{
var user = await this.dbContext.Users.SingleOrDefaultAsync(u => u.Username == username);
// some logic
}
}
Causes the following exception (message):
Cannot access a disposed context instance. A common cause of this error is disposing a context instance that was resolved from dependency injection and then later trying to use the same context instance elsewhere in your application. This may occur if you are calling ‘Dispose’ on the context instance, or wrapping it in a using statement. If you are using dependency injection, you should let the dependency injection container take care of disposing context instances.
How i did configure my db context and UserRepository:
Startup.cs:
public void ConfigureServices(IServiceCollection services)
{
// some logic
if (MyDbContextFactory.GetConnectionString() != null)
{
services.AddDbContext<MyDbContext>(options => options.UseMySQL(MyDbContextFactory.GetConnectionString())
.LogTo(s => System.Diagnostics.Debug.WriteLine(s)));
}
services.AddScoped(typeof(UserService));
services.AddScoped(typeof(UserRepository));
// some logic
}
public void Configure(IApplicationBuilder app, IHostingEnvironment env)
{
// some logic
using (var serviceScope = app.ApplicationServices.CreateScope())
{
var dbContext = serviceScope.ServiceProvider.GetService<MyDbContext>();
if (dbContext != null)
{
dbContext.Database.Migrate();
}
}
// some logic
}
public class MysqlEntityFrameworkDesignTimeServices : IDesignTimeServices
{
public void ConfigureDesignTimeServices(IServiceCollection serviceCollection)
{
serviceCollection.AddEntityFrameworkMySQL();
new EntityFrameworkRelationalDesignServicesBuilder(serviceCollection)
.TryAddCoreServices();
}
}
MyDbContextFactory.cs
public MyDbContext CreateDbContext(string[] args)
{
var optionsBuilder = new DbContextOptionsBuilder<MyDbContext>();
optionsBuilder.UseMySQL(GetConnectionString());
return new MyDbContext(optionsBuilder.Options);
}
If i replace Task.Run
with BackgroundJob.Enqueue
it works fine. But hangfire creates a lot (> 1k) of entries in redis within some minutes, since this method is called very often. Besides that, if it works with hangfire, it should also work with Task.Run,
2
Answers
Your problem essentially is that
Task.Run(() => MethodCallAsync());
starts a separate thread that is still relying on the class fielduserRepository
(andDbContext
inside it) – this is inherently not thread-safe and is not supposed to be by design. One way you can fix this is by passing a higher-level instance into your async method – e.g. replacinguserRepository
not just withDbContext
itself, but rather withIDbContextFactory<DbContext>
. This is about how it would look like:DbContextFactory
is supposed to be thread-safe, so this would probably work. However, if we dive deeper into your code, I would join Panagiotis Kanavos and say that you likely don’t need theUserRepository
class at all.DbContext
is supposed to be your Repository and Unit of Work already, so why not use it directly?You will have two times less code for the same thing and it will be working in a thread-safe manner.
The core problem is that you now have code that runs outside the scope of a request (i.e., request-extrinsic code). You want to return early and then update the database after the user receives the response (or more accurately, after the response is sent).
Part of tearing down that request scope is disposing any instances that are scoped to that request, including
DbContext
and friends.No.
The entire point of Hangfire is to provide reliable request-extrinsic code.
Task.Run
does not do this; it just throws work onto the thread pool. If, for example, you want to do a rolling update, then your app will be shut down once all its responses are sent, and work just tossed onto the thread pool may be lost. Hangfire prevents work loss by using a durable queue; this is usually a database but it sounds like yours is set up to use Redis (note that Redis is not durable by default, and it should be configured to be durable if you’re using it as a backend for Hangfire).Since
MethodCallAsync
accesses the database, I interpret it as work you don’t want to ever lose. If that’s correct, then your options are:await MethodCallAsync
directly without using Hangfire orTask.Run
. This increases your response time but gives you one nice guarantee: if your client receives a successful response, then it knows the work completed successfully.Note that
Task.Run
is not a valid option, because it’s work you can’t lose, andTask.Run
can lose work.It sounds like your actual problem is that Hangfire isn’t very efficient, which is true. First, I’d recommend segregating the Hangfire data from any application data; e.g., if your app also uses the same Redis instance, then move Hangfire’s data to a different Redis instance. Then scale up if necessary. If you still can’t get it efficient enough, then I recommend using your own durable queue (e.g., Azure Storage Queues / Amazon SQS / Google Cloud Tasks / Kafka / RabbitMQ if configured to be durable / etc) instead of Hangfire.