11

Based on what i have read asp.net core have dropped the synchronization context. This means that the thread that executes codes after await call might not be the same one that executes codes before await

So is HttpContext still safe to use in async methods? or is it possible to get a different context after the await call?

For example in a controller action

public async Task<IActionResult> Index()
{
    var context1 = HttpContext;
    await Task.Delay(1000);
    var context2 = HttpContext;
    ....
}

could context1 be different from context2?

and the recommended way to get the context in none controller method is by dependency injecting IHttpContextAccessor

Is IHttpContextAccessor.HttpContext safe from async await pattern?

I.E. could context1 be different from context2?

public async void Foo(IHttpContextAccessor accessor)
{
    var context1 = accessor.HttpContext;
    await Task.Delay(1000);
    var context2 = accessor.HttpContext;
}
Steve
  • 11,696
  • 7
  • 43
  • 81
  • 3
    If you are in controller where you have direct access to `HttpContext` use this. If you are in a service or somewhere else where you don't have direct HttpContext use IHttpContextAccessor.HttpContext which was designed to be async-safe. – ckuri Sep 10 '19 at 07:34
  • 1
    @ckuri is there any documentation for that? Can't seem to get the logic of how asp.net core is able to restore reference to HttpContext without SynchronizationContext – Steve Sep 11 '19 at 15:50
  • 1
    [Internally](https://github.com/aspnet/HttpAbstractions/blob/master/src/Microsoft.AspNetCore.Http/HttpContextAccessor.cs) the accessor uses [AsyncLocal](https://docs.microsoft.com/en-us/dotnet/api/system.threading.asynclocal-1) which preserves information along an async call graph. The example there shows a console application which has no SynchronisationContext. The [ASP .NET Core documentation](https://docs.microsoft.com/en-us/aspnet/core/fundamentals/http-context) only mentions this implicitly at the end when they say the accessor doesn't work when you don't await your async calls. – ckuri Sep 11 '19 at 16:09

2 Answers2

18

So is HttpContext still safe to use in async methods? or is it possible to get a different context after the await call?

The whole problem with async and HttpContext and ASP.NET pre-Core was due to the fact that code usually got its HttpContext from HttpContext.Current. ASP.NET is a multithreaded server, and each await could resume on a different thread. So ASP.NET pre-Core had to have an AspNetSynchronizationContext that managed setting HttpContext.Current before the asynchronous code resumed.

The modern ASP.NET Core does not have a synchronization context. But that's fine, because it also doesn't have HttpContext.Current. The only way of getting the HttpContext instance is through a local property (e.g., HttpContext on your controller class) or dependency injection (IHttpContextAccessor).

(Pedantic note: the explanation above is a bit simplified - the ASP.NET pre-Core synchronization context did handle other things besides HttpContext.Current - but the same overall exaplanation holds for all of its other responsibilities - i.e., they are not necessary in the Core world)

So, it is not possible for the context to be different. They are the same property - the same object instance. The problem with ASP.NET pre-Core was a static property value HttpContext.Current, which has been removed in ASP.NET Core.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • 2
    you are my hero for async related questions – Steve Sep 12 '19 at 15:18
  • @Steve out of curiosity, im wondering if you could add some remarks on AsyncLocal and why that would not be suitable to implement a static HttpContext.Current in ASP.NET Core (assuming HttpContext.Current were read-only) - because at a glance it appears to still be technically feasible, even with multi-threading; perhaps there is also the "opinionated design" factor to take into consideration? – nicholas Aug 23 '21 at 01:10
  • @nicholas: `AsyncLocal` can be used to implement a static `HttpContext.Current`. I believe the primary reason it wasn't done is because of opinionated design. Quasi-globals and implicit state are generally not considered a good design, although there are exceptions to that general rule. By injecting `IHttpContextAccessor`, a component is explicitly stating its dependence on that implicit state. – Stephen Cleary Aug 23 '21 at 12:42
2

According to documentation: https://learn.microsoft.com/en-us/aspnet/core/performance/performance-best-practices?view=aspnetcore-5.0#do-not-access-httpcontext-from-multiple-threads

HttpContext is NOT thread-safe. Accessing HttpContext from multiple threads in parallel can result in undefined behavior such as hangs, crashes, and data corruption.

Do not do this: The following example makes three parallel requests and logs the incoming request path before and after the outgoing HTTP request. The request path is accessed from multiple threads, potentially in parallel.

public class AsyncBadSearchController : Controller
{       
    [HttpGet("/search")]
    public async Task<SearchResults> Get(string query)
    {
       var query1 = SearchAsync(SearchEngine.Google, query);
       var query2 = SearchAsync(SearchEngine.Bing, query);
       var query3 = SearchAsync(SearchEngine.DuckDuckGo, query);

       await Task.WhenAll(query1, query2, query3);

       var results1 = await query1;
       var results2 = await query2;
       var results3 = await query3;

       return SearchResults.Combine(results1, results2, results3);
    }       

    private async Task<SearchResults> SearchAsync(SearchEngine engine, string query)
    {
        var searchResults = _searchService.Empty();
        try
        {
            _logger.LogInformation("Starting search query from {path}.", 
                                    HttpContext.Request.Path);
            searchResults = _searchService.Search(engine, query);
            _logger.LogInformation("Finishing search query from {path}.", 
                                    HttpContext.Request.Path);
        }
        catch (Exception ex)
        {
            _logger.LogError(ex, "Failed query from {path}", 
                             HttpContext.Request.Path);
        }

        return await searchResults;
    }
}

Do this: The following example copies all data from the incoming request before making the three parallel requests.

public class AsyncGoodSearchController : Controller
{       
    [HttpGet("/search")]
    public async Task<SearchResults> Get(string query)
    {
        string path = HttpContext.Request.Path;
        var query1 = SearchAsync(SearchEngine.Google, query,
                             path);
        var query2 = SearchAsync(SearchEngine.Bing, query, path);
        var query3 = SearchAsync(SearchEngine.DuckDuckGo, query, path);

        await Task.WhenAll(query1, query2, query3);

        var results1 = await query1;
        var results2 = await query2;
        var results3 = await query3;

        return SearchResults.Combine(results1, results2, results3);
    }

       private async Task<SearchResults> SearchAsync(SearchEngine engine, string query,
                                                  string path)
       {
        var searchResults = _searchService.Empty();
        try
        {
            _logger.LogInformation("Starting search query from {path}.",
                                   path);
            searchResults = await _searchService.SearchAsync(engine, query);
            _logger.LogInformation("Finishing search query from {path}.", path);
        }
        catch (Exception ex)
        {
            _logger.LogError(ex, "Failed query from {path}", path);
        }

        return await searchResults;
    }
}
RobSiklos
  • 8,348
  • 5
  • 47
  • 77
0lukasz0
  • 3,155
  • 1
  • 24
  • 40