HttpContext is lost when executing async methods

[ArnaudB88]

I want to cache the result of a method with LazyCache.
That method uses other classes which are instantiated with dependency injection. Instances of that class are created with a lifetime manager using the HTTPContext.
In short, the 'addItemFactory' method which I want to cache the result of, needs the HttpContext.

var result = appCache.GetOrAddAsync("foo", async () => {
await Task.Delay(1000);
//Here a method is executed which need the HttpContext, can be seen as:
return System.Web.HttpContext.Current.Request;
}

It seems that when the execution is started of the async method, the HTTP context is lost.
The point where it loses the context is the contructor:

LazyCache.AsyncLazy<T>.AsyncLasy(taskFactory) :
base(() => Task.Factory.StartNew(taskFactory).Unwrap())

When starting a new task (Factory.StartNew()), the context is lost.

[ArnaudB88]

My pull request was closed for the reason that the library should not depend on System.Web and because the solution was not 100% safe.
#111 (comment)

Following suggested solution does not work:

var httpContext = System.Web.HttpContext.Current;
var addItemFactory = async entry => 
{
    // here you can use httpContext instead of System.Web.HttpContext.Current
};
await AppCache.GetOrAddAsync(key, addItemFactory, absoluteExpiration);

Error: 'The name 'httpContext' does not exist in the current context'.

Possible next steps:

• Use following implementation of AsyncLazy:
  public AsyncLazy(Func<Task<T>> taskFactory) : base(taskFactory) { }
  from: https://devblogs.microsoft.com/pfxteam/asynclazyt/
  I don't really see the need for wrapping inside a new thread. Without wrapping inside a new thread, the author says it will synchronously execute the delegate if you call asyncLazy.Value.
  But since the CachingService class has GetOrAddAsync() for async delegates and GetOrAdd() for synchronous delegates, it is up to the user to make the correct choise. When calling the GetOrAddAsync() you specify in your delegate if you want to 'await' the async method or not.

• Check other implementations of AsyncLazy.
  Eg. following implementation keeps httpContext:
  https://github.com/peterekepeter/async-lazy/blob/master/AsyncLazy/AsyncLazy.cs

• Ask the MS team when this implementation will be finished:
  Add async support to System.Lazy dotnet/runtime#27510

[ArnaudB88]

@alastairtree Can you please take a look at my suggestions above?
Thanks!

[alastairtree]

Never use HttpContext object outside the context of the current http request - this is a general rule for asp.net to avoid memory leaks/odd behaviours, not just a lazy cache thing. You need to pass the values not the context into the delegate to avoid captring the context, e.g if i need the url and content type from the context i could pass them like this:

var url = System.Web.HttpContext.Current.Request.Url.ToString();
var contentType = System.Web.HttpContext.Current.Request.Headers["content-type"].Value;

var addItemFactory = async entry => 
{
    // here you CANNOT use httpContext, but you can use the url and contentType
};
await AppCache.GetOrAddAsync(key, addItemFactory, absoluteExpiration);

[ArnaudB88]

As you can see in my original post, that is not an option:

That method uses other classes which are instantiated with dependency injection. Instances of that class are created with a lifetime manager using the HTTPContext.

More specific, some database data must be cached and for instantiating the EntityFramework dbContext with dependency injection, the httpContext is needed.

I know using the HttpContext outside the context of the http request is not a good solution, therefor my 3 suggestions above. With the first two suggestions, caching remains in the same context.

[alastairtree]

Without a fully detailed example it is hard to solve your particluar issue, but as long as the method whose results you want to cache first runs in the context of the request then lazy cache will only cache the result of the method and the http context will not be needed outsode request lifetime. Maybe the issue is that you are not awaiting the cached value, as in your example you are missing the await:

// remeber to await the result
var result = await appCache.GetOrAddAsync("foo", async () => {
    //call some method that needs to run in request lifetime with lifetime dependent on htt context
   var SomeObjectNotReferencingHttpContext  = await thingDependingOnHttpContext.GetStuff();

   return SomeObjectNotReferencingHttpContext;
}

[ArnaudB88]

I added a unit test project illustrating the behavior I am trying to explain:
https://github.com/ArnaudB88/LazyCache/tree/Bug_110

See the project LazyCache.UnittestsAspNet.
The cached method should be executed in the current HttpContext since it's a database call, and database connections are set to 'one per http call' (=PerRequest lifetime manager in dependency injection)

[alastairtree]

I have looked at your test code from here.

You cannot do dependency injection resolution (_container.Resolve<IDbContext>();) inside the cached function LongRunningDatabaseCallAsync because that func does not run on the http request thread and so does not have access to the ambient http context. The only way of solving I can think of is to split up the resolution and the database call, and only cache the latter, something like so, using your test code:

            ....
            //3. Retrieve cached value needing httpContext
            var appCache = _container.Resolve<IAppCache>();
            var dbContext = _container.Resolve<IDbContext>();
            try
            {
                var cachedValue = await appCache.GetOrAddAsync("foo",
                    async () => await dbContext.GetInfoFromDatabaseAsync());

                Assert.AreEqual(DbContext.EXPECTED_VALUE, cachedValue);
            }
            catch(Exception e)
            {
                Assert.Fail(e.Message);
            }

In summary - you must refactor, the cached func will never have access to the http context, you must pass in values or references from the context you require into the cached func.

[ArnaudB88]

Thank you for looking at my test code.
Your suggested solution indeed works for simple DB calls and is a possible workaround. But when trying to cache a method which itself calls different methods (and so-on), it is impossible to do all the DI resolving before the caching starts.

I still don't think that a caching operation needs to start a new thread, and so losing it's context. I am curious about the official MS implementation (dotnet/runtime#27510). Maybe that can be used if available in the future.

[alastairtree]

It's not LazyCache starts a new thread, it is that because it is async the resumed task will be picked up by an availed thread from the pool you don't get any guarantees that the thread that resumes is the same one that initiated it. This is by design in the framework in the general sense for optimal performance and to minimise deadlocks.

The changing context is a result of using Lazy - the context that creates the lazy, is not necessarily the same context that will call .Result. this is a very deliberate design decision to achieve the required performance and locking.

Because you are caching, the cached result probably needs to be abstracted away from the http request and context anyway. E.g. two users might share the same cached list of products in an e-commerce website, so the product fetching code should not reference the user, otherwise the second user might then execute code in the context of the first user.

As general guidance, it is useful to have clear separation of concerns between web stuff associated with the http request such as headers, auth and references to the HttpContext and then back end business logic and data access.

If this absence of context is causing you issues and you cannot refactor around it, probably LazyCache is not a great fit for you, and you might be better off using MemoryCache directly, or just a static dictionary.