Correct way to use HttpContext.Current.User with async await

Question

I am working with async actions and use the HttpContext.Current.User like this

public class UserService : IUserService
{
   public ILocPrincipal Current
   {
       get { return HttpContext.Current.User as ILocPrincipal; }
   }
}
    
public class ChannelService : IDisposable
{
    // In the service layer 
    public ChannelService()
          : this(new Entities.LocDbContext(), new UserService())
      {
      }

    public ChannelService(Entities.LocDbContext locDbContext, IUserService userService)
    {
      this.LocDbContext = locDbContext;
      this.UserService = userService;
    }

    public async Task<ViewModels.DisplayChannel> FindOrDefaultAsync(long id)
    {
     var currentMemberId = this.UserService.Current.Id;
     // do some async EF request …
    }
}

// In the controller
[Authorize]
[RoutePrefix("channel")]
public class ChannelController : BaseController
{
    public ChannelController()
        : this(new ChannelService()
    {
    }

    public ChannelController(ChannelService channelService)
    {
        this.ChannelService = channelService;
    }
    
    // …

    [HttpGet, Route("~/api/channels/{id}/messages")]
    public async Task<ActionResult> GetMessages(long id)
    {
        var channel = await this.ChannelService
            .FindOrDefaultAsync(id);
 
        return PartialView("_Messages", channel);
    }

    // …
}

I have the code recently refactored, previously I had to give the user on each call to the service. Now I read this article https://www.trycatchfail.com/2014/04/25/using-httpcontext-safely-after-async-in-asp-net-mvc-applications/ and I’m not sure if my code still works. Has anyone a better approach to handle this? I don’t want to give the user on every request to the service.

Solution 1

As long as your web.config settings are correct, async/await works perfectly well with HttpContext.Current. I recommend setting httpRuntime targetFramework to 4.5 to remove all "quirks mode" behavior.

Once that is done, plain async/await will work perfectly well. You'll only run into problems if you're doing work on another thread or if your await code is incorrect.

---

First, the "other thread" problem; this is the second problem in the blog post you linked to. Code like this will of course not work correctly:

async Task FakeAsyncMethod()
{
  await Task.Run(() =>
  {
    var user = _userService.Current;
    ...
  });
}

This problem actually has nothing to do with asynchronous code; it has to do with retrieving a context variable from a (non-request) thread pool thread. The exact same problem would occur if you try to do it synchronously.

The core problem is that the asynchronous version is using fake asynchrony. This inappropriate, especially on ASP.NET. The solution is to simply remove the fake-asynchronous code and make it synchronous (or truly asynchronous, if it actually has real asynchronous work to do):

void Method()
{
  var user = _userService.Current;
  ...
}

The technique recommended in the linked blog (wrapping the HttpContext and providing it to the worker thread) is extremely dangerous. HttpContext is designed to be accessed only from one thread at a time and AFAIK is not threadsafe at all. So sharing it among different threads is asking for a world of hurt.

---

If the await code is incorrect, then it causes a similar problem. ConfigureAwait(false) is a technique commonly used in library code to notify the runtime that it doesn't need to return to a specific context. Consider this code:

async Task MyMethodAsync()
{
  await Task.Delay(1000).ConfigureAwait(false);
  var context = HttpContext.Current;
  // Note: "context" is not correct here.
  // It could be null; it could be the correct context;
  //  it could be a context for a different request.
}

In this case, the problem is obvious. ConfigureAwait(false) is telling ASP.NET that the rest of the current method does not need the context, and then it immediately accesses that context. When you start using context values in your interface implementations, though, the problem is not as obvious:

async Task MyMethodAsync()
{
  await Task.Delay(1000).ConfigureAwait(false);
  var user = _userService.Current;
}

This code is just as wrong but not as obviously wrong, since the context is hidden behind an interface.

So, the general guideline is: use ConfigureAwait(false) if you know that the method does not depend on its context (directly or indirectly); otherwise, do not use ConfigureAwait. If it's acceptable in your design to have interface implementations use the context in their implementation, then any method that calls an interface method should not use ConfigureAwait(false):

async Task MyMethodAsync()
{
  await Task.Delay(1000);
  var user = _userService.Current; // works fine
}

As long as you follow that guideline, async/await will work perfectly with HttpContext.Current.

Solution 2

Async is fine. The problem is when you post the work to a different thread. If your application is setup as 4.5+, the asynchronous callback will be posted in the original context, so you'll also have the proper HttpContext etc.

You don't want to access shared state in a different thread anyway, and with Tasks, you rarely need to handle that explicitly - just make sure you put all your inputs as arguments, and only return a response, rather than reading or writing to a shared state (e.g. HttpContext, static fields etc.)

Solution 3

There is no problem, if your ViewModels.DisplayChannel is a simple object without additional logic.

A problem may occur, if the result of your Task references to "some context objects", f.e. to HttpContext.Current. Such objects are often attached to the thread, but entire code after await may be executed in another thread.

Keep in mind, that UseTaskFriendlySynchronizationContext doesn't solve all your problems. If we are talking about ASP.NET MVC, this setting ensures that Controller.HttpContext contains correct value as before await as after. But it doesn't ensure that HttpContext.Current contains correct value, and after await it still can be null.