search

Orckestra / C1-CMS-Foundation

C1 CMS Foundation - .NET based, open source and a bundle of joy!
https://c1.orckestra.com/
Other
250 stars 109 forks source link

Replace the use of CallContext.Get|SetData, ThreadLocal and ThreadStatic with AsyncLocal #730

Open burningice2866 opened 4 years ago

burningice2866 commented 4 years ago

The use of CallContext.Set|GetData doesn't play nicely with the new era of asynchronous task-based programming were we care less of explicit Threads but think in execution flows where our code is split up in smaller chunks and executed by all number of threads at various points in time.

Using AsyncLocal we ensure that context data from our thread at the point in time we await a async method is copied to the thread that handles the continuation and will continue to flow through our tree of async method chain.

This should iron out issues around ThreadDataManager, ScopeManagers and the use of RequestLifetimeCache in especially background-threads where we don't have a HttpContext.Current to store context in and where the use of async/await is more predominant.

https://docs.microsoft.com/en-us/dotnet/api/system.threading.asynclocal-1?view=netframework-4.7.1 https://www.tabsoverspaces.com/233526-asynclocal-t-in-net-46

burningice2866 commented 4 years ago

Can i go ahead and make a pull request?

napernik commented 4 years ago

Not yet, I'm thinking of doing some testing first :)

napernik commented 4 years ago

https://github.com/Orckestra/C1-CMS-Foundation/commits/features/async-await-support

napernik commented 4 years ago

Closed by misclicking :) I got a prototype working, I'll look more into testing and merging it later.

https://github.com/Orckestra/C1-CMS-Foundation/commits/features/async-await-support

burningice2866 commented 4 years ago

What about the RequestLifetimeCache, don't it suffer the same issues of missing their state when crossing async boundaries.

napernik commented 4 years ago

Ideally I would try to find a way to preserve HttpContext.Current in those context switches as well. But yes, RequestLifetimeCache should also use the async/await friendly data.

burningice2866 commented 4 years ago

Don't forget that you have plenty of code not running in a HttpContext at all in the first place.

burningice2866 commented 4 years ago

Any updates on this... do you need help on further improving https://github.com/Orckestra/C1-CMS-Foundation/commits/features/async-await-support ?

napernik commented 4 years ago

I have a working prototype, the biggest problem I encountered - the context would get inherited in existing code during task creation where it wasn't inherited before (ThreadManager.EnsureInitialize() vs ThreadManager.Initialize()). There are a few workarounds in mind, but it is something I'll have to test later on.

It is a cool feature, as it will help up with implementing support for async functions / allows build-in performance profiler not to loose context in async/await code.

burningice2866 commented 2 years ago

Any updates on this... do you need help on further improving https://github.com/Orckestra/C1-CMS-Foundation/commits/features/async-await-support ?