0

I'm using async methods with EF Core - getting the context through the built in DI

 public async Task<bool> Upvote(int userId, int articleId)
{
    var article = await context.Articles
                               .FirstOrDefaultAsync(x => x.Id == articleId);
    if (article == null)
    {
        return false;
    }

    var existing = await context.Votes
                                .FirstOrDefaultAsync(x => x.UserId == userId
                                                       && x.ArticleId == articleId);
    if (existing != null)
    ...

which is ran when someone upvotes an article.

Everything runs fine if this function gets ran one at a time (one after another).

When I hit this function several times at the same time, I get this exception:

fail: Microsoft.EntityFrameworkCore.Query.Internal.MySqlQueryCompilationContextFactory[1]
      An exception occurred in the database while iterating the results of a query.
      System.NullReferenceException: Object reference not set to an instance of an object.
         at Microsoft.EntityFrameworkCore.Query.Internal.AsyncQueryingEnumerable.AsyncEnumerator.<BufferAllAsync>d__12.MoveNext()
      --- End of stack trace from previous location where exception was thrown ---
         at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
         at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

The breakpoint hits: var existing = await context.Votes.FirstOrDefaultAsync(x => x.UserId == userId && x.ArticleId == articleId);

I'm also getting this error: Message [string]:"A second operation started on this context before a previous operation completed. Any instance members are not guaranteed to be thread safe."

What are some possible solutions?

Edit 1: This is how I'm setting up the context: In Startup.cs, I configure the context:

public void ConfigureServices(IServiceCollection services)
{
    services.AddDbContext<ArticlesContext>(options => 
        options.UseMySql(Configuration.GetConnectionString("ArticlesDB")));
...

And then I inject it in the constructor of the containing class:

private ArticlesContext context;
private ILoggingApi loggingApi;

public VoteRepository(ArticlesContext context, ILoggingApi loggingApi)
{
    this.context = context;
    this.loggingApi = loggingApi;
}

Edit 2: I'm awaiting all the way down to the controller via:

public async Task<bool> Upvote(int articleId)
{
    return await this.votesRepository.Upvote(userId, articleId);
}

And then in the controller...

[HttpPost]
[Route("upvote")]
public async Task<IActionResult> Upvote([FromBody]int articleId)
{
    var success = await votesService.Upvote(articleId);
    return new ObjectResult(success);
}

Edit 3:

I've changed my services/repos to be transient instead of singletons, but now I'm running into another issue with :

public int getCurrentUserId()
{
    if (!httpContextAccessor.HttpContext.User.HasClaim(c => c.Type == "UserId"))
    {
        return -1;
    }

It's the same async issue - but this time, HttpContext is null. I'm injecting the context accessor via

public UserService(IUserRepository userRepository, IHttpContextAccessor httpContextAccessor)
{
    this.userRepository = userRepository;
    this.httpContextAccessor = httpContextAccessor;
}

Answer: IHttpContextAccessor needs to be registered as a singleton an not transient services.AddSingleton<IHttpContextAccessor, HttpContextAccessor>();

Sherman Szeto
  • 2,325
  • 4
  • 18
  • 26
  • Can you show how you are registering your context on the container? – Klinger Dec 21 '16 at 04:03
  • This feels like you are calling async methods without awaiting them making them run in parallel? – Pawel Dec 21 '16 at 04:54
  • @Klinger I've updated the answer with edits – Sherman Szeto Dec 21 '16 at 17:15
  • @Pawel Im not sure what you mean, but I've posted the code path in edit #2 – Sherman Szeto Dec 21 '16 at 17:15
  • I'm seeing a similar issue to this - I ended up having to abandon Async for a specific method because it was the only thing that would work. All of my DI registrations are transient. I can only reproduce the issue on the Test server (which is slow and has a huge database table that it's reading from). The API call which fails makes 2 simultaneous queries and munges them together (by design). Probably the cause of the problem but no obvious solution still using async. – Keith Jackson Apr 14 '17 at 17:03

2 Answers2

4

Entity framework should be added to the services container using the Scoped lifetime, repo and services should be configured as transient so that a new instance is created and injected as needed and guarantees that instances are not reused.

EF should be scoped so that it is created on every request and disposed once the request ends.

Following those guidelines I don't see a problem in storing the injected instances on the controller constructor. The controller should be instantiated on every request and disposed, along with all scoped injected instances, at the end of the request.

This Microsoft docs page explains all this.

Klinger
  • 4,900
  • 1
  • 30
  • 35
0

Some other code use same context at same time. Check this question and answer.

If class that contains Upvote method is singleton - check that you do not store context in constructor, instead you should obtain it from IServiceProvider (HttpContext.RequestServices) for each request (scope), or pass it as parameter.

Community
  • 1
  • 1
Dmitry
  • 16,110
  • 4
  • 61
  • 73
  • Can I get around this by configuring the class to be scoped or transient instead? This class doesn't need to be a singleton, so what's the best way around this? – Sherman Szeto Dec 21 '16 at 17:20
  • Register them with "scope" should be enough. You will be able to receive same instance in all "places" during one HTTP request, and this will not mix with instances from other/parallel request. – Dmitry Dec 21 '16 at 22:14