From 04a58365e6f997cb55c1ab534e5befdf502002e0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 10 Jan 2022 15:32:03 -0800 Subject: [PATCH] [release/6.0] 1388 xml serializer assembly load context awareness (#61266) * Generate dynamic serialization assembly in the appropriate ALC, and don't keep any hard refs to types that could prevent unloading. * Add small test for ALC unloading. * PR feedback; Move into TempAssembly; Strong/Weak lookup tables; Test refinement * Refining TempAssembly cache; Reflection-based fix; Test corrections * Mono/wasm test fixup * Ensure that serializers from XmlMappings don't run afoul of collectible ALCs. * PR feedback * Unloadable contexts not yet supported in Mono. * Fix trimming of types for XmlSerializer tests that got moved out of main test assembly. * Encapsulating the duality of dealing with unloadable ALC instead of rewriting similar code everywhere. * Missed new file in last commit. * Compilation bug not seen locally. * Perf improvement. * Remove extraneous line. Test feedback. * Fix unloadability (#58948) There is a bug in AppDomain::FindAssembly code path that iterates over the domain assemblies. The iteration loop leaves the refcount of the LoaderAllocator related to the returned DomainAssembly bumped, but nothing ever decrements it. So when a code that needs to be unloaded ends up in that code path, all the managed things like managed LoaderAllocator, LoaderAllocatorScout are destroyed, but the unloading doesn't complete due to the refcount. We have never found it before as this code path is never executed in any of the coreclr tests even with unloadability testing option. (cherry picked from commit 8b38c1973eeaf1c13e8d2729ab1b1fe1a7cf9a5b) Co-authored-by: Steve Molloy Co-authored-by: Jan Vorlicek --- src/coreclr/vm/appdomain.cpp | 4 +- .../System/Runtime/Serialization/Utils.cs | 29 ++ .../src/Resources/Strings.resx | 3 + .../src/System.Private.Xml.csproj | 2 + .../System/Xml/Serialization/Compilation.cs | 309 +++++++++++------- .../Xml/Serialization/ContextAwareTables.cs | 66 ++++ .../ReflectionXmlSerializationReader.cs | 54 +-- .../Serialization/XmlSerializationWriter.cs | 10 +- .../System/Xml/Serialization/XmlSerializer.cs | 60 ++-- ....XmlSerializer.ReflectionOnly.Tests.csproj | 6 +- .../System.Xml.XmlSerializer.Tests.csproj | 6 +- .../tests/XmlSerializer/XmlSerializerTests.cs | 54 ++- .../tests/SerializationTypes.cs | 10 + 13 files changed, 418 insertions(+), 195 deletions(-) create mode 100644 src/libraries/System.Private.Xml/src/System/Xml/Serialization/ContextAwareTables.cs diff --git a/src/coreclr/vm/appdomain.cpp b/src/coreclr/vm/appdomain.cpp index a55090bac1ec5f..13201596b7cfc3 100644 --- a/src/coreclr/vm/appdomain.cpp +++ b/src/coreclr/vm/appdomain.cpp @@ -3533,8 +3533,8 @@ DomainAssembly * AppDomain::FindAssembly(PEAssembly * pFile, FindAssemblyOptions !pManifestFile->IsResource() && pManifestFile->Equals(pFile)) { - // Caller already has PEAssembly, so we can give DomainAssembly away freely without AddRef - return pDomainAssembly.Extract(); + // Caller already has PEAssembly, so we can give DomainAssembly away freely without added reference + return pDomainAssembly.GetValue(); } } return NULL; diff --git a/src/libraries/Common/tests/System/Runtime/Serialization/Utils.cs b/src/libraries/Common/tests/System/Runtime/Serialization/Utils.cs index ca01381a6dff10..2e2ac299c2c53b 100644 --- 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 index d19dc6ec4eb314..112359dfecba8c 100644 --- a/src/libraries/System.Private.Xml/src/Resources/Strings.resx +++ b/src/libraries/System.Private.Xml/src/Resources/Strings.resx @@ -2787,6 +2787,9 @@ Type '{0}' is not serializable. + + Type '{0}' is from an AssemblyLoadContext which is incompatible with that which contains this XmlSerializer. + Invalid XmlSerializerAssemblyAttribute usage. Please use {0} property or {1} property. diff --git a/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj b/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj index 2afc279cf5f805..648245cdb49703 100644 --- a/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj +++ b/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj @@ -446,6 +446,7 @@ + @@ -565,6 +566,7 @@ + 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 index 8fdeed7b60b770..c3df77a5a2bc3f 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/Compilation.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/Compilation.cs @@ -11,6 +11,8 @@ using System.Globalization; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; +using System.Runtime.Loader; namespace System.Xml.Serialization { @@ -149,79 +151,82 @@ internal void InitAssemblyMethods(XmlMapping[] xmlMappings) contract = null; string? serializerName = null; - // check to see if we loading explicit pre-generated assembly - object[] attrs = type.GetCustomAttributes(typeof(System.Xml.Serialization.XmlSerializerAssemblyAttribute), false); - if (attrs.Length == 0) + using (AssemblyLoadContext.EnterContextualReflection(type.Assembly)) { - // Guess serializer name: if parent assembly signed use strong name - AssemblyName name = type.Assembly.GetName(); - serializerName = Compiler.GetTempAssemblyName(name, defaultNamespace); - // use strong name - name.Name = serializerName; - name.CodeBase = null; - name.CultureInfo = CultureInfo.InvariantCulture; - - try - { - serializer = Assembly.Load(name); - } - catch (Exception e) - { - if (e is OutOfMemoryException) + // check to see if we loading explicit pre-generated assembly + object[] attrs = type.GetCustomAttributes(typeof(System.Xml.Serialization.XmlSerializerAssemblyAttribute), false); + if (attrs.Length == 0) + { + // Guess serializer name: if parent assembly signed use strong name + AssemblyName name = type.Assembly.GetName(); + serializerName = Compiler.GetTempAssemblyName(name, defaultNamespace); + // use strong name + name.Name = serializerName; + name.CodeBase = null; + name.CultureInfo = CultureInfo.InvariantCulture; + + try { - throw; + serializer = Assembly.Load(name); } - } - - serializer ??= LoadAssemblyByPath(type, serializerName); - - if (serializer == null) - { - if (XmlSerializer.Mode == SerializationMode.PreGenOnly) + catch (Exception e) { - throw new Exception(SR.Format(SR.FailLoadAssemblyUnderPregenMode, serializerName)); + if (e is OutOfMemoryException) + { + throw; + } } - return null; - } + serializer ??= LoadAssemblyByPath(type, serializerName); - if (!IsSerializerVersionMatch(serializer, type, defaultNamespace)) - { - XmlSerializationEventSource.Log.XmlSerializerExpired(serializerName, type.FullName!); - return null; - } - } - else - { - System.Xml.Serialization.XmlSerializerAssemblyAttribute assemblyAttribute = (System.Xml.Serialization.XmlSerializerAssemblyAttribute)attrs[0]; - if (assemblyAttribute.AssemblyName != null && assemblyAttribute.CodeBase != null) - throw new InvalidOperationException(SR.Format(SR.XmlPregenInvalidXmlSerializerAssemblyAttribute, "AssemblyName", "CodeBase")); + if (serializer == null) + { + if (XmlSerializer.Mode == SerializationMode.PreGenOnly) + { + throw new Exception(SR.Format(SR.FailLoadAssemblyUnderPregenMode, serializerName)); + } - // found XmlSerializerAssemblyAttribute attribute, it should have all needed information to load the pre-generated serializer - if (assemblyAttribute.AssemblyName != null) - { - serializerName = assemblyAttribute.AssemblyName; - serializer = Assembly.Load(serializerName); // LoadWithPartialName just does this in .Net Core; changing the obsolete call. - } - else if (assemblyAttribute.CodeBase != null && assemblyAttribute.CodeBase.Length > 0) - { - serializerName = assemblyAttribute.CodeBase; - serializer = Assembly.LoadFrom(serializerName); + return null; + } + + if (!IsSerializerVersionMatch(serializer, type, defaultNamespace)) + { + XmlSerializationEventSource.Log.XmlSerializerExpired(serializerName, type.FullName!); + return null; + } } else { - serializerName = type.Assembly.FullName; - serializer = type.Assembly; - } - if (serializer == null) - { - throw new FileNotFoundException(null, serializerName); + System.Xml.Serialization.XmlSerializerAssemblyAttribute assemblyAttribute = (System.Xml.Serialization.XmlSerializerAssemblyAttribute)attrs[0]; + if (assemblyAttribute.AssemblyName != null && assemblyAttribute.CodeBase != null) + throw new InvalidOperationException(SR.Format(SR.XmlPregenInvalidXmlSerializerAssemblyAttribute, "AssemblyName", "CodeBase")); + + // found XmlSerializerAssemblyAttribute attribute, it should have all needed information to load the pre-generated serializer + if (assemblyAttribute.AssemblyName != null) + { + serializerName = assemblyAttribute.AssemblyName; + serializer = Assembly.Load(serializerName); // LoadWithPartialName just does this in .Net Core; changing the obsolete call. + } + else if (assemblyAttribute.CodeBase != null && assemblyAttribute.CodeBase.Length > 0) + { + serializerName = assemblyAttribute.CodeBase; + serializer = Assembly.LoadFrom(serializerName); + } + else + { + serializerName = type.Assembly.FullName; + serializer = type.Assembly; + } + if (serializer == null) + { + throw new FileNotFoundException(null, serializerName); + } } + Type contractType = GetTypeFromAssembly(serializer, "XmlSerializerContract"); + contract = (XmlSerializerImplementation)Activator.CreateInstance(contractType)!; + if (contract.CanSerialize(type)) + return serializer; } - Type contractType = GetTypeFromAssembly(serializer, "XmlSerializerContract"); - contract = (XmlSerializerImplementation)Activator.CreateInstance(contractType)!; - if (contract.CanSerialize(type)) - return serializer; return null; } @@ -449,81 +454,93 @@ internal static bool GenerateSerializerToStream(XmlMapping[] xmlMappings, Type?[ } [RequiresUnreferencedCode("calls GenerateElement")] - internal static Assembly GenerateRefEmitAssembly(XmlMapping[] xmlMappings, Type?[]? types, string? defaultNamespace) + internal static Assembly GenerateRefEmitAssembly(XmlMapping[] xmlMappings, Type?[] types, string? defaultNamespace) { + var mainType = (types.Length > 0) ? types[0] : null; + Assembly? mainAssembly = mainType?.Assembly; var scopeTable = new Dictionary(); foreach (XmlMapping mapping in xmlMappings) scopeTable[mapping.Scope!] = mapping; TypeScope[] scopes = new TypeScope[scopeTable.Keys.Count]; scopeTable.Keys.CopyTo(scopes, 0); - string assemblyName = "Microsoft.GeneratedCode"; - AssemblyBuilder assemblyBuilder = CodeGenerator.CreateAssemblyBuilder(assemblyName); - // Add AssemblyVersion attribute to match parent assembly version - if (types != null && types.Length > 0 && types[0] != null) + using (AssemblyLoadContext.EnterContextualReflection(mainAssembly)) { - ConstructorInfo AssemblyVersionAttribute_ctor = typeof(AssemblyVersionAttribute).GetConstructor( - new Type[] { typeof(string) } - )!; - string assemblyVersion = types[0]!.Assembly.GetName().Version!.ToString(); - assemblyBuilder.SetCustomAttribute(new CustomAttributeBuilder(AssemblyVersionAttribute_ctor, new object[] { assemblyVersion })); - } - CodeIdentifiers classes = new CodeIdentifiers(); - classes.AddUnique("XmlSerializationWriter", "XmlSerializationWriter"); - classes.AddUnique("XmlSerializationReader", "XmlSerializationReader"); - string? suffix = null; - if (types != null && types.Length == 1 && types[0] != null) - { - suffix = CodeIdentifier.MakeValid(types[0]!.Name); - if (types[0]!.IsArray) + // Before generating any IL, check each mapping and supported type to make sure + // they are compatible with the current ALC + for (int i = 0; i < types.Length; i++) + VerifyLoadContext(types[i], mainAssembly); + foreach (var mapping in xmlMappings) + VerifyLoadContext(mapping.Accessor.Mapping?.TypeDesc?.Type, mainAssembly); + + string assemblyName = "Microsoft.GeneratedCode"; + AssemblyBuilder assemblyBuilder = CodeGenerator.CreateAssemblyBuilder(assemblyName); + // Add AssemblyVersion attribute to match parent assembly version + if (mainType != null) + { + ConstructorInfo AssemblyVersionAttribute_ctor = typeof(AssemblyVersionAttribute).GetConstructor( + new Type[] { typeof(string) } + )!; + string assemblyVersion = mainType.Assembly.GetName().Version!.ToString(); + assemblyBuilder.SetCustomAttribute(new CustomAttributeBuilder(AssemblyVersionAttribute_ctor, new object[] { assemblyVersion })); + } + CodeIdentifiers classes = new CodeIdentifiers(); + classes.AddUnique("XmlSerializationWriter", "XmlSerializationWriter"); + classes.AddUnique("XmlSerializationReader", "XmlSerializationReader"); + string? suffix = null; + if (mainType != null) { - suffix += "Array"; + suffix = CodeIdentifier.MakeValid(mainType.Name); + if (mainType.IsArray) + { + suffix += "Array"; + } } - } - ModuleBuilder moduleBuilder = CodeGenerator.CreateModuleBuilder(assemblyBuilder, assemblyName); + ModuleBuilder moduleBuilder = CodeGenerator.CreateModuleBuilder(assemblyBuilder, assemblyName); - string writerClass = "XmlSerializationWriter" + suffix; - writerClass = classes.AddUnique(writerClass, writerClass); - XmlSerializationWriterILGen writerCodeGen = new XmlSerializationWriterILGen(scopes, "public", writerClass); - writerCodeGen.ModuleBuilder = moduleBuilder; + string writerClass = "XmlSerializationWriter" + suffix; + writerClass = classes.AddUnique(writerClass, writerClass); + XmlSerializationWriterILGen writerCodeGen = new XmlSerializationWriterILGen(scopes, "public", writerClass); + writerCodeGen.ModuleBuilder = moduleBuilder; - writerCodeGen.GenerateBegin(); - string[] writeMethodNames = new string[xmlMappings.Length]; + writerCodeGen.GenerateBegin(); + string[] writeMethodNames = new string[xmlMappings.Length]; - for (int i = 0; i < xmlMappings.Length; i++) - { - writeMethodNames[i] = writerCodeGen.GenerateElement(xmlMappings[i])!; - } - Type writerType = writerCodeGen.GenerateEnd(); + for (int i = 0; i < xmlMappings.Length; i++) + { + writeMethodNames[i] = writerCodeGen.GenerateElement(xmlMappings[i])!; + } + Type writerType = writerCodeGen.GenerateEnd(); - string readerClass = "XmlSerializationReader" + suffix; - readerClass = classes.AddUnique(readerClass, readerClass); - XmlSerializationReaderILGen readerCodeGen = new XmlSerializationReaderILGen(scopes, "public", readerClass); + string readerClass = "XmlSerializationReader" + suffix; + readerClass = classes.AddUnique(readerClass, readerClass); + XmlSerializationReaderILGen readerCodeGen = new XmlSerializationReaderILGen(scopes, "public", readerClass); - readerCodeGen.ModuleBuilder = moduleBuilder; - readerCodeGen.CreatedTypes.Add(writerType.Name, writerType); + readerCodeGen.ModuleBuilder = moduleBuilder; + readerCodeGen.CreatedTypes.Add(writerType.Name, writerType); - readerCodeGen.GenerateBegin(); - string[] readMethodNames = new string[xmlMappings.Length]; - for (int i = 0; i < xmlMappings.Length; i++) - { - readMethodNames[i] = readerCodeGen.GenerateElement(xmlMappings[i])!; - } - readerCodeGen.GenerateEnd(readMethodNames, xmlMappings, types!); + readerCodeGen.GenerateBegin(); + string[] readMethodNames = new string[xmlMappings.Length]; + for (int i = 0; i < xmlMappings.Length; i++) + { + readMethodNames[i] = readerCodeGen.GenerateElement(xmlMappings[i])!; + } + readerCodeGen.GenerateEnd(readMethodNames, xmlMappings, types!); - string baseSerializer = readerCodeGen.GenerateBaseSerializer("XmlSerializer1", readerClass, writerClass, classes); - var serializers = new Dictionary(); - for (int i = 0; i < xmlMappings.Length; i++) - { - if (!serializers.ContainsKey(xmlMappings[i].Key!)) + string baseSerializer = readerCodeGen.GenerateBaseSerializer("XmlSerializer1", readerClass, writerClass, classes); + var serializers = new Dictionary(); + for (int i = 0; i < xmlMappings.Length; i++) { - serializers[xmlMappings[i].Key!] = readerCodeGen.GenerateTypedSerializer(readMethodNames[i], writeMethodNames[i], xmlMappings[i], classes, baseSerializer, readerClass, writerClass); + if (!serializers.ContainsKey(xmlMappings[i].Key!)) + { + serializers[xmlMappings[i].Key!] = readerCodeGen.GenerateTypedSerializer(readMethodNames[i], writeMethodNames[i], xmlMappings[i], classes, baseSerializer, readerClass, writerClass); + } } - } - readerCodeGen.GenerateSerializerContract("XmlSerializerContract", xmlMappings, types!, readerClass, readMethodNames, writerClass, writeMethodNames, serializers); + readerCodeGen.GenerateSerializerContract("XmlSerializerContract", xmlMappings, types!, readerClass, readMethodNames, writerClass, writeMethodNames, serializers); - return writerType.Assembly; + return writerType.Assembly; + } } private static MethodInfo GetMethodFromType( @@ -588,6 +605,23 @@ internal bool CanRead(XmlMapping mapping, XmlReader xmlReader) return encodingStyle; } + internal static void VerifyLoadContext(Type? t, Assembly? assembly) + { + // The quick case, t is null or in the same assembly + if (t == null || assembly == null || t.Assembly == assembly) + return; + + // No worries if the type is not collectible + var typeALC = AssemblyLoadContext.GetLoadContext(t.Assembly); + if (typeALC == null || !typeALC.IsCollectible) + return; + + // Collectible types should be in the same collectible context + var baseALC = AssemblyLoadContext.GetLoadContext(assembly) ?? AssemblyLoadContext.CurrentContextualReflectionContext; + if (typeALC != baseALC) + throw new InvalidOperationException(SR.Format(SR.XmlTypeInBadLoadContext, t.FullName)); + } + [RequiresUnreferencedCode("calls Contract")] internal object? InvokeReader(XmlMapping mapping, XmlReader xmlReader, XmlDeserializationEvents events, string? encodingStyle) { @@ -666,9 +700,9 @@ internal sealed class TempMethodDictionary : Dictionary internal sealed class TempAssemblyCacheKey { private readonly string? _ns; - private readonly object _type; + private readonly Type _type; - internal TempAssemblyCacheKey(string? ns, object type) + internal TempAssemblyCacheKey(string? ns, Type type) { _type = type; _ns = ns; @@ -690,29 +724,52 @@ public override int GetHashCode() internal sealed class TempAssemblyCache { - private Dictionary _cache = new Dictionary(); + private Dictionary _fastCache = new Dictionary(); + private ConditionalWeakTable> _collectibleCaches = new ConditionalWeakTable>(); - internal TempAssembly? this[string? ns, object o] + internal TempAssembly? this[string? ns, Type t] { get { TempAssembly? tempAssembly; - _cache.TryGetValue(new TempAssemblyCacheKey(ns, o), out tempAssembly); + TempAssemblyCacheKey key = new TempAssemblyCacheKey(ns, t); + + if (_fastCache.TryGetValue(key, out tempAssembly)) + return tempAssembly; + + if (_collectibleCaches.TryGetValue(t.Assembly, out var cCache)) + cCache.TryGetValue(key, out tempAssembly); + return tempAssembly; } } - internal void Add(string? ns, object o, TempAssembly assembly) + internal void Add(string? ns, Type t, TempAssembly assembly) { - TempAssemblyCacheKey key = new TempAssemblyCacheKey(ns, o); lock (this) { - TempAssembly? tempAssembly; - if (_cache.TryGetValue(key, out tempAssembly) && tempAssembly == assembly) + TempAssembly? tempAssembly = this[ns, t]; + if (tempAssembly == assembly) return; - Dictionary _copy = new Dictionary(_cache); // clone - _copy[key] = assembly; - _cache = _copy; + + AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(t.Assembly); + TempAssemblyCacheKey key = new TempAssemblyCacheKey(ns, t); + Dictionary? cache; + + if (alc != null && alc.IsCollectible) + { + cache = _collectibleCaches.TryGetValue(t.Assembly, out var c) // Clone or create + ? new Dictionary(c) + : new Dictionary(); + cache[key] = assembly; + _collectibleCaches.AddOrUpdate(t.Assembly, cache); + } + else + { + cache = new Dictionary(_fastCache); // Clone + cache[key] = assembly; + _fastCache = cache; + } } } } 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 new file mode 100644 index 00000000000000..64e09bbc063b2c --- /dev/null +++ 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 _collectibleTable; + + public ContextAwareTables() + { + _defaultTable = new Hashtable(); + _collectibleTable = new ConditionalWeakTable(); + } + + internal T GetOrCreateValue(Type t, Func 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/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationReader.cs b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationReader.cs index 0c55638a27a66a..2477f283b01f16 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationReader.cs +++ b/src/libraries/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 s_setMemberValueDelegateCache = new ContextAwareTables(); [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)getSetMemberValueDelegateWithTypeMi.CreateDelegate(typeof(Func)); - 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)getSetMemberValueDelegateWithTypeMi.CreateDelegate(typeof(Func)); + 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 index 46877f900e9028..7ebc4462dfb7a6 100644 --- 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; /// 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 s_tableIsTypeDynamic = new ContextAwareTables(); // 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 index 20f9d18343cbb8..b920b8f64a1cf6 100644 --- 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> s_xmlSerializerTable = new Dictionary>(); - + private static readonly ContextAwareTables> s_xmlSerializerTable = new ContextAwareTables>(); 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,9 @@ internal static bool GenerateSerializer(Type[]? types, XmlMapping[] mappings, St private static XmlSerializer[] GetSerializersFromCache(XmlMapping[] mappings, Type type) { XmlSerializer?[] serializers = new XmlSerializer?[mappings.Length]; - Dictionary? typedMappingTable = null; - lock (s_xmlSerializerTable) - { - if (!s_xmlSerializerTable.TryGetValue(type, out typedMappingTable)) - { - typedMappingTable = new Dictionary(); - s_xmlSerializerTable[type] = typedMappingTable; - } - } + + typedMappingTable = s_xmlSerializerTable.GetOrCreateValue(type, () => new Dictionary()); lock (typedMappingTable) { diff --git a/src/libraries/System.Private.Xml/tests/XmlSerializer/ReflectionOnly/System.Xml.XmlSerializer.ReflectionOnly.Tests.csproj b/src/libraries/System.Private.Xml/tests/XmlSerializer/ReflectionOnly/System.Xml.XmlSerializer.ReflectionOnly.Tests.csproj index 53abcbd3957022..d7d1f3af3bb87f 100644 --- a/src/libraries/System.Private.Xml/tests/XmlSerializer/ReflectionOnly/System.Xml.XmlSerializer.ReflectionOnly.Tests.csproj +++ b/src/libraries/System.Private.Xml/tests/XmlSerializer/ReflectionOnly/System.Xml.XmlSerializer.ReflectionOnly.Tests.csproj @@ -3,9 +3,13 @@ $(DefineConstants);ReflectionOnly $(NetCoreAppCurrent) + + + + - + 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 index dbc8447b87c1cb..7818cd132825c4 100644 --- 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 @@ $(NetCoreAppCurrent) + + + + - + diff --git a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs index c918520a19c8ff..9fefff2137faa5 100644 --- a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs +++ b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs @@ -8,6 +8,8 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.Loader; using System.Text; using System.Threading; using System.Xml; @@ -1960,6 +1962,52 @@ public static void Xml_TypeWithSpecialCharacterInStringMember() Assert.Equal(x.Name, y.Name); } + [Fact] +#if XMLSERIALIZERGENERATORTESTS + // Lack of AssemblyDependencyResolver results in assemblies that are not loaded by path to get + // loaded in the default ALC, which causes problems for this test. + [SkipOnPlatform(TestPlatforms.Browser, "AssemblyDependencyResolver not supported in wasm")] +#endif + [ActiveIssue("34072", TestRuntimes.Mono)] + public static void Xml_TypeInCollectibleALC() + { + ExecuteAndUnload("SerializableAssembly.dll", "SerializationTypes.SimpleType", out var weakRef); + + for (int i = 0; weakRef.IsAlive && i < 10; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + Assert.True(!weakRef.IsAlive); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ExecuteAndUnload(string assemblyfile, string typename, out WeakReference wref) + { + var fullPath = Path.GetFullPath(assemblyfile); + var alc = new TestAssemblyLoadContext("XmlSerializerTests", true, fullPath); + wref = new WeakReference(alc); + + // Load assembly by path. By name, and it gets loaded in the default ALC. + var asm = alc.LoadFromAssemblyPath(fullPath); + + // Ensure the type loaded in the intended non-Default ALC + var type = asm.GetType(typename); + Assert.Equal(AssemblyLoadContext.GetLoadContext(type.Assembly), alc); + Assert.NotEqual(alc, AssemblyLoadContext.Default); + + // Round-Trip the instance + XmlSerializer serializer = new XmlSerializer(type); + var obj = Activator.CreateInstance(type); + var rtobj = SerializeAndDeserialize(obj, null, () => serializer, true); + Assert.NotNull(rtobj); + Assert.True(rtobj.Equals(obj)); + Assert.Equal(AssemblyLoadContext.GetLoadContext(rtobj.GetType().Assembly), alc); + + alc.Unload(); + } + + private static readonly string s_defaultNs = "http://tempuri.org/"; private static T RoundTripWithXmlMembersMapping(object requestBodyValue, string memberName, string baseline, bool skipStringCompare = false, string wrapperName = null) { @@ -2080,11 +2128,7 @@ private static Stream GenerateStreamFromString(string s) private static T SerializeAndDeserialize(T value, string baseline, Func serializerFactory = null, bool skipStringCompare = false, XmlSerializerNamespaces xns = null) { - XmlSerializer serializer = new XmlSerializer(typeof(T)); - if (serializerFactory != null) - { - serializer = serializerFactory(); - } + XmlSerializer serializer = (serializerFactory != null) ? serializerFactory() : new XmlSerializer(typeof(T)); using (MemoryStream ms = new MemoryStream()) { diff --git a/src/libraries/System.Runtime.Serialization.Xml/tests/SerializationTypes.cs b/src/libraries/System.Runtime.Serialization.Xml/tests/SerializationTypes.cs index bc10a370b033cd..747f661129a94a 100644 --- a/src/libraries/System.Runtime.Serialization.Xml/tests/SerializationTypes.cs +++ b/src/libraries/System.Runtime.Serialization.Xml/tests/SerializationTypes.cs @@ -47,6 +47,16 @@ public static bool AreEqual(SimpleType x, SimpleType y) return (x.P1 == y.P1) && (x.P2 == y.P2); } } + + public override bool Equals(object? obj) + { + if (obj is SimpleType st) + return AreEqual(this, st); + + return base.Equals(obj); + } + + public override int GetHashCode() => base.GetHashCode(); } public class TypeWithGetSetArrayMembers