From bcc0fa391a3be747ce131eeb8b8703c030b1bb87 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 20 Dec 2019 12:50:33 +1100 Subject: [PATCH 1/4] adds notes, changes to use AsyncLocal then will look at full refactor --- src/Umbraco.Abstractions/SafeCallContext.cs | 5 ++++ .../Scoping/CallContext.cs | 30 ++++++++----------- .../Scoping/ScopeProvider.cs | 22 ++++++++------ src/Umbraco.Tests/Scoping/ScopeTests.cs | 7 ++--- 4 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/Umbraco.Abstractions/SafeCallContext.cs b/src/Umbraco.Abstractions/SafeCallContext.cs index 7a64f3f7b5..ee1416e833 100644 --- a/src/Umbraco.Abstractions/SafeCallContext.cs +++ b/src/Umbraco.Abstractions/SafeCallContext.cs @@ -4,6 +4,11 @@ using System.Collections.Generic; namespace Umbraco.Core { + // TODO: This may no longer be necessary but I'm unsure. This is based on the premise of using the old CallContext which + // requires all objects to be serializable. Stephane wrote a blog post here https://www.zpqrtbnk.net/posts/putting-things-in-contexts/ + // about this. But now we are not using the CallContext since it doesn't exist and instead using AsyncLocal which probably + // doesn't have this problem... but that would require some testing. For now we'll leave this class here until we + // can acquire/discover more info. 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..6a5354fb04 100644 --- a/src/Umbraco.Infrastructure/Scoping/CallContext.cs +++ b/src/Umbraco.Infrastructure/Scoping/CallContext.cs @@ -8,34 +8,30 @@ namespace Umbraco.Core.Scoping /// Provides a way to set contextual data that flows with the call and /// async context of a test or invocation. /// - public static class CallContext + public static class CallContext { - private static readonly ConcurrentDictionary _state = new ConcurrentDictionary(); + 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; - public static bool RemoveData(string name) - { - return _state.TryRemove(name+ Thread.CurrentThread.ManagedThreadId, out _); - } + /// + /// Clears the state from for the given name. + /// + /// + /// + public static bool RemoveData(string name) => _state.TryRemove(name, out _); } } diff --git a/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs index ef3afb2dac..7ae8c63905 100644 --- a/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs +++ b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs @@ -75,6 +75,9 @@ namespace Umbraco.Core.Scoping #region Context + // TODO: I don't think this whole thing is necessary anymore since we are using AsyncLocal which + // I don't believe has the same odd requirements as the old CallContext! Also see SafeCallContext + // 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), @@ -88,6 +91,7 @@ namespace Umbraco.Core.Scoping // 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. + // see https://www.zpqrtbnk.net/posts/putting-things-in-contexts/ private static readonly object StaticCallContextObjectsLock = new object(); private static readonly Dictionary StaticCallContextObjects @@ -110,12 +114,12 @@ namespace Umbraco.Core.Scoping private static T GetCallContextObject(string key) where T : class { - var objectKey = CallContext.GetData(key); - if (objectKey is null) return null; + var objectKey = CallContext.GetData(key); + if (objectKey == Guid.Empty) return null; lock (StaticCallContextObjectsLock) { - if (StaticCallContextObjects.TryGetValue(objectKey.Value, out object callContextObject)) + if (StaticCallContextObjects.TryGetValue(objectKey, out object callContextObject)) { #if DEBUG_SCOPES Current.Logger.Debug("Got " + typeof(T).Name + " Object " + objectKey.ToString("N").Substring(0, 8)); @@ -125,7 +129,7 @@ namespace Umbraco.Core.Scoping } // hard to inject into a static method :( - Current.Logger.Warn("Missed {TypeName} Object {ObjectKey}", typeof(T).Name, objectKey.Value.ToString("N").Substring(0, 8)); + Current.Logger.Warn("Missed {TypeName} Object {ObjectKey}", typeof(T).Name, objectKey.ToString("N").Substring(0, 8)); #if DEBUG_SCOPES //Current.Logger.Debug("At:\r\n" + Head(Environment.StackTrace, 24)); #endif @@ -157,16 +161,16 @@ namespace Umbraco.Core.Scoping #endif if (value == null) { - var objectKey = CallContext.GetData(key); - CallContext.RemoveData(key); - if (objectKey is null) return; + var objectKey = CallContext.GetData(key); + CallContext.RemoveData(key); + if (objectKey == Guid.Empty) 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); + StaticCallContextObjects.Remove(objectKey); } } else @@ -183,7 +187,7 @@ namespace Umbraco.Core.Scoping #endif StaticCallContextObjects.Add(objectKey, value); } - CallContext.SetData(key, objectKey); + CallContext.SetData(key, objectKey); } } diff --git a/src/Umbraco.Tests/Scoping/ScopeTests.cs b/src/Umbraco.Tests/Scoping/ScopeTests.cs index d1f77d4ae0..0f35554472 100644 --- a/src/Umbraco.Tests/Scoping/ScopeTests.cs +++ b/src/Umbraco.Tests/Scoping/ScopeTests.cs @@ -8,10 +8,7 @@ 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; - +using CallContext = Umbraco.Core.Scoping.CallContext; namespace Umbraco.Tests.Scoping { @@ -126,7 +123,7 @@ namespace Umbraco.Tests.Scoping Assert.AreSame(scope, ((Scope) nested).ParentScope); // it's moved over to call context - var callContextKey = CallContext.GetData(ScopeProvider.ScopeItemKey).AsGuid(); + var callContextKey = CallContext.GetData(ScopeProvider.ScopeItemKey); Assert.AreNotEqual(Guid.Empty, callContextKey); // only if Core.DEBUG_SCOPES are defined From ef5a6a1db6db10f00d1f54fdf6080f859772fd26 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 20 Dec 2019 15:12:54 +1100 Subject: [PATCH 2/4] Adds notes changes some logic --- .../Scoping/CallContext.cs | 20 +++++++------- .../Scoping/ScopeProvider.cs | 6 ++--- .../CoreThings/CallContextTests.cs | 27 ++++++++++--------- .../Migrations/MigrationTests.cs | 12 +++++++++ src/Umbraco.Tests/Persistence/LocksTests.cs | 2 +- src/Umbraco.Tests/Scoping/ScopeTests.cs | 6 +---- 6 files changed, 42 insertions(+), 31 deletions(-) diff --git a/src/Umbraco.Infrastructure/Scoping/CallContext.cs b/src/Umbraco.Infrastructure/Scoping/CallContext.cs index 6a5354fb04..2937990eab 100644 --- a/src/Umbraco.Infrastructure/Scoping/CallContext.cs +++ b/src/Umbraco.Infrastructure/Scoping/CallContext.cs @@ -5,12 +5,14 @@ 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. /// + /// + /// This is just a simple wrapper around + /// public static class CallContext { - static ConcurrentDictionary> _state = new ConcurrentDictionary>(); + private static ConcurrentDictionary> _state = new ConcurrentDictionary>(); /// /// Stores a given object and associates it with the specified name. @@ -27,11 +29,11 @@ namespace Umbraco.Core.Scoping /// 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; - /// - /// Clears the state from for the given name. - /// - /// - /// - public static bool RemoveData(string name) => _state.TryRemove(name, out _); + // 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 + } } diff --git a/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs index 7ae8c63905..10130e820e 100644 --- a/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs +++ b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs @@ -145,11 +145,11 @@ namespace Umbraco.Core.Scoping if (key == ScopeItemKey) { // first, null-register the existing value - var ambientKey = CallContext.GetData(ScopeItemKey).AsGuid(); + var ambientKey = CallContext.GetData(ScopeItemKey); object o = null; lock (StaticCallContextObjectsLock) { - if (ambientKey != default(Guid)) + if (ambientKey != default) StaticCallContextObjects.TryGetValue(ambientKey, out o); } var ambientScope = o as IScope; @@ -162,7 +162,7 @@ namespace Umbraco.Core.Scoping if (value == null) { var objectKey = CallContext.GetData(key); - CallContext.RemoveData(key); + CallContext.SetData(key, default); // aka remove if (objectKey == Guid.Empty) return; lock (StaticCallContextObjectsLock) { 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..64318bc193 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,19 @@ 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 Dictionary CallContextObjects => 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 0f35554472..1e1f202809 100644 --- a/src/Umbraco.Tests/Scoping/ScopeTests.cs +++ b/src/Umbraco.Tests/Scoping/ScopeTests.cs @@ -1,14 +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; namespace Umbraco.Tests.Scoping { @@ -123,7 +119,7 @@ namespace Umbraco.Tests.Scoping Assert.AreSame(scope, ((Scope) nested).ParentScope); // it's moved over to call context - var callContextKey = CallContext.GetData(ScopeProvider.ScopeItemKey); + var callContextKey = CallContext.GetData(ScopeProvider.ScopeItemKey); Assert.AreNotEqual(Guid.Empty, callContextKey); // only if Core.DEBUG_SCOPES are defined From 95d3a87bb2638ece9841b5f5693ba116d2ecf4cc Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 20 Dec 2019 16:19:29 +1100 Subject: [PATCH 3/4] Simplifies ScopeProvider now that we are using AsyncLocal we don't need to worry about the weird constraints of CallContext which was being worked around with the static object dictionary --- .../Scoping/IScopeProvider.cs | 2 +- .../Scoping/ScopeProvider.cs | 125 +++++------------- .../Migrations/MigrationTests.cs | 1 - 3 files changed, 36 insertions(+), 92 deletions(-) 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 10130e820e..d4bd224eae 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); }); } @@ -93,51 +93,16 @@ namespace Umbraco.Core.Scoping // care of de-registering themselves from context. // see https://www.zpqrtbnk.net/posts/putting-things-in-contexts/ - 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 == Guid.Empty) return null; - - lock (StaticCallContextObjectsLock) - { - if (StaticCallContextObjects.TryGetValue(objectKey, 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.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") @@ -145,14 +110,8 @@ namespace Umbraco.Core.Scoping if (key == ScopeItemKey) { // first, null-register the existing value - var ambientKey = CallContext.GetData(ScopeItemKey); - object o = null; - lock (StaticCallContextObjectsLock) - { - if (ambientKey != default) - 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; @@ -161,33 +120,18 @@ namespace Umbraco.Core.Scoping #endif if (value == null) { - var objectKey = CallContext.GetData(key); - CallContext.SetData(key, default); // aka remove - if (objectKey == Guid.Empty) 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); - } + 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); } } @@ -249,12 +193,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); } } @@ -274,21 +218,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); } } @@ -299,9 +244,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) @@ -316,8 +261,8 @@ namespace Umbraco.Core.Scoping } else { - SetCallContextObject(ScopeItemKey, scope); - SetCallContextObject(ContextItemKey, context); + SetCallContextObject(ScopeItemKey, scope); + SetCallContextObject(ContextItemKey, context); } } diff --git a/src/Umbraco.Tests/Migrations/MigrationTests.cs b/src/Umbraco.Tests/Migrations/MigrationTests.cs index 64318bc193..bfadd45b0d 100644 --- a/src/Umbraco.Tests/Migrations/MigrationTests.cs +++ b/src/Umbraco.Tests/Migrations/MigrationTests.cs @@ -56,7 +56,6 @@ namespace Umbraco.Tests.Migrations { throw new NotImplementedException(); } - public Dictionary CallContextObjects => throw new NotImplementedException(); public IEnumerable ScopeInfos => throw new NotImplementedException(); #endif } From 9dd258320a12695533875541ba3e9fc8cb05f359 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 23 Dec 2019 16:04:17 +1100 Subject: [PATCH 4/4] Adds notes, fixes tests --- src/Umbraco.Abstractions/SafeCallContext.cs | 13 ++++++++----- .../Scoping/ScopeProvider.cs | 18 ------------------ src/Umbraco.Tests/Scoping/ScopeTests.cs | 4 ++-- 3 files changed, 10 insertions(+), 25 deletions(-) diff --git a/src/Umbraco.Abstractions/SafeCallContext.cs b/src/Umbraco.Abstractions/SafeCallContext.cs index ee1416e833..aeaf4af17e 100644 --- a/src/Umbraco.Abstractions/SafeCallContext.cs +++ b/src/Umbraco.Abstractions/SafeCallContext.cs @@ -4,11 +4,14 @@ using System.Collections.Generic; namespace Umbraco.Core { - // TODO: This may no longer be necessary but I'm unsure. This is based on the premise of using the old CallContext which - // requires all objects to be serializable. Stephane wrote a blog post here https://www.zpqrtbnk.net/posts/putting-things-in-contexts/ - // about this. But now we are not using the CallContext since it doesn't exist and instead using AsyncLocal which probably - // doesn't have this problem... but that would require some testing. For now we'll leave this class here until we - // can acquire/discover more info. + /// + /// 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/ScopeProvider.cs b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs index d4bd224eae..0dba73b55b 100644 --- a/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs +++ b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs @@ -75,24 +75,6 @@ namespace Umbraco.Core.Scoping #region Context - // TODO: I don't think this whole thing is necessary anymore since we are using AsyncLocal which - // I don't believe has the same odd requirements as the old CallContext! Also see SafeCallContext - - // 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. - // see https://www.zpqrtbnk.net/posts/putting-things-in-contexts/ - private static T GetCallContextObject(string key) where T : class, IInstanceIdentifiable { diff --git a/src/Umbraco.Tests/Scoping/ScopeTests.cs b/src/Umbraco.Tests/Scoping/ScopeTests.cs index 1e1f202809..eb4a01d06b 100644 --- a/src/Umbraco.Tests/Scoping/ScopeTests.cs +++ b/src/Umbraco.Tests/Scoping/ScopeTests.cs @@ -119,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); - 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];