diff --git a/src/Umbraco.Abstractions/SafeCallContext.cs b/src/Umbraco.Abstractions/SafeCallContext.cs index 7a64f3f7b5..aeaf4af17e 100644 --- a/src/Umbraco.Abstractions/SafeCallContext.cs +++ b/src/Umbraco.Abstractions/SafeCallContext.cs @@ -4,6 +4,14 @@ using System.Collections.Generic; namespace Umbraco.Core { + /// + /// Provides a way to stop the data flow of a logical call context (i.e. CallContext or AsyncLocal) from within + /// a SafeCallContext and then have the original data restored to the current logical call context. + /// + /// + /// Some usages of this might be when spawning async thread or background threads in which the current logical call context will be flowed but + /// you don't want it to flow there yet you don't want to clear it either since you want the data to remain on the current thread. + /// public class SafeCallContext : IDisposable { private static readonly List> EnterFuncs = new List>(); diff --git a/src/Umbraco.Infrastructure/Scoping/CallContext.cs b/src/Umbraco.Infrastructure/Scoping/CallContext.cs index 7b256434cd..2937990eab 100644 --- a/src/Umbraco.Infrastructure/Scoping/CallContext.cs +++ b/src/Umbraco.Infrastructure/Scoping/CallContext.cs @@ -5,37 +5,35 @@ using System.Threading; namespace Umbraco.Core.Scoping { /// - /// Provides a way to set contextual data that flows with the call and - /// async context of a test or invocation. + /// Represents ambient data that is local to a given asynchronous control flow, such as an asynchronous method. /// - public static class CallContext + /// + /// This is just a simple wrapper around + /// + public static class CallContext { - private static readonly ConcurrentDictionary _state = new ConcurrentDictionary(); + private static ConcurrentDictionary> _state = new ConcurrentDictionary>(); /// /// Stores a given object and associates it with the specified name. /// /// The name with which to associate the new item in the call context. /// The object to store in the call context. - public static void SetData(string name, Guid? data) - { - _state[name + Thread.CurrentThread.ManagedThreadId] = data; - } - + public static void SetData(string name, T data) => _state.GetOrAdd(name, _ => new AsyncLocal()).Value = data; /// - /// Retrieves an object with the specified name from the . + /// Retrieves an object with the specified name from the . /// + /// The type of the data being retrieved. Must match the type used when the was set via . /// The name of the item in the call context. - /// The object in the call context associated with the specified name, or if not found. - public static Guid? GetData(string name) - { - return _state.TryGetValue(name + Thread.CurrentThread.ManagedThreadId, out var data) ? data : null; - } + /// The object in the call context associated with the specified name, or a default value for if none is found. + public static T GetData(string name) => _state.TryGetValue(name, out var data) ? data.Value : default; + + // NOTE: If you have used the old CallContext in the past you might be thinking you need to clean this up but that is not the case. + // With CallContext you had to call FreeNamedDataSlot to prevent leaks but with AsyncLocal this is not the case, there is no way to clean this up. + // The above dictionary is sort of a trick because sure, there is always going to be a string key that will exist in the collection but the values + // themselves are managed per ExecutionContext so they don't build up. + // There's an SO article relating to this here https://stackoverflow.com/questions/36511243/safety-of-asynclocal-in-asp-net-core - public static bool RemoveData(string name) - { - return _state.TryRemove(name+ Thread.CurrentThread.ManagedThreadId, out _); - } } } diff --git a/src/Umbraco.Infrastructure/Scoping/IScopeProvider.cs b/src/Umbraco.Infrastructure/Scoping/IScopeProvider.cs index 6c9eb63ba0..4a7ccae481 100644 --- a/src/Umbraco.Infrastructure/Scoping/IScopeProvider.cs +++ b/src/Umbraco.Infrastructure/Scoping/IScopeProvider.cs @@ -88,7 +88,7 @@ namespace Umbraco.Core.Scoping ISqlContext SqlContext { get; } #if DEBUG_SCOPES - Dictionary CallContextObjects { get; } + IEnumerable ScopeInfos { get; } ScopeInfo GetScopeInfo(IScope scope); #endif diff --git a/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs index ef3afb2dac..0dba73b55b 100644 --- a/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs +++ b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs @@ -49,23 +49,23 @@ namespace Umbraco.Core.Scoping SafeCallContext.Register( () => { - var scope = GetCallContextObject(ScopeItemKey); - var context = GetCallContextObject(ContextItemKey); - SetCallContextObject(ScopeItemKey, null); - SetCallContextObject(ContextItemKey, null); + var scope = GetCallContextObject(ScopeItemKey); + var context = GetCallContextObject(ContextItemKey); + SetCallContextObject(ScopeItemKey, null); + SetCallContextObject(ContextItemKey, null); return Tuple.Create(scope, context); }, o => { // cannot re-attached over leaked scope/context - if (GetCallContextObject(ScopeItemKey) != null) + if (GetCallContextObject(ScopeItemKey) != null) throw new Exception("Found leaked scope when restoring call context."); - if (GetCallContextObject(ContextItemKey) != null) + if (GetCallContextObject(ContextItemKey) != null) throw new Exception("Found leaked context when restoring call context."); - var t = (Tuple) o; - SetCallContextObject(ScopeItemKey, t.Item1); - SetCallContextObject(ContextItemKey, t.Item2); + var t = (Tuple) o; + SetCallContextObject(ScopeItemKey, t.Item1); + SetCallContextObject(ContextItemKey, t.Item2); }); } @@ -75,65 +75,16 @@ namespace Umbraco.Core.Scoping #region Context - // objects that go into the logical call context better be serializable else they'll eventually - // cause issues whenever some cross-AppDomain code executes - could be due to ReSharper running - // tests, any other things (see https://msdn.microsoft.com/en-us/library/dn458353(v=vs.110).aspx), - // but we don't want to make all of our objects serializable since they are *not* meant to be - // used in cross-AppDomain scenario anyways. - // in addition, whatever goes into the logical call context is serialized back and forth any - // time cross-AppDomain code executes, so if we put an "object" there, we'll can *another* - // "object" instance - and so we cannot use a random object as a key. - // so what we do is: we register a guid in the call context, and we keep a table mapping those - // guids to the actual objects. the guid serializes back and forth without causing any issue, - // and we can retrieve the actual objects from the table. - // only issue: how are we supposed to clear the table? we can't, really. objects should take - // care of de-registering themselves from context. - - private static readonly object StaticCallContextObjectsLock = new object(); - private static readonly Dictionary StaticCallContextObjects - = new Dictionary(); - -#if DEBUG_SCOPES - public Dictionary CallContextObjects - { - get - { - lock (StaticCallContextObjectsLock) - { - // capture in a dictionary - return StaticCallContextObjects.ToDictionary(x => x.Key, x => x.Value); - } - } - } -#endif - private static T GetCallContextObject(string key) - where T : class + where T : class, IInstanceIdentifiable { - var objectKey = CallContext.GetData(key); - if (objectKey is null) return null; - - lock (StaticCallContextObjectsLock) - { - if (StaticCallContextObjects.TryGetValue(objectKey.Value, out object callContextObject)) - { -#if DEBUG_SCOPES - Current.Logger.Debug("Got " + typeof(T).Name + " Object " + objectKey.ToString("N").Substring(0, 8)); - //_logger.Debug("At:\r\n" + Head(Environment.StackTrace, 24)); -#endif - return (T)callContextObject; - } - - // hard to inject into a static method :( - Current.Logger.Warn("Missed {TypeName} Object {ObjectKey}", typeof(T).Name, objectKey.Value.ToString("N").Substring(0, 8)); -#if DEBUG_SCOPES - //Current.Logger.Debug("At:\r\n" + Head(Environment.StackTrace, 24)); -#endif - return null; - } + var obj = CallContext.GetData(key); + if (obj == default(T)) return null; + return obj; } - private static void SetCallContextObject(string key, IInstanceIdentifiable value) + private static void SetCallContextObject(string key, T value) + where T: class, IInstanceIdentifiable { #if DEBUG_SCOPES // manage the 'context' that contains the scope (null, "http" or "call") @@ -141,14 +92,8 @@ namespace Umbraco.Core.Scoping if (key == ScopeItemKey) { // first, null-register the existing value - var ambientKey = CallContext.GetData(ScopeItemKey).AsGuid(); - object o = null; - lock (StaticCallContextObjectsLock) - { - if (ambientKey != default(Guid)) - StaticCallContextObjects.TryGetValue(ambientKey, out o); - } - var ambientScope = o as IScope; + var ambientScope = CallContext.GetData(ScopeItemKey); + if (ambientScope != null) RegisterContext(ambientScope, null); // then register the new value var scope = value as IScope; @@ -157,33 +102,18 @@ namespace Umbraco.Core.Scoping #endif if (value == null) { - var objectKey = CallContext.GetData(key); - CallContext.RemoveData(key); - if (objectKey is null) return; - lock (StaticCallContextObjectsLock) - { -#if DEBUG_SCOPES - Current.Logger.Debug("Remove Object " + objectKey.ToString("N").Substring(0, 8)); - //Current.Logger.Debug("At:\r\n" + Head(Environment.StackTrace, 24)); -#endif - StaticCallContextObjects.Remove(objectKey.Value); - } + var obj = CallContext.GetData(key); + CallContext.SetData(key, default); // aka remove + if (obj == null) return; } else { - // note - we are *not* detecting an already-existing value - // because our code in this class *always* sets to null before - // setting to a real value - var objectKey = value.InstanceId; - lock (StaticCallContextObjectsLock) - { + #if DEBUG_SCOPES - Current.Logger.Debug("AddObject " + objectKey.ToString("N").Substring(0, 8)); - //Current.Logger.Debug("At:\r\n" + Head(Environment.StackTrace, 24)); + Current.Logger.Debug("AddObject " + value.InstanceId.ToString("N").Substring(0, 8)); #endif - StaticCallContextObjects.Add(objectKey, value); - } - CallContext.SetData(key, objectKey); + + CallContext.SetData(key, value); } } @@ -245,12 +175,12 @@ namespace Umbraco.Core.Scoping { // clear both SetHttpContextObject(ContextItemKey, null, false); - SetCallContextObject(ContextItemKey, null); + SetCallContextObject(ContextItemKey, null); if (value == null) return; // set http/call context if (SetHttpContextObject(ContextItemKey, value, false) == false) - SetCallContextObject(ContextItemKey, value); + SetCallContextObject(ContextItemKey, value); } } @@ -270,21 +200,22 @@ namespace Umbraco.Core.Scoping public Scope AmbientScope { // try http context, fallback onto call context - get => GetHttpContextObject(ScopeItemKey, false) - ?? GetCallContextObject(ScopeItemKey); + // we are casting here because we know its a concrete type + get => (Scope)GetHttpContextObject(ScopeItemKey, false) + ?? (Scope)GetCallContextObject(ScopeItemKey); set { // clear both SetHttpContextObject(ScopeItemKey, null, false); SetHttpContextObject(ScopeRefItemKey, null, false); - SetCallContextObject(ScopeItemKey, null); + SetCallContextObject(ScopeItemKey, null); if (value == null) return; // set http/call context if (value.CallContext == false && SetHttpContextObject(ScopeItemKey, value, false)) SetHttpContextObject(ScopeRefItemKey, _scopeReference); else - SetCallContextObject(ScopeItemKey, value); + SetCallContextObject(ScopeItemKey, value); } } @@ -295,9 +226,9 @@ namespace Umbraco.Core.Scoping // clear all SetHttpContextObject(ScopeItemKey, null, false); SetHttpContextObject(ScopeRefItemKey, null, false); - SetCallContextObject(ScopeItemKey, null); + SetCallContextObject(ScopeItemKey, null); SetHttpContextObject(ContextItemKey, null, false); - SetCallContextObject(ContextItemKey, null); + SetCallContextObject(ContextItemKey, null); if (scope == null) { if (context != null) @@ -312,8 +243,8 @@ namespace Umbraco.Core.Scoping } else { - SetCallContextObject(ScopeItemKey, scope); - SetCallContextObject(ContextItemKey, context); + SetCallContextObject(ScopeItemKey, scope); + SetCallContextObject(ContextItemKey, context); } } diff --git a/src/Umbraco.Tests/CoreThings/CallContextTests.cs b/src/Umbraco.Tests/CoreThings/CallContextTests.cs index e01534c8d9..b97a87542c 100644 --- a/src/Umbraco.Tests/CoreThings/CallContextTests.cs +++ b/src/Umbraco.Tests/CoreThings/CallContextTests.cs @@ -1,6 +1,7 @@ -using System.Runtime.Remoting.Messaging; -using NUnit.Framework; +using NUnit.Framework; +using System; using Umbraco.Core; +using Umbraco.Core.Scoping; namespace Umbraco.Tests.CoreThings { @@ -13,10 +14,10 @@ namespace Umbraco.Tests.CoreThings { SafeCallContext.Register(() => { - CallContext.FreeNamedDataSlot("test1"); - CallContext.FreeNamedDataSlot("test2"); + CallContext.SetData("test1", null); + CallContext.SetData("test2", null); return null; - }, o => {}); + }, o => { }); } [OneTimeSetUp] @@ -44,10 +45,10 @@ namespace Umbraco.Tests.CoreThings [Test] public void Test1() { - CallContext.LogicalSetData("test1", "test1"); - Assert.IsNull(CallContext.LogicalGetData("test2")); + CallContext.SetData("test1", "test1"); + Assert.IsNull(CallContext.GetData("test2")); - CallContext.LogicalSetData("test3b", "test3b"); + CallContext.SetData("test3b", "test3b"); if (_first) { @@ -55,21 +56,21 @@ namespace Umbraco.Tests.CoreThings } else { - Assert.IsNotNull(CallContext.LogicalGetData("test3a")); // leak! + Assert.IsNotNull(CallContext.GetData("test3a")); // leak! } } [Test] public void Test2() { - CallContext.LogicalSetData("test2", "test2"); - Assert.IsNull(CallContext.LogicalGetData("test1")); + CallContext.SetData("test2", "test2"); + Assert.IsNull(CallContext.GetData("test1")); } [Test] public void Test3() { - CallContext.LogicalSetData("test3a", "test3a"); + CallContext.SetData("test3a", "test3a"); if (_first) { @@ -77,7 +78,7 @@ namespace Umbraco.Tests.CoreThings } else { - Assert.IsNotNull(CallContext.LogicalGetData("test3b")); // leak! + Assert.IsNotNull(CallContext.GetData("test3b")); // leak! } } } diff --git a/src/Umbraco.Tests/Migrations/MigrationTests.cs b/src/Umbraco.Tests/Migrations/MigrationTests.cs index 6b2d21e4a5..bfadd45b0d 100644 --- a/src/Umbraco.Tests/Migrations/MigrationTests.cs +++ b/src/Umbraco.Tests/Migrations/MigrationTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Data; using Moq; using NUnit.Framework; @@ -45,8 +46,18 @@ namespace Umbraco.Tests.Migrations throw new NotImplementedException(); } + + public IScopeContext Context { get; set; } public ISqlContext SqlContext { get; set; } + +#if DEBUG_SCOPES + public ScopeInfo GetScopeInfo(IScope scope) + { + throw new NotImplementedException(); + } + public IEnumerable ScopeInfos => throw new NotImplementedException(); +#endif } [Test] diff --git a/src/Umbraco.Tests/Persistence/LocksTests.cs b/src/Umbraco.Tests/Persistence/LocksTests.cs index afcd481f9f..d4e3d23a70 100644 --- a/src/Umbraco.Tests/Persistence/LocksTests.cs +++ b/src/Umbraco.Tests/Persistence/LocksTests.cs @@ -13,7 +13,7 @@ namespace Umbraco.Tests.Persistence { [TestFixture] [Timeout(60000)] - [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest, Logger = UmbracoTestOptions.Logger.Serilog)] + [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest, Logger = UmbracoTestOptions.Logger.Console)] public class LocksTests : TestWithDatabaseBase { protected override void Initialize() diff --git a/src/Umbraco.Tests/Scoping/ScopeTests.cs b/src/Umbraco.Tests/Scoping/ScopeTests.cs index d1f77d4ae0..eb4a01d06b 100644 --- a/src/Umbraco.Tests/Scoping/ScopeTests.cs +++ b/src/Umbraco.Tests/Scoping/ScopeTests.cs @@ -1,17 +1,10 @@ using System; -using System.Collections; -using System.Runtime.Remoting.Messaging; -using System.Threading; using NUnit.Framework; using Umbraco.Core; using Umbraco.Core.Persistence; using Umbraco.Core.Scoping; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.Testing; -using CallContext = Umbraco.Core.Scoping.CallContext; - -//using CallContext = Umbraco.Core.Scoping.CallContext; - namespace Umbraco.Tests.Scoping { @@ -126,8 +119,8 @@ namespace Umbraco.Tests.Scoping Assert.AreSame(scope, ((Scope) nested).ParentScope); // it's moved over to call context - var callContextKey = CallContext.GetData(ScopeProvider.ScopeItemKey).AsGuid(); - Assert.AreNotEqual(Guid.Empty, callContextKey); + var callContextScope = CallContext.GetData(ScopeProvider.ScopeItemKey); + Assert.IsNotNull(callContextScope); // only if Core.DEBUG_SCOPES are defined //var ccnested = scopeProvider.CallContextObjects[callContextKey];