Ciwan Ciwan - 11 months ago 76
C# Question

await on UserManager causes the error A second operation started on this context before a previous operation completed

BACKGROUND

I am working on a .Net Core API to drive a lyrics application. Users can sign up, submit artists & lyrics, and earn kudos / XP points in the process. Basically a community-driven lyrics website.

CODE

Here's my ArtistController class:

[Route("api/artists")]
public class ArtistsController : Controller
{
private readonly IPermissionsService _permissionsService;
private readonly IArtistsService _artistsService;

public ArtistsController(IArtistsService artistsService, IPermissionsService permissionsService)
{
_permissionsService = permissionsService ?? throw new ArgumentNullException(nameof(permissionsService));
_artistsService = artistsService ?? throw new ArgumentNullException(nameof(artistsService));
}

[HttpGet("{slug}")]
[HttpGet("{slug}/lyrics", Name = "GetArtist")]
public async Task<IActionResult> GetArtist(string slug)
{
if (!_artistsService.ArtistExists(slug)) return NotFound();
var permissions = await _permissionsService.GetPermissions(HttpContext);
var artist = _artistsService.GetArtistBySlug(slug, permissions.UserId, permissions.IsAdministrator);
if (artist == null) return NotFound();
return Ok(artist);
}

// other methods omitted
}


In the spirit of testability, I created an
IPermissionsService
, this way when I come to Unit Test the controller, I can easily do so without worrying about
HttpContext
and
User
.

Here is the code for
PermissionsService
class:

public class PermissionsService : IPermissionsService
{
private string _userId;
private bool _isAdministrator;
private HttpContext _httpContext;
private readonly UserManager<BbUser> _userManager;

public PermissionsService(UserManager<BbUser> userManager)
{
_userManager = userManager;
}

public async Task<Permissions> GetPermissions(HttpContext httpContext)
{
_httpContext = httpContext;
PopulateUserIdAndIsAdminFlag();
var permissions = new Permissions
{
UserId = _userId,
IsAdministrator = _isAdministrator
};

return await Task.Run(() => permissions);
}

private async void PopulateUserIdAndIsAdminFlag()
{
if (!IsAuthenticated()) return;
var username = _httpContext.User.FindFirstValue(ClaimTypes.NameIdentifier);
var user = await _userManager.FindByNameAsync(username);
var roles = await _userManager.GetRolesAsync(user);
_userId = user.Id;
_isAdministrator = roles.Contains("Admin");
}

private bool IsAuthenticated()
{
return _httpContext.User.Identity.IsAuthenticated;
}
}


PROBLEM

When I run the API and attempt to call that end-point. I get the following error:


A second operation started on this context before a previous operation completed. Any instance members are not guaranteed to be thread safe.


The message is pretty clear, but I am not sure how to overcome this error. Before moving that logic out to the
PermissionsService
I was not getting the error and all was working fine!

Any help and clarification would be greatly appreciated.

Answer Source

Methods that do async work should return Task and not void unless they are event handlers. This will allow the resulting Task to be awaited. Because PopulateUserIdAndIsAdminFlag is not awaited you are making calls to the same DbContext instance simultaneously across threads. If you follow the call stack:

  1. Code enters GetPermissions
  2. You start work in PopulateUserIdAndIsAdminFlag but do not wait for it to be completed
  3. Code immediately returns from GetPermissions (code in PopulateUserIdAndIsAdminFlag is also still executing)
  4. Code continues and calls method on _artistsService

This can result in the DbContext being called by multiple threads at the same time which causes your exception.

Fix the code so that the result of PopulateUserIdAndIsAdminFlag is awaited.

  • Change your code to await the PopulateUserIdAndIsAdminFlag method so it returns type Task
  • await the result of PopulateUserIdAndIsAdminFlag
  • Wrapping the result in a Task is no longer necessary at the end of GetPermissions
  • I also recommend renaming it and adding the suffix Async as this is considered proper naming convention for methods that return type Task. That would result in methods named GetPermissionsAsync and PopulateUserIdAndIsAdminFlagAsync

Changed code:

public async Task<Permissions> GetPermissions(HttpContext httpContext)
{
    _httpContext = httpContext;
    // await result
    await PopulateUserIdAndIsAdminFlag();
    var permissions = new Permissions
    {
      UserId = _userId,
      IsAdministrator = _isAdministrator
    };

    // wrapping the result in Task is no longer necessary 
    return permissions;
}

// change void to Task
private async Task PopulateUserIdAndIsAdminFlag()
{
    if (!IsAuthenticated()) return;
    var username = _httpContext.User.FindFirstValue(ClaimTypes.NameIdentifier);
    var user = await _userManager.FindByNameAsync(username);
    var roles = await _userManager.GetRolesAsync(user);
    _userId = user.Id;
    _isAdministrator = roles.Contains("Admin");
}
Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download