1388 xml serializer assembly load context awareness (#58932)

* Generate dynamic serialization assembly in the appropriate ALC, and don't keep any hard refs to types that could prevent unloading.
dotnet · Nov 5, 2021 · d2415c4 · d2415c4
1 parent 1c7100b
commit d2415c4
Show file tree

Hide file tree

Showing 12 changed files with 416 additions and 193 deletions.
diff --git a/src/libraries/Common/tests/System/Runtime/Serialization/Utils.cs b/src/libraries/Common/tests/System/Runtime/Serialization/Utils.cs
@@ -9,6 +9,8 @@
 using System.Threading.Tasks;
 using System.Xml.Linq;
 using System.Linq;
+using System.Reflection;
+using System.Runtime.Loader;
 using Xunit;
 
 internal static class Utils
@@ -351,3 +353,30 @@ private static bool IsPrefixedAttributeValue(string atrValue, out string localPr
         return false;
     }
 }
+
+internal class TestAssemblyLoadContext : AssemblyLoadContext
+{
+    private AssemblyDependencyResolver _resolver;
+
+    public TestAssemblyLoadContext(string name, bool isCollectible, string mainAssemblyToLoadPath = null) : base(name, isCollectible)
+    {
+        if (!PlatformDetection.IsBrowser)
+            _resolver = new AssemblyDependencyResolver(mainAssemblyToLoadPath ?? Assembly.GetExecutingAssembly().Location);
+    }
+
+    protected override Assembly Load(AssemblyName name)
+    {
+        if (PlatformDetection.IsBrowser)
+        {
+            return base.Load(name);
+        }
+
+        string assemblyPath = _resolver.ResolveAssemblyToPath(name);
+        if (assemblyPath != null)
+        {
+            return LoadFromAssemblyPath(assemblyPath);
+        }
+
+        return null;
+    }
+}
diff --git a/src/libraries/System.Private.Xml/src/Resources/Strings.resx b/src/libraries/System.Private.Xml/src/Resources/Strings.resx
@@ -2787,6 +2787,9 @@
   <data name="XmlNotSerializable" xml:space="preserve">
     <value>Type '{0}' is not serializable.</value>
   </data>
+  <data name="XmlTypeInBadLoadContext" xml:space="preserve">
+    <value>Type '{0}' is from an AssemblyLoadContext which is incompatible with that which contains this XmlSerializer.</value>
+  </data>
   <data name="XmlPregenInvalidXmlSerializerAssemblyAttribute" xml:space="preserve">
     <value>Invalid XmlSerializerAssemblyAttribute usage. Please use {0} property or {1} property.</value>
   </data>

diff --git a/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj b/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj
@@ -445,6 +445,7 @@
     <Compile Include="System\Xml\Serialization\CodeIdentifiers.cs" />
     <Compile Include="System\Xml\Serialization\Compilation.cs" />
     <Compile Include="System\Xml\Serialization\Compiler.cs" />
+    <Compile Include="System\Xml\Serialization\ContextAwareTables.cs" />
     <Compile Include="System\Xml\Serialization\ImportContext.cs" />
     <Compile Include="System\Xml\Serialization\indentedWriter.cs" />
     <Compile Include="System\Xml\Serialization\IXmlSerializable.cs" />
@@ -564,6 +565,7 @@
     <Reference Include="System.Runtime.CompilerServices.Unsafe" />
     <Reference Include="System.Runtime.Extensions" />
     <Reference Include="System.Runtime.InteropServices" />
+    <Reference Include="System.Runtime.Loader" />
     <Reference Include="System.Security.Cryptography" />
     <Reference Include="System.Security.Cryptography.Algorithms" />
     <Reference Include="System.Text.Encoding.Extensions" />

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/Compilation.cs b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/Compilation.cs
diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ContextAwareTables.cs b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ContextAwareTables.cs
@@ -0,0 +1,66 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace System.Xml.Serialization
+{
+    using System;
+    using System.Collections;
+    using System.Diagnostics.CodeAnalysis;
+    using System.Runtime.CompilerServices;
+    using System.Runtime.Loader;
+
+    internal class ContextAwareTables<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T> where T : class?
+    {
+        private Hashtable _defaultTable;
+        private ConditionalWeakTable<Type, T> _collectibleTable;
+
+        public ContextAwareTables()
+        {
+            _defaultTable = new Hashtable();
+            _collectibleTable = new ConditionalWeakTable<Type, T>();
+        }
+
+        internal T GetOrCreateValue(Type t, Func<T> f)
+        {
+            // The fast and most common default case
+            T? ret = (T?)_defaultTable[t];
+            if (ret != null)
+                return ret;
+
+            // Common case for collectible contexts
+            if (_collectibleTable.TryGetValue(t, out ret))
+                return ret;
+
+            // Not found. Do the slower work of creating the value in the correct collection.
+            AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(t.Assembly);
+
+            // Null and non-collectible load contexts use the default table
+            if (alc == null || !alc.IsCollectible)
+            {
+                lock (_defaultTable)
+                {
+                    if ((ret = (T?)_defaultTable[t]) == null)
+                    {
+                        ret = f();
+                        _defaultTable[t] = ret;
+                    }
+                }
+            }
+
+            // Collectible load contexts should use the ConditionalWeakTable so they can be unloaded
+            else
+            {
+                lock (_collectibleTable)
+                {
+                    if (!_collectibleTable.TryGetValue(t, out ret))
+                    {
+                        ret = f();
+                        _collectibleTable.AddOrUpdate(t, ret);
+                    }
+                }
+            }
+
+            return ret;
+        }
+    }
+}
diff --git a/...aries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationReader.cs b/...aries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationReader.cs
@@ -627,40 +627,48 @@ private static void AddObjectsIntoTargetCollection(object targetCollection, List
             }
         }
 
-        private static readonly ConcurrentDictionary<(Type, string), ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate> s_setMemberValueDelegateCache = new ConcurrentDictionary<(Type, string), ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate>();
+        private static readonly ContextAwareTables<Hashtable> s_setMemberValueDelegateCache = new ContextAwareTables<Hashtable>();
 
         [RequiresUnreferencedCode(XmlSerializer.TrimSerializationWarning)]
         private static ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate GetSetMemberValueDelegate(object o, string memberName)
         {
             Debug.Assert(o != null, "Object o should not be null");
             Debug.Assert(!string.IsNullOrEmpty(memberName), "memberName must have a value");
-            (Type, string) typeMemberNameTuple = (o.GetType(), memberName);
-            if (!s_setMemberValueDelegateCache.TryGetValue(typeMemberNameTuple, out ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate? result))
+            Type type = o.GetType();
+            var delegateCacheForType = s_setMemberValueDelegateCache.GetOrCreateValue(type, () => new Hashtable());
+            var result = delegateCacheForType[memberName];
+            if (result == null)
             {
-                MemberInfo memberInfo = ReflectionXmlSerializationHelper.GetEffectiveSetInfo(o.GetType(), memberName);
-                Debug.Assert(memberInfo != null, "memberInfo could not be retrieved");
-                Type memberType;
-                if (memberInfo is PropertyInfo propInfo)
-                {
-                    memberType = propInfo.PropertyType;
-                }
-                else if (memberInfo is FieldInfo fieldInfo)
-                {
-                    memberType = fieldInfo.FieldType;
-                }
-                else
+                lock (delegateCacheForType)
                 {
-                    throw new InvalidOperationException(SR.XmlInternalError);
-                }
+                    if ((result = delegateCacheForType[memberName]) == null)
+                    {
+                        MemberInfo memberInfo = ReflectionXmlSerializationHelper.GetEffectiveSetInfo(o.GetType(), memberName);
+                        Debug.Assert(memberInfo != null, "memberInfo could not be retrieved");
+                        Type memberType;
+                        if (memberInfo is PropertyInfo propInfo)
+                        {
+                            memberType = propInfo.PropertyType;
+                        }
+                        else if (memberInfo is FieldInfo fieldInfo)
+                        {
+                            memberType = fieldInfo.FieldType;
+                        }
+                        else
+                        {
+                            throw new InvalidOperationException(SR.XmlInternalError);
+                        }
 
-                MethodInfo getSetMemberValueDelegateWithTypeGenericMi = typeof(ReflectionXmlSerializationReaderHelper).GetMethod("GetSetMemberValueDelegateWithType", BindingFlags.Static | BindingFlags.Public)!;
-                MethodInfo getSetMemberValueDelegateWithTypeMi = getSetMemberValueDelegateWithTypeGenericMi.MakeGenericMethod(o.GetType(), memberType);
-                var getSetMemberValueDelegateWithType = (Func<MemberInfo, ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate>)getSetMemberValueDelegateWithTypeMi.CreateDelegate(typeof(Func<MemberInfo, ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate>));
-                result = getSetMemberValueDelegateWithType(memberInfo);
-                s_setMemberValueDelegateCache.TryAdd(typeMemberNameTuple, result);
+                        MethodInfo getSetMemberValueDelegateWithTypeGenericMi = typeof(ReflectionXmlSerializationReaderHelper).GetMethod("GetSetMemberValueDelegateWithType", BindingFlags.Static | BindingFlags.Public)!;
+                        MethodInfo getSetMemberValueDelegateWithTypeMi = getSetMemberValueDelegateWithTypeGenericMi.MakeGenericMethod(o.GetType(), memberType);
+                        var getSetMemberValueDelegateWithType = (Func<MemberInfo, ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate>)getSetMemberValueDelegateWithTypeMi.CreateDelegate(typeof(Func<MemberInfo, ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate>));
+                        result = getSetMemberValueDelegateWithType(memberInfo);
+                        delegateCacheForType[memberName] = result;
+                    }
+                }
             }
 
-            return result;
+            return (ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate)result;
         }
 
         private object? GetMemberValue(object o, MemberInfo memberInfo)

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/XmlSerializationWriter.cs b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/XmlSerializationWriter.cs
@@ -19,6 +19,7 @@ namespace System.Xml.Serialization
     using System.Xml.Serialization;
     using System.Xml;
     using System.Diagnostics.CodeAnalysis;
+    using System.Runtime.CompilerServices;
 
     ///<internalonly/>
     public abstract class XmlSerializationWriter : XmlSerializationGeneratedCode
@@ -1465,14 +1466,13 @@ internal static class DynamicAssemblies
     {
         private static readonly Hashtable s_nameToAssemblyMap = new Hashtable();
         private static readonly Hashtable s_assemblyToNameMap = new Hashtable();
-        private static readonly Hashtable s_tableIsTypeDynamic = Hashtable.Synchronized(new Hashtable());
+        private static readonly ContextAwareTables<object> s_tableIsTypeDynamic = new ContextAwareTables<object>();
 
         // SxS: This method does not take any resource name and does not expose any resources to the caller.
         // It's OK to suppress the SxS warning.
         internal static bool IsTypeDynamic(Type type)
         {
-            object? oIsTypeDynamic = s_tableIsTypeDynamic[type];
-            if (oIsTypeDynamic == null)
+            object oIsTypeDynamic = s_tableIsTypeDynamic.GetOrCreateValue(type, () =>
             {
                 Assembly assembly = type.Assembly;
                 bool isTypeDynamic = assembly.IsDynamic /*|| string.IsNullOrEmpty(assembly.Location)*/;
@@ -1500,8 +1500,8 @@ internal static bool IsTypeDynamic(Type type)
                         }
                     }
                 }
-                s_tableIsTypeDynamic[type] = oIsTypeDynamic = isTypeDynamic;
-            }
+                return isTypeDynamic;
+            });
             return (bool)oIsTypeDynamic;
         }
 

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/XmlSerializer.cs b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/XmlSerializer.cs
@@ -10,6 +10,7 @@
 using System.IO;
 using System.Reflection;
 using System.Runtime.CompilerServices;
+using System.Runtime.Loader;
 using System.Runtime.Versioning;
 using System.Security;
 using System.Text;
@@ -161,8 +162,7 @@ private static XmlSerializerNamespaces DefaultNamespaces
         internal const string TrimSerializationWarning = "Members from serialized types may be trimmed if not referenced directly";
         private const string TrimDeserializationWarning = "Members from deserialized types may be trimmed if not referenced directly";
 
-        private static readonly Dictionary<Type, Dictionary<XmlSerializerMappingKey, XmlSerializer>> s_xmlSerializerTable = new Dictionary<Type, Dictionary<XmlSerializerMappingKey, XmlSerializer>>();
-
+        private static readonly ContextAwareTables<Dictionary<XmlSerializerMappingKey, XmlSerializer>> s_xmlSerializerTable = new ContextAwareTables<Dictionary<XmlSerializerMappingKey, XmlSerializer>>();
         protected XmlSerializer()
         {
         }
@@ -235,30 +235,28 @@ public XmlSerializer(Type type, string? defaultNamespace)
                     _tempAssembly = s_cache[defaultNamespace, type];
                     if (_tempAssembly == null)
                     {
+                        XmlSerializerImplementation? contract = null;
+                        Assembly? assembly = TempAssembly.LoadGeneratedAssembly(type, defaultNamespace, out contract);
+                        if (assembly == null)
                         {
-                            XmlSerializerImplementation? contract = null;
-                            Assembly? assembly = TempAssembly.LoadGeneratedAssembly(type, defaultNamespace, out contract);
-                            if (assembly == null)
-                            {
-                                if (Mode == SerializationMode.PreGenOnly)
-                                {
-                                    AssemblyName name = type.Assembly.GetName();
-                                    var serializerName = Compiler.GetTempAssemblyName(name, defaultNamespace);
-                                    throw new FileLoadException(SR.Format(SR.FailLoadAssemblyUnderPregenMode, serializerName));
-                                }
-
-                                // need to reflect and generate new serialization assembly
-                                XmlReflectionImporter importer = new XmlReflectionImporter(defaultNamespace);
-                                _mapping = importer.ImportTypeMapping(type, null, defaultNamespace);
-                                _tempAssembly = GenerateTempAssembly(_mapping, type, defaultNamespace)!;
-                            }
-                            else
+                            if (Mode == SerializationMode.PreGenOnly)
                             {
-                                // we found the pre-generated assembly, now make sure that the assembly has the right serializer
-                                // try to avoid the reflection step, need to get ElementName, namespace and the Key form the type
-                                _mapping = XmlReflectionImporter.GetTopLevelMapping(type, defaultNamespace);
-                                _tempAssembly = new TempAssembly(new XmlMapping[] { _mapping }, assembly, contract);
+                                AssemblyName name = type.Assembly.GetName();
+                                var serializerName = Compiler.GetTempAssemblyName(name, defaultNamespace);
+                                throw new FileLoadException(SR.Format(SR.FailLoadAssemblyUnderPregenMode, serializerName));
                             }
+
+                            // need to reflect and generate new serialization assembly
+                            XmlReflectionImporter importer = new XmlReflectionImporter(defaultNamespace);
+                            _mapping = importer.ImportTypeMapping(type, null, defaultNamespace);
+                            _tempAssembly = GenerateTempAssembly(_mapping, type, defaultNamespace)!;
+                        }
+                        else
+                        {
+                            // we found the pre-generated assembly, now make sure that the assembly has the right serializer
+                            // try to avoid the reflection step, need to get ElementName, namespace and the Key form the type
+                            _mapping = XmlReflectionImporter.GetTopLevelMapping(type, defaultNamespace);
+                            _tempAssembly = new TempAssembly(new XmlMapping[] { _mapping }, assembly, contract);
                         }
                     }
                     s_cache.Add(defaultNamespace, type, _tempAssembly);
@@ -403,7 +401,9 @@ public void Serialize(XmlWriter xmlWriter, object? o, XmlSerializerNamespaces? n
                     }
                 }
                 else
+                {
                     _tempAssembly.InvokeWriter(_mapping, xmlWriter, o, namespaces == null || namespaces.Count == 0 ? DefaultNamespaces : namespaces, encodingStyle, id);
+                }
             }
             catch (Exception? e)
             {
@@ -629,7 +629,10 @@ public static XmlSerializer[] FromMappings(XmlMapping[]? mappings, Type? type)
             {
                 XmlSerializer[] serializers = new XmlSerializer[mappings.Length];
                 for (int i = 0; i < serializers.Length; i++)
+                {
                     serializers[i] = (XmlSerializer)contract!.TypedSerializers[mappings[i].Key!]!;
+                    TempAssembly.VerifyLoadContext(serializers[i]._rootType, type!.Assembly);
+                }
                 return serializers;
             }
         }
@@ -696,16 +699,10 @@ internal static bool GenerateSerializer(Type[]? types, XmlMapping[] mappings, St
         private static XmlSerializer[] GetSerializersFromCache(XmlMapping[] mappings, Type type)
         {
             XmlSerializer?[] serializers = new XmlSerializer?[mappings.Length];
-
             Dictionary<XmlSerializerMappingKey, XmlSerializer>? typedMappingTable = null;
-            lock (s_xmlSerializerTable)
-            {
-                if (!s_xmlSerializerTable.TryGetValue(type, out typedMappingTable))
-                {
-                    typedMappingTable = new Dictionary<XmlSerializerMappingKey, XmlSerializer>();
-                    s_xmlSerializerTable[type] = typedMappingTable;
-                }
-            }
+            AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(type.Assembly);
+
+            typedMappingTable = s_xmlSerializerTable.GetOrCreateValue(type, () => new Dictionary<XmlSerializerMappingKey, XmlSerializer>());
 
             lock (typedMappingTable)
             {

diff --git a/...l/tests/XmlSerializer/ReflectionOnly/System.Xml.XmlSerializer.ReflectionOnly.Tests.csproj b/...l/tests/XmlSerializer/ReflectionOnly/System.Xml.XmlSerializer.ReflectionOnly.Tests.csproj
@@ -3,9 +3,13 @@
     <DefineConstants>$(DefineConstants);ReflectionOnly</DefineConstants>
     <TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
   </PropertyGroup>
+  <ItemGroup>
+    <ProjectReference Include="..\..\..\..\Microsoft.XmlSerializer.Generator\tests\SerializableAssembly.csproj" />
+    <TrimmerRootAssembly Include="SerializableAssembly" />
+  </ItemGroup>
   <ItemGroup>
     <Compile Include="$(CommonTestPath)System\Runtime\Serialization\Utils.cs" />
-    <Compile Include="$(TestSourceFolder)..\..\..\..\System.Runtime.Serialization.Xml\tests\SerializationTypes.cs" />
+    <None Include="$(TestSourceFolder)..\..\..\..\System.Runtime.Serialization.Xml\tests\SerializationTypes.cs" />
     <Compile Include="$(TestSourceFolder)..\..\..\..\System.Runtime.Serialization.Xml\tests\SerializationTypes.RuntimeOnly.cs" />
     <Compile Include="$(TestSourceFolder)..\XmlSerializerTests.cs" />
     <Compile Include="$(TestSourceFolder)..\XmlSerializerTests.RuntimeOnly.cs" />

diff --git a/src/libraries/System.Private.Xml/tests/XmlSerializer/System.Xml.XmlSerializer.Tests.csproj b/src/libraries/System.Private.Xml/tests/XmlSerializer/System.Xml.XmlSerializer.Tests.csproj
@@ -2,10 +2,14 @@
   <PropertyGroup>
     <TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
   </PropertyGroup>
+  <ItemGroup>
+    <ProjectReference Include="..\..\..\Microsoft.XmlSerializer.Generator\tests\SerializableAssembly.csproj" />
+    <TrimmerRootAssembly Include="SerializableAssembly" />
+  </ItemGroup>
   <ItemGroup>
     <Compile Include="$(CommonTestPath)System\Runtime\Serialization\Utils.cs" />
     <Compile Include="$(TestSourceFolder)..\..\..\System.Runtime.Serialization.Xml\tests\SerializationTypes.RuntimeOnly.cs" />
-    <Compile Include="$(TestSourceFolder)..\..\..\System.Runtime.Serialization.Xml\tests\SerializationTypes.cs" />
+    <None Include="$(TestSourceFolder)..\..\..\System.Runtime.Serialization.Xml\tests\SerializationTypes.cs" />
     <Compile Include="$(TestSourceFolder)XmlSerializerTests.cs" />
     <Compile Include="$(TestSourceFolder)XmlSerializerTests.Internal.cs" />
     <Compile Include="$(TestSourceFolder)XmlSerializerTests.RuntimeOnly.cs" />