Introduce SafeCallContext

src/Umbraco.Core/Packaging/PackageBinaryInspector.cs

@@ -30,20 +30,26 @@ namespace Umbraco.Core.Packaging
         /// </remarks>
         public static IEnumerable<string> ScanAssembliesForTypeReference<T>(IEnumerable<byte[]> assemblys, out string[] errorReport)
         {
-            var appDomain = GetTempAppDomain();
-            var type = typeof(PackageBinaryInspector);
-            try
+            // need to wrap in a safe call context in order to prevent whatever Umbraco stores
+            // in the logical call context from flowing to the created app domain (eg, the
+            // UmbracoDatabase is *not* serializable and cannot and should not flow).
+            using (new SafeCallContext())
             {
-                var value = (PackageBinaryInspector)appDomain.CreateInstanceAndUnwrap(
-                       type.Assembly.FullName,
-                       type.FullName);
-                // do NOT turn PerformScan into static (even if ReSharper says so)!
-                var result = value.PerformScan<T>(assemblys.ToArray(), out errorReport);
-                return result;
-            }
-            finally
-            {
-                AppDomain.Unload(appDomain);
+                var appDomain = GetTempAppDomain();
+                var type = typeof(PackageBinaryInspector);
+                try
+                {
+                    var value = (PackageBinaryInspector)appDomain.CreateInstanceAndUnwrap(
+                           type.Assembly.FullName,
+                           type.FullName);
+                    // do NOT turn PerformScan into static (even if ReSharper says so)!
+                    var result = value.PerformScan<T>(assemblys.ToArray(), out errorReport);
+                    return result;
+                }
+                finally
+                {
+                    AppDomain.Unload(appDomain);
+                }
             }
         }
 

src/Umbraco.Core/SafeCallContext.cs

@@ -0,0 +1,96 @@
+using System;
+using System.Linq;
+using System.Collections.Generic;
+
+namespace Umbraco.Core
+{
+    internal class SafeCallContext : IDisposable
+    {
+        private static readonly List<Func<object>> EnterFuncs = new List<Func<object>>();
+        private static readonly List<Action<object>> ExitActions = new List<Action<object>>();
+        private static int _count;
+        private readonly List<object> _objects;
+        private bool _disposed;
+
+        public static void Register(Func<object> enterFunc, Action<object> exitAction)
+        {
+            if (enterFunc == null) throw new ArgumentNullException(nameof(enterFunc));
+            if (exitAction == null) throw new ArgumentNullException(nameof(exitAction));
+
+            lock (EnterFuncs)
+            {
+                if (_count > 0) throw new InvalidOperationException("Cannot register while some SafeCallContext instances exist.");
+                EnterFuncs.Add(enterFunc);
+                ExitActions.Add(exitAction);
+            }
+        }
+
+        // tried to make the UmbracoDatabase serializable but then it leaks to weird places
+        // in ReSharper and so on, where Umbraco.Core is not available. Tried to serialize
+        // as an object instead but then it comes *back* deserialized into the original context
+        // as an object and of course it breaks everything. Cannot prevent this from flowing,
+        // and ExecutionContext.SuppressFlow() works for threads but not domains. and we'll
+        // have the same issue with anything that toys with logical call context...
+        //
+        // so this class lets anything that uses the logical call context register itself,
+        // providing two methods:
+        // - an enter func that removes and returns whatever is in the logical call context
+        // - an exit action that restores the value into the logical call context
+        // whenever a SafeCallContext instance is created, it uses these methods to capture
+        // and clear the logical call context, and restore it when disposed.
+        //
+        // in addition, a static Clear method is provided - which uses the enter funcs to
+        // remove everything from logical call context - not to be used when the app runs,
+        // but can be useful during tests
+        //
+        // note
+        // see System.Transactions
+        // pre 4.5.1, the TransactionScope would not flow in async, and then then introduced
+        // an option to store in in the LLC so that it flows
+        // they are using a conditional weak table to store the data, and what they store in
+        // LLC is the key - which is just an empty MarshalByRefObject that is created with
+        // the transaction scope - that way, they can "clear current data" provided that
+        // they have the key - but they need to hold onto a ref to the scope... not ok for us
+
+        public static void Clear()
+        {
+            lock (EnterFuncs)
+            {
+                foreach (var enter in EnterFuncs)
+                    enter();
+            }
+        }
+
+        public SafeCallContext()
+        {
+            lock (EnterFuncs)
+            {
+                _count++;
+                _objects = EnterFuncs.Select(x => x()).ToList();
+            }
+        }
+
+        public void Dispose()
+        {
+            if (_disposed) throw new ObjectDisposedException("this");
+            _disposed = true;
+            lock (EnterFuncs)
+            {
+                for (var i = 0; i < ExitActions.Count; i++)
+                    ExitActions[i](_objects[i]);
+                _count--;
+            }
+        }
+
+        // for unit tests ONLY
+        internal static void Reset()
+        {
+            lock (EnterFuncs)
+            {
+                if (_count > 0) throw new InvalidOperationException("Cannot reset while some SafeCallContext instances exist.");
+                EnterFuncs.Clear();
+                ExitActions.Clear();
+            }
+        }
+    }
+}

src/Umbraco.Core/Umbraco.Core.csproj

@@ -476,6 +476,7 @@
     <Compile Include="PropertyEditors\ValueConverters\ImageCropperValueConverter.cs" />
     <Compile Include="RuntimeLevel.cs" />
     <Compile Include="RuntimeState.cs" />
+    <Compile Include="SafeCallContext.cs" />
     <Compile Include="Serialization\ForceInt32Converter.cs" />
     <Compile Include="Services\Changes\ContentTypeChange.cs" />
     <Compile Include="Services\Changes\ContentTypeChangeExtensions.cs" />

src/Umbraco.Web/HybridAccessorBase.cs

@@ -1,8 +1,11 @@
+using System;
 using System.Runtime.Remoting.Messaging;
+using Umbraco.Core;
 
 namespace Umbraco.Web
 {
     internal abstract class HybridAccessorBase<T>
+        where T : class
     {
         private readonly IHttpContextAccessor _httpContextAccessor;
 
@@ -18,6 +21,7 @@ namespace Umbraco.Web
         // anything that is ThreadStatic will stay with the thread and NOT flow in async threads
         // the only thing that flows is the logical call context (safe in 4.5+)
         // now...
+        // fixme - which tests?!
         // tests seem to show that either newing Thread or enqueuing in the ThreadPool both produce a thread
         // with a clear logical call context, which would mean that it is somewhat safe to "fire and forget"
         // because whatever is in the call context will be gone when the thread returns to the pool
@@ -37,6 +41,30 @@ namespace Umbraco.Web
             }
         }
 
+        // every class inheriting from this class *must* implement a static ctor
+        // and register itself against the SafeCallContext using this method.
+        //
+        // note: because the item key is not static, we cannot register here in the
+        // base class - unless we do it in the non-static Value property setter,
+        // with a static bool to keep track of registration - less error-prone but
+        // perfs impact - considering implementors should be careful.
+        //
+        protected static void SafeCallContextRegister(string itemKey)
+        {
+            SafeCallContext.Register(() =>
+            {
+                var value = CallContext.LogicalGetData(itemKey);
+                CallContext.FreeNamedDataSlot(itemKey);
+                return value;
+            }, o =>
+            {
+                if (o == null) return;
+                var value = o as T;
+                if (value == null) throw new ArgumentException($"Expected type {typeof(T).FullName}, got {o.GetType().FullName}", nameof(o));
+                CallContext.LogicalSetData(itemKey, value);
+            });
+        }
+
         protected HybridAccessorBase(IHttpContextAccessor httpContextAccessor)
         {
             _httpContextAccessor = httpContextAccessor;
@@ -55,11 +83,11 @@ namespace Umbraco.Web
             {
                 var httpContext = _httpContextAccessor.HttpContext;
                 if (httpContext == null)
-                {
                     NonContextValue = value;
-                    return;
-                }
-                httpContext.Items[ItemKey] = value;
+                else if (value == null)
+                    httpContext.Items.Remove(ItemKey);
+                else
+                    httpContext.Items[ItemKey] = value;
             }
         }
     }

src/Umbraco.Web/HybridEventMessagesAccessor.cs

@@ -4,7 +4,14 @@ namespace Umbraco.Web
 {
     internal class HybridEventMessagesAccessor : HybridAccessorBase<EventMessages>, IEventMessagesAccessor
     {
-        protected override string ItemKey => "Umbraco.Core.Events.EventMessages";
+        private const string ItemKeyConst = "Umbraco.Core.Events.HybridEventMessagesAccessor";
+
+        protected override string ItemKey => ItemKeyConst;
+
+        static HybridEventMessagesAccessor()
+        {
+            SafeCallContextRegister(ItemKeyConst);
+        }
 
         public HybridEventMessagesAccessor(IHttpContextAccessor httpContextAccessor)
             : base(httpContextAccessor)

src/Umbraco.Web/HybridUmbracoContextAccessor.cs

@@ -2,7 +2,14 @@
 {
     internal class HybridUmbracoContextAccessor : HybridAccessorBase<UmbracoContext>, IUmbracoContextAccessor
     {
-        protected override string ItemKey => "Umbraco.Web.UmbracoContext";
+        private const string ItemKeyConst = "Umbraco.Web.HybridUmbracoContextAccessor";
+
+        protected override string ItemKey => ItemKeyConst;
+
+        static HybridUmbracoContextAccessor()
+        {
+            SafeCallContextRegister(ItemKeyConst);
+        }
 
         public HybridUmbracoContextAccessor(IHttpContextAccessor httpContextAccessor)
             : base(httpContextAccessor)

src/Umbraco.Web/HybridUmbracoDatabaseAccessor.cs

@@ -4,7 +4,14 @@ namespace Umbraco.Web
 {
     internal class HybridUmbracoDatabaseAccessor : HybridAccessorBase<UmbracoDatabase>, IUmbracoDatabaseAccessor
     {
-        protected override string ItemKey => "Umbraco.Core.Persistence.UmbracoDatabase";
+        private const string ItemKeyConst = "Umbraco.Core.Persistence.HybridUmbracoDatabaseAccessor";
+
+        protected override string ItemKey => ItemKeyConst;
+
+        static HybridUmbracoDatabaseAccessor()
+        {
+            SafeCallContextRegister(ItemKeyConst);
+        }
 
         public HybridUmbracoDatabaseAccessor(IHttpContextAccessor httpContextAccessor)
             : base(httpContextAccessor)