Using HttpContext.Current in WebApi is dangerous because of async
Asked Answered
L

3

69

My question is a bit related to this: WebApi equivalent for HttpContext.Items with Dependency Injection.

We want to inject a class using HttpContext.Current in WebApi area using Ninject.

My concern is, this could be very dangerous, as in WebApi (everything?) is async.

Please correct me if I am wrong in these points, this is what I investigated so far:

  1. HttpContext.Current gets the current context by Thread (I looked into the implementation directly).

  2. Using HttpContext.Current inside of async Task is not possible, because it can run on another Thread.

  3. WebApi uses IHttpController with method Task<HttpResponseMessage> ExecuteAsync => every request is async => you cannot use HttpContext.Current inside of action method. It could even happen, more Request are executed on the same thread by coicidence.

  4. For creating controllers with injected stuff into constructors IHttpControllerActivator is used with sync method IHttpController Create. This is, where ninject creates Controller with all its dependencies.


  • If I am correct in all of these 4 points, using of HttpContext.Current inside of an action method or any layer below is very dangerous and can have unexpected results. I saw on StackOverflow lot of accepted answers suggesting exactly this. In my opinion this can work for a while, but will fail under load.

  • But when using DI to create a Controller and its dependencies, it is Ok, because this runs on one separated thread. I could get a value from the HttpContext in the constructor and it would be safe?. I wonder if each Controller is created on single thread for every request, as this could cause problem under heavy loads, where all threads from IIS could be consumed.

Just to explain why I want to inject HttpContext stuff:

  • one solution would be to get the request in controller action method and pass the needed value all the layers as param until its used somewhere deep in the code.
  • our wanted solution: all the layers between are not affected by this, and we can use the injected request somewhere deep in code (e.g. in some ConfigurationProvider which is dependent on URL)

Please give me your opinion if I am totally wrong or my suggestions are correct, as this theme seems to be very complicated.

Lorineloriner answered 25/7, 2014 at 12:47 Comment(5)
This is a very interesting question. However, as far as i know, if the code does not explicitly defer work to another thread, stuff will still be executed on the same thread. This is also true for async tasks. Although it won't block the caller thread, it'll still be executed on the caller thread. But i'm not a 100% sure.. you should give it a try ;-)Indonesian
this is exactly the problem - with await you mostly stay on the same thread, but you cannot be sure about this. thats why IMO everyone thinks it works, but will break under circumstances.. With Task.Run, it is for sure, it is another thread.Lorineloriner
why do you think that you can't be sure about it? More so than with a "sync" method? The sync method could also execute Task.Run(..) or it could spawn a new thread and in that thread try to get the HttpContext injected.. etc. So what's the difference?Indonesian
When you do Task.Run, you do it by yourself and you know what to expect, this is ok for my. My concern is,as the action-method in webapi is run asynchronously by the framework, it is allways to be considered as async with all its pitfalls. I think, it could even happen, two Requests run on the same thread (this is the purpose of await/async I think) => HttpContext.Current will return wrong value. Actually this issue already hit me once, by realising that log4net LogicalThreadContext.Properties are not working by logging in WebApi, but in MVC they do work.Lorineloriner
To answere your question why would I think it's not sure it is running on same thread? - I read many articles on the internet and it seems the community is not really sure about this, but I got the feeling that the SyncronizationContext will decide whether start new Thread or not depending on current circumstances, but practicaly this happens very rare. (still not sure 100% about this as to many different opinions are on the internet)Lorineloriner
A
116

HttpContext.Current gets the current context by Thread (I looked into the implementation directly).

It would be more correct to say that HttpContext is applied to a thread; or a thread "enters" the HttpContext.

Using HttpContext.Current inside of async Task is not possible, because it can run on another Thread.

Not at all; the default behavior of async/await will resume on an arbitrary thread, but that thread will enter the request context before resuming your async method.


The key to this is the SynchronizationContext. I have an MSDN article on the subject if you're not familiar with it. A SynchronizationContext defines a "context" for a platform, with the common ones being UI contexts (WPF, WinPhone, WinForms, etc), the thread pool context, and the ASP.NET request context.

The ASP.NET request context manages HttpContext.Current as well as a few other things such as culture and security. The UI contexts are all tightly associated with a single thread (the UI thread), but the ASP.NET request context is not tied to a specific thread. It will, however, only allow one thread in the request context at a time.

The other part of the solution is how async and await work. I have an async intro on my blog that describes their behavior. In summary, await by default will capture the current context (which is SynchronizationContext.Current unless it is null), and use that context to resume the async method. So, await is automatically capturing the ASP.NET SynchronizationContext and will resume the async method within that request context (thus preserving culture, security, and HttpContext.Current).

If you await ConfigureAwait(false), then you're explicitly telling await to not capture the context.

Note that ASP.NET did have to change its SynchronizationContext to work cleanly with async/await. You have to ensure that the application is compiled against .NET 4.5 and also explicitly targets 4.5 in its web.config; this is the default for new ASP.NET 4.5 projects but must be explicitly set if you upgraded an existing project from ASP.NET 4.0 or earlier.

You can ensure these settings are correct by executing your application against .NET 4.5 and observing SynchronizationContext.Current. If it is AspNetSynchronizationContext, then you're good; if it's LegacyAspNetSynchronizationContext, then the settings are wrong.

As long as the settings are correct (and you are using the ASP.NET 4.5 AspNetSynchronizationContext), then you can safely use HttpContext.Current after an await without worrying about it.

Alessi answered 26/7, 2014 at 16:30 Comment(23)
thanks Stephen for this great explanation! I needed couple days to study it more into detail. This was a nice surprise for me, we can safely use it. Makes our code cleaner. I see again, the async stuff is pretty complicated to fully understand.Lorineloriner
It is complicated under the covers, so that our code is cleaner and simpler. :)Alessi
@Stephen you mention that the settings are wrong if you are getting the Legacy, but that seems to be the default. If you want to get the "non-legacy" SynchronizationContext you must add this to your Web.Config: <add key="aspnet:UseTaskFriendlySynchronizationContext" value="true" />Kristin
@ScottGartner: The legacy context is only used if the server-side quirks mode is active. This is only the case if you're upgrading to .NET 4.5 and haven't turned it off yet. For a new .NET 4.5 ASP.NET project, using the new context is the default. More info here.Alessi
@Stephen, thanks for this terrific answer. I had to read it twice before I realized the problem I faced was the point you mentioned of not setting the httpRuntime to 4.5. As soon as I did that, all my problems vanished.Reflux
Just wanted to say that this whole thing with UseTaskFriendlySynchronizationContext and targetFramework is not working if you calling async method in non async way withing ASP.NET. See yourself: dotnetfiddle.net/9rPOoqEuphorbiaceous
@MichaelLogutov: That code is using ConfigureAwait(false), which is explicitly telling ASP.NET that it doesn't need the HTTP request context, and then it accesses the HTTP request context. So yes, I'd expect it to fail. The ideal solution is to use await instead of Task.Wait.Alessi
@StephenCleary: If I skip .ConfigureAwait(false) I will get deadlock.Euphorbiaceous
@MichaelLogutov: Not if you use await instead of Task.Wait.Alessi
@StephenCleary: as you can see in my example - it's a case when you can't use await because of legacy code is not async (it's usually a lot more methods upwards which can't be quickly changed into async). But the new code is async.Euphorbiaceous
@MichaelLogutov: Then you need to either change the legacy code, or use both async and sync APIs in the new code. It's not possible to not be on the context and to be on the context.Alessi
@StephenCleary: Request for comments on this related [question/answer] (https://mcmap.net/q/42248/-httpcontext-current-is-null-in-an-asynchronous-callback).Mckissick
@crokusek: There should be no need for AsyncLocal<T>; if you properly target .NET 4.5 or higher, HttpContext.Current flows across await points.Alessi
@StephenCleary: Request to elaborate stackoverflow.com/questions/38340880Mckissick
If ConfigureAwait(false) is used at any point in the call stack does that mean the thread context will be lost by a caller that intended to synchronize the context? i.e. the caller didn't specify ConfigureAwait(false)?Dobrinsky
@Mick: Each method chooses to capture and resume its own context. If a called method discards its context, that does not affect the caller. If a caller method discards its context after calling this method, that does not affect this method. If a caller method discards its context before calling this method, then this method will not have a context.Alessi
So would it be correct to say that ConfigureAwait(false) should be used on all async calls within libraries. The default, ConfigureAwait(true), should only be used at the application level where there is a GUI or ASP.NET context to synchronize with?Dobrinsky
So- correct me if I'm wrong but tl;dr - if you're using httpRuntime > 4.5, HttpContext.Current is safe (thanks to SynchronisationContext)?Already
@Shawson: Yes. It's not recommended (it's still considered a poor design), but it will work.Alessi
@StephenCleary: regarding the AsyncLocal wrapper and your comment on HttpContext.Current flowing across awaits - it doesn't seem to be the case in WCF. Does it mean the workaround can still be used there?Myrtlemyrvyn
@WiktorZychla: I believe it depends on how your WCF is hosted; if it's in a modern ASP.NET environment, it should be flowed (WCF does not provide a SynchronizationContext; it leaves that up to its host). But you can use AsyncLocal (in a .NET 4.5 or newer runtime) regardless.Alessi
@StephenCleary: thanks. Unfortunately, we currently debug a nasty scenario where ASP.NET integrated WCF drops HttpContext.Current after a couple of recursive awaits. Unfortunately, even the proposed approach involving AsyncLocal doesn't seem to help (or at least, we have some serious trouble using it). It's really shame WCF can't fit into the async model as nicely as other subsystems do. My question was asked basically to make sure WCF just doesn't follow.Myrtlemyrvyn
Yes; historically, async came out when WebAPI was becoming all the rage, so WCF did get left behind somewhat. One thing to check is the httpRuntime@targetFramework setting in your web.config; other than that, you're probably best off opening a MS support issue.Alessi
S
6

I am using a web api, which is using async/await methodology.

also using

1) HttpContext.Current.Server.MapPath
2) System.Web.HttpContext.Current.Request.ServerVariables

This was working fine for a good amount of time which broke suddenly for no code change.

Spending a lot of time by reverting back to previous old versions, found the missing key causes the issue.

< httpRuntime targetFramework="4.5.2"  /> under system.web

I am not an expert technically. But I suggest to add the key to your web config and give it a GO.

Smallpox answered 10/10, 2016 at 15:47 Comment(1)
thx for the comment. This actually corresponds with the accepted answer from Stephen. In the new framework the synchronization context was reworked and they make sure that exactly that issue I mention is working correctly as expected.Lorineloriner
L
2

I found very good article describing exactly this problem: http://byterot.blogspot.cz/2012/04/aspnet-web-api-series-part-3-async-deep.html?m=1

author investigated deeply, how the ExecuteAsync method is called in the WebApi framework and came to this conclusion:

ASP.NET Web API actions (and all the pipeline methods) will be called asynchronously only if you return a Task or Task<T>. This might sound obvious but none of the pipeline methods with Async suffix will run in their own threads. Using blanket Async could be a misnomer. [UPDATE: ASP.NET team indeed have confirmed that the Async is used to denote methods that return Task and can run asynchronously but do not have to]

What I understood from the article is, that the Action methods are called synchronously, but it is the caller decision.

I created a small test app for this purpose, something like this:

public class ValuesController : ApiController
{
    public object Get(string clientId, string specialValue)
    {
        HttpRequest staticContext = HttpContext.Current.Request;
        string staticUrl = staticContext.Url.ToString();

        HttpRequestMessage dynamicContext = Request;
        string dynamicUrl = dynamicContext.RequestUri.ToString();

        return new {one = staticUrl, two = dynamicUrl};
    }
}

and one Async version returning async Task<object>

I tried to do a little DOS attack on it with jquery and could not determine any issue until I used await Task.Delay(1).ConfigureAwait(false);, which is obvious it would fail.

What I took from the article is, that the problem is very complicated and Thread switch can happen when using async action method, so it is definetly NOT a good idea to use HttpContext.Current anywhere in the code called from the action methods. But as the controller is created synchronously, using HttpContext.Current in the constructor and as well in dependency injection is OK.

When somebody has another explanation to this problem please correct me as this problem is very complicated an I am still not 100% convinced.

diclaimer:

  • I ignore for now the problem of self-hosted Web-Api withoud IIS, where HttpContext.Current would not work probably anyway. We now rely on IIS.
Lorineloriner answered 26/7, 2014 at 9:57 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.