From cbc2970d48b3407258410e75a8eb1b5267d82702 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Fri, 10 Sep 2021 02:48:54 -0700 Subject: [PATCH 01/15] Generate dynamic serialization assembly in the appropriate ALC, and don't keep any hard refs to types that could prevent unloading. --- .../src/System.Private.Xml.csproj | 1 + .../System/Xml/Serialization/Compilation.cs | 13 ++- .../Serialization/XmlSerializationWriter.cs | 7 +- .../System/Xml/Serialization/XmlSerializer.cs | 93 +++++++++++-------- 4 files changed, 67 insertions(+), 47 deletions(-) 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..9a1a75c662c831 100644 --- a/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj +++ b/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj @@ -565,6 +565,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..7c432f4d3c799b 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,13 +11,14 @@ using System.Globalization; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; namespace System.Xml.Serialization { internal sealed class TempAssembly { internal const string GeneratedAssemblyNamespace = "Microsoft.Xml.Serialization.GeneratedAssembly"; - private readonly Assembly? _assembly; + internal readonly Assembly? _assembly; private XmlSerializerImplementation? _contract; private IDictionary? _writerMethods; private IDictionary? _readerMethods; @@ -690,7 +691,7 @@ public override int GetHashCode() internal sealed class TempAssemblyCache { - private Dictionary _cache = new Dictionary(); + private ConditionalWeakTable _cache = new ConditionalWeakTable(); internal TempAssembly? this[string? ns, object o] { @@ -710,8 +711,12 @@ internal void Add(string? ns, object o, TempAssembly assembly) TempAssembly? tempAssembly; if (_cache.TryGetValue(key, out tempAssembly) && tempAssembly == assembly) return; - Dictionary _copy = new Dictionary(_cache); // clone - _copy[key] = assembly; + ConditionalWeakTable _copy = new ConditionalWeakTable(); // clone + foreach (KeyValuePair kvp in _cache) + { + _copy.Add(kvp.Key, kvp.Value); + } + _copy.Add(key, assembly); _cache = _copy; } } 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..7048f371c1ff4e 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,13 +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 ConditionalWeakTable s_tableIsTypeDynamic = new ConditionalWeakTable(); // 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]; + s_tableIsTypeDynamic.TryGetValue(type, out object? oIsTypeDynamic); if (oIsTypeDynamic == null) { Assembly assembly = type.Assembly; @@ -1500,7 +1501,7 @@ internal static bool IsTypeDynamic(Type type) } } } - s_tableIsTypeDynamic[type] = oIsTypeDynamic = isTypeDynamic; + s_tableIsTypeDynamic.AddOrUpdate(type, oIsTypeDynamic = 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..189f85dd1cd458 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,7 +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 ConditionalWeakTable> s_xmlSerializerTable = new ConditionalWeakTable>(); protected XmlSerializer() { @@ -235,6 +236,7 @@ public XmlSerializer(Type type, string? defaultNamespace) _tempAssembly = s_cache[defaultNamespace, type]; if (_tempAssembly == null) { + using (AssemblyLoadContext.EnterContextualReflection(type.Assembly)) { XmlSerializerImplementation? contract = null; Assembly? assembly = TempAssembly.LoadGeneratedAssembly(type, defaultNamespace, out contract); @@ -403,7 +405,12 @@ public void Serialize(XmlWriter xmlWriter, object? o, XmlSerializerNamespaces? n } } else - _tempAssembly.InvokeWriter(_mapping, xmlWriter, o, namespaces == null || namespaces.Count == 0 ? DefaultNamespaces : namespaces, encodingStyle, id); + { + using (AssemblyLoadContext.EnterContextualReflection(_tempAssembly._assembly)) + { + _tempAssembly.InvokeWriter(_mapping, xmlWriter, o, namespaces == null || namespaces.Count == 0 ? DefaultNamespaces : namespaces, encodingStyle, id); + } + } } catch (Exception? e) { @@ -501,7 +508,10 @@ private XmlMapping GetMapping() } else { - return _tempAssembly.InvokeReader(_mapping, xmlReader, events, encodingStyle); + using (AssemblyLoadContext.EnterContextualReflection(_tempAssembly._assembly)) + { + return _tempAssembly.InvokeReader(_mapping, xmlReader, events, encodingStyle); + } } } catch (Exception? e) @@ -585,52 +595,55 @@ public static XmlSerializer[] FromMappings(XmlMapping[]? mappings, Type? type) return serializers; } - XmlSerializerImplementation? contract = null; - Assembly? assembly = type == null ? null : TempAssembly.LoadGeneratedAssembly(type, null, out contract); - TempAssembly? tempAssembly = null; - if (assembly == null) + using (AssemblyLoadContext.EnterContextualReflection(type?.Assembly)) { - if (Mode == SerializationMode.PreGenOnly) + XmlSerializerImplementation? contract = null; + Assembly? assembly = type == null ? null : TempAssembly.LoadGeneratedAssembly(type, null, out contract); + TempAssembly? tempAssembly = null; + if (assembly == null) { - AssemblyName name = type!.Assembly.GetName(); - string serializerName = Compiler.GetTempAssemblyName(name, null); - throw new FileLoadException(SR.Format(SR.FailLoadAssemblyUnderPregenMode, serializerName)); - } + if (Mode == SerializationMode.PreGenOnly) + { + AssemblyName name = type!.Assembly.GetName(); + string serializerName = Compiler.GetTempAssemblyName(name, null); + throw new FileLoadException(SR.Format(SR.FailLoadAssemblyUnderPregenMode, serializerName)); + } - if (XmlMapping.IsShallow(mappings)) - { - return Array.Empty(); - } - else - { - if (type == null) + if (XmlMapping.IsShallow(mappings)) + { + return Array.Empty(); + } + else { - tempAssembly = new TempAssembly(mappings, new Type?[] { type }, null, null); - XmlSerializer[] serializers = new XmlSerializer[mappings.Length]; + if (type == null) + { + tempAssembly = new TempAssembly(mappings, new Type?[] { type }, null, null); + XmlSerializer[] serializers = new XmlSerializer[mappings.Length]; + + contract = tempAssembly.Contract; - contract = tempAssembly.Contract; + for (int i = 0; i < serializers.Length; i++) + { + serializers[i] = (XmlSerializer)contract.TypedSerializers[mappings[i].Key!]!; + serializers[i].SetTempAssembly(tempAssembly, mappings[i]); + } - for (int i = 0; i < serializers.Length; i++) + return serializers; + } + else { - serializers[i] = (XmlSerializer)contract.TypedSerializers[mappings[i].Key!]!; - serializers[i].SetTempAssembly(tempAssembly, mappings[i]); + // Use XmlSerializer cache when the type is not null. + return GetSerializersFromCache(mappings, type); } - - return serializers; - } - else - { - // Use XmlSerializer cache when the type is not null. - return GetSerializersFromCache(mappings, type); } } - } - else - { - XmlSerializer[] serializers = new XmlSerializer[mappings.Length]; - for (int i = 0; i < serializers.Length; i++) - serializers[i] = (XmlSerializer)contract!.TypedSerializers[mappings[i].Key!]!; - return serializers; + else + { + XmlSerializer[] serializers = new XmlSerializer[mappings.Length]; + for (int i = 0; i < serializers.Length; i++) + serializers[i] = (XmlSerializer)contract!.TypedSerializers[mappings[i].Key!]!; + return serializers; + } } } @@ -703,7 +716,7 @@ private static XmlSerializer[] GetSerializersFromCache(XmlMapping[] mappings, Ty if (!s_xmlSerializerTable.TryGetValue(type, out typedMappingTable)) { typedMappingTable = new Dictionary(); - s_xmlSerializerTable[type] = typedMappingTable; + s_xmlSerializerTable.Add(type, typedMappingTable); } } From 875788eb7addb34f876dfe0d6dcb549d092302af Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Fri, 10 Sep 2021 04:01:19 -0700 Subject: [PATCH 02/15] Add small test for ALC unloading. --- .../tests/XmlSerializer/XmlSerializerTests.cs | 42 ++++++++++++++++--- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs index c918520a19c8ff..b59b7e9886f3a3 100644 --- a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs +++ b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs @@ -1960,6 +1960,42 @@ public static void Xml_TypeWithSpecialCharacterInStringMember() Assert.Equal(x.Name, y.Name); } + [Fact] + public static void Xml_TypeInCollectibleALC() + { + WeakReference weakRef; + +#if !XMLSERIALIZERGENERATORTESTS + ExecuteAndUnload("System.Collections.Specialized", "System.Collections.Specialized.StringCollection", out weakRef); +#else + ExecuteAndUnload("SerializableAssembly", "SerializationTypes.SimpleType", out weakRef); +#endif + + for (int i = 0; weakRef.IsAlive && i < 10; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + Assert.True(!weakRef.IsAlive); + } + + private static void ExecuteAndUnload(string assemblyname, string typename, out WeakReference wref) + { + var alc = new System.Runtime.Loader.AssemblyLoadContext("XmlSerializerTests", true); + wref = new WeakReference(alc); + var asm = alc.LoadFromAssemblyName(new AssemblyName(assemblyname)); + var type = asm.GetType(typename); + var obj = Activator.CreateInstance(type); + + XmlSerializer serializer = new XmlSerializer(type); + var rto = SerializeAndDeserialize(obj, null, () => serializer, true); + + Assert.NotNull(rto); + + 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 +2116,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()) { From 908e4d1f4e4195054da7e7bedf9106b5ccbb6fee Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Tue, 28 Sep 2021 15:43:13 -0700 Subject: [PATCH 03/15] PR feedback; Move into TempAssembly; Strong/Weak lookup tables; Test refinement --- .../System/Xml/Serialization/Compilation.cs | 285 ++++++++++-------- .../System/Xml/Serialization/XmlSerializer.cs | 155 +++++----- ....XmlSerializer.ReflectionOnly.Tests.csproj | 5 +- .../System.Xml.XmlSerializer.Tests.csproj | 5 +- .../tests/XmlSerializer/XmlSerializerTests.cs | 19 +- 5 files changed, 257 insertions(+), 212 deletions(-) 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 7c432f4d3c799b..ee9e1e875211a9 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 @@ -12,6 +12,7 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; +using System.Runtime.Loader; namespace System.Xml.Serialization { @@ -150,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; } @@ -452,79 +456,83 @@ internal static bool GenerateSerializerToStream(XmlMapping[] xmlMappings, Type?[ [RequiresUnreferencedCode("calls GenerateElement")] internal static Assembly GenerateRefEmitAssembly(XmlMapping[] xmlMappings, Type?[]? types, string? defaultNamespace) { + var mainType = (types != null && types.Length > 0) ? types[0] : null; 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) - { - 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) + using (AssemblyLoadContext.EnterContextualReflection(mainType?.Assembly)) { - suffix = CodeIdentifier.MakeValid(types[0]!.Name); - if (types[0]!.IsArray) + 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( @@ -667,9 +675,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; @@ -691,33 +699,56 @@ public override int GetHashCode() internal sealed class TempAssemblyCache { - private ConditionalWeakTable _cache = new ConditionalWeakTable(); + private Dictionary _cache = 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 (_cache.TryGetValue(key, out tempAssembly)) + return tempAssembly; + + var alc = AssemblyLoadContext.GetLoadContext(t.Assembly); + Dictionary? cache; + + if (alc != null && _collectibleCaches.TryGetValue(alc, out cache)) + cache.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); + var alc = AssemblyLoadContext.GetLoadContext(t.Assembly); + TempAssemblyCacheKey key = new TempAssemblyCacheKey(ns, t); + lock (this) { - TempAssembly? tempAssembly; - if (_cache.TryGetValue(key, out tempAssembly) && tempAssembly == assembly) + TempAssembly? tempAssembly = this[ns, t]; + + if (tempAssembly == assembly) return; - ConditionalWeakTable _copy = new ConditionalWeakTable(); // clone - foreach (KeyValuePair kvp in _cache) + + if (alc != null && alc.IsCollectible) + { + Dictionary? collectibleCache; + if (!_collectibleCaches.TryGetValue(alc, out collectibleCache)) + { + collectibleCache = new Dictionary(); + _collectibleCaches.Add(alc, collectibleCache); + } + + collectibleCache.Add(key, assembly); + } + else { - _copy.Add(kvp.Key, kvp.Value); + _cache.Add(key, assembly); } - _copy.Add(key, assembly); - _cache = _copy; } } } 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 189f85dd1cd458..0175c9a9bf44f1 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 @@ -162,7 +162,8 @@ 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 ConditionalWeakTable> s_xmlSerializerTable = new ConditionalWeakTable>(); + private static readonly Dictionary> s_xmlSerializerTable = new Dictionary>(); + private static readonly ConditionalWeakTable> s_xmlSerializerWeakTable = new ConditionalWeakTable>(); protected XmlSerializer() { @@ -236,31 +237,28 @@ public XmlSerializer(Type type, string? defaultNamespace) _tempAssembly = s_cache[defaultNamespace, type]; if (_tempAssembly == null) { - using (AssemblyLoadContext.EnterContextualReflection(type.Assembly)) + 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) { - 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 - { - // 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); @@ -406,10 +404,7 @@ public void Serialize(XmlWriter xmlWriter, object? o, XmlSerializerNamespaces? n } else { - using (AssemblyLoadContext.EnterContextualReflection(_tempAssembly._assembly)) - { - _tempAssembly.InvokeWriter(_mapping, xmlWriter, o, namespaces == null || namespaces.Count == 0 ? DefaultNamespaces : namespaces, encodingStyle, id); - } + _tempAssembly.InvokeWriter(_mapping, xmlWriter, o, namespaces == null || namespaces.Count == 0 ? DefaultNamespaces : namespaces, encodingStyle, id); } } catch (Exception? e) @@ -508,10 +503,7 @@ private XmlMapping GetMapping() } else { - using (AssemblyLoadContext.EnterContextualReflection(_tempAssembly._assembly)) - { - return _tempAssembly.InvokeReader(_mapping, xmlReader, events, encodingStyle); - } + return _tempAssembly.InvokeReader(_mapping, xmlReader, events, encodingStyle); } } catch (Exception? e) @@ -595,55 +587,52 @@ public static XmlSerializer[] FromMappings(XmlMapping[]? mappings, Type? type) return serializers; } - using (AssemblyLoadContext.EnterContextualReflection(type?.Assembly)) + XmlSerializerImplementation? contract = null; + Assembly? assembly = type == null ? null : TempAssembly.LoadGeneratedAssembly(type, null, out contract); + TempAssembly? tempAssembly = null; + if (assembly == null) { - XmlSerializerImplementation? contract = null; - Assembly? assembly = type == null ? null : TempAssembly.LoadGeneratedAssembly(type, null, out contract); - TempAssembly? tempAssembly = null; - if (assembly == null) + if (Mode == SerializationMode.PreGenOnly) { - if (Mode == SerializationMode.PreGenOnly) - { - AssemblyName name = type!.Assembly.GetName(); - string serializerName = Compiler.GetTempAssemblyName(name, null); - throw new FileLoadException(SR.Format(SR.FailLoadAssemblyUnderPregenMode, serializerName)); - } + AssemblyName name = type!.Assembly.GetName(); + string serializerName = Compiler.GetTempAssemblyName(name, null); + throw new FileLoadException(SR.Format(SR.FailLoadAssemblyUnderPregenMode, serializerName)); + } - if (XmlMapping.IsShallow(mappings)) - { - return Array.Empty(); - } - else + if (XmlMapping.IsShallow(mappings)) + { + return Array.Empty(); + } + else + { + if (type == null) { - if (type == null) - { - tempAssembly = new TempAssembly(mappings, new Type?[] { type }, null, null); - XmlSerializer[] serializers = new XmlSerializer[mappings.Length]; - - contract = tempAssembly.Contract; + tempAssembly = new TempAssembly(mappings, new Type?[] { type }, null, null); + XmlSerializer[] serializers = new XmlSerializer[mappings.Length]; - for (int i = 0; i < serializers.Length; i++) - { - serializers[i] = (XmlSerializer)contract.TypedSerializers[mappings[i].Key!]!; - serializers[i].SetTempAssembly(tempAssembly, mappings[i]); - } + contract = tempAssembly.Contract; - return serializers; - } - else + for (int i = 0; i < serializers.Length; i++) { - // Use XmlSerializer cache when the type is not null. - return GetSerializersFromCache(mappings, type); + serializers[i] = (XmlSerializer)contract.TypedSerializers[mappings[i].Key!]!; + serializers[i].SetTempAssembly(tempAssembly, mappings[i]); } + + return serializers; + } + else + { + // Use XmlSerializer cache when the type is not null. + return GetSerializersFromCache(mappings, type); } } - else - { - XmlSerializer[] serializers = new XmlSerializer[mappings.Length]; - for (int i = 0; i < serializers.Length; i++) - serializers[i] = (XmlSerializer)contract!.TypedSerializers[mappings[i].Key!]!; - return serializers; - } + } + else + { + XmlSerializer[] serializers = new XmlSerializer[mappings.Length]; + for (int i = 0; i < serializers.Length; i++) + serializers[i] = (XmlSerializer)contract!.TypedSerializers[mappings[i].Key!]!; + return serializers; } } @@ -709,14 +698,30 @@ 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) + AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(type.Assembly); + + // Look up in the fast table if not collectible + if (alc == null || !alc.IsCollectible) + { + lock (s_xmlSerializerTable) + { + if (!s_xmlSerializerTable.TryGetValue(type, out typedMappingTable)) + { + typedMappingTable = new Dictionary(); + s_xmlSerializerTable[type] = typedMappingTable; + } + } + } + else // Otherwise, look up in the collectible-friendly table { - if (!s_xmlSerializerTable.TryGetValue(type, out typedMappingTable)) + lock (s_xmlSerializerWeakTable) { - typedMappingTable = new Dictionary(); - s_xmlSerializerTable.Add(type, typedMappingTable); + if (!s_xmlSerializerWeakTable.TryGetValue(type, out typedMappingTable)) + { + typedMappingTable = new Dictionary(); + s_xmlSerializerWeakTable.Add(type, 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..c00abc814f673e 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,12 @@ $(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..ba44fefb9b261a 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,13 @@ $(NetCoreAppCurrent) + + + - + diff --git a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs index b59b7e9886f3a3..7ddefb7e63efad 100644 --- a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs +++ b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs @@ -8,6 +8,7 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.Loader; using System.Text; using System.Threading; using System.Xml; @@ -1965,11 +1966,11 @@ public static void Xml_TypeInCollectibleALC() { WeakReference weakRef; -#if !XMLSERIALIZERGENERATORTESTS - ExecuteAndUnload("System.Collections.Specialized", "System.Collections.Specialized.StringCollection", out weakRef); -#else - ExecuteAndUnload("SerializableAssembly", "SerializationTypes.SimpleType", out weakRef); -#endif +//#if !XMLSERIALIZERGENERATORTESTS +// ExecuteAndUnload("System.Collections.Specialized", "System.Collections.Specialized.StringCollection", out weakRef); +//#else + ExecuteAndUnload("SerializableAssembly.dll", "SerializationTypes.SimpleType", out weakRef); +//#endif for (int i = 0; weakRef.IsAlive && i < 10; i++) { @@ -1979,14 +1980,16 @@ public static void Xml_TypeInCollectibleALC() Assert.True(!weakRef.IsAlive); } - private static void ExecuteAndUnload(string assemblyname, string typename, out WeakReference wref) + private static void ExecuteAndUnload(string assemblyfile, string typename, out WeakReference wref) { - var alc = new System.Runtime.Loader.AssemblyLoadContext("XmlSerializerTests", true); + var alc = new AssemblyLoadContext("XmlSerializerTests", true); wref = new WeakReference(alc); - var asm = alc.LoadFromAssemblyName(new AssemblyName(assemblyname)); + var asm = alc.LoadFromAssemblyPath(Path.GetFullPath(assemblyfile)); var type = asm.GetType(typename); var obj = Activator.CreateInstance(type); + Assert.NotEqual(AssemblyLoadContext.GetLoadContext(type.Assembly), AssemblyLoadContext.Default); + XmlSerializer serializer = new XmlSerializer(type); var rto = SerializeAndDeserialize(obj, null, () => serializer, true); From 3908b89d5fd0b67f83918ea5945ebe0bd127eb55 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Wed, 29 Sep 2021 23:10:05 -0700 Subject: [PATCH 04/15] Refining TempAssembly cache; Reflection-based fix; Test corrections --- .../System/Runtime/Serialization/Utils.cs | 23 +++++++++++ .../System/Xml/Serialization/Compilation.cs | 40 +++++++++---------- .../ReflectionXmlSerializationReader.cs | 9 +++-- .../tests/XmlSerializer/XmlSerializerTests.cs | 33 ++++++++------- .../tests/SerializationTypes.cs | 10 +++++ 5 files changed, 74 insertions(+), 41 deletions(-) diff --git a/src/libraries/Common/tests/System/Runtime/Serialization/Utils.cs b/src/libraries/Common/tests/System/Runtime/Serialization/Utils.cs index ca01381a6dff10..23cb68d4bb08f8 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,24 @@ 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) + { + _resolver = new AssemblyDependencyResolver(mainAssemblyToLoadPath ?? Assembly.GetExecutingAssembly().Location); + } + + protected override Assembly Load(AssemblyName name) + { + string assemblyPath = _resolver.ResolveAssemblyToPath(name); + if (assemblyPath != null) + { + return LoadFromAssemblyPath(assemblyPath); + } + + return null; + } +} 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 ee9e1e875211a9..021d04b1e7d851 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 @@ -19,7 +19,7 @@ namespace System.Xml.Serialization internal sealed class TempAssembly { internal const string GeneratedAssemblyNamespace = "Microsoft.Xml.Serialization.GeneratedAssembly"; - internal readonly Assembly? _assembly; + private readonly Assembly? _assembly; private XmlSerializerImplementation? _contract; private IDictionary? _writerMethods; private IDictionary? _readerMethods; @@ -699,8 +699,8 @@ public override int GetHashCode() internal sealed class TempAssemblyCache { - private Dictionary _cache = new Dictionary(); - private ConditionalWeakTable> _collectibleCaches = new ConditionalWeakTable>(); + private Dictionary _fastCache = new Dictionary(); + private ConditionalWeakTable> _collectibleCaches = new ConditionalWeakTable>(); internal TempAssembly? this[string? ns, Type t] { @@ -709,14 +709,11 @@ internal sealed class TempAssemblyCache TempAssembly? tempAssembly; TempAssemblyCacheKey key = new TempAssemblyCacheKey(ns, t); - if (_cache.TryGetValue(key, out tempAssembly)) + if (_fastCache.TryGetValue(key, out tempAssembly)) return tempAssembly; - var alc = AssemblyLoadContext.GetLoadContext(t.Assembly); - Dictionary? cache; - - if (alc != null && _collectibleCaches.TryGetValue(alc, out cache)) - cache.TryGetValue(key, out tempAssembly); + if (_collectibleCaches.TryGetValue(t.Assembly, out var cCache)) + cCache.TryGetValue(key, out tempAssembly); return tempAssembly; } @@ -724,30 +721,29 @@ internal sealed class TempAssemblyCache internal void Add(string? ns, Type t, TempAssembly assembly) { - var alc = AssemblyLoadContext.GetLoadContext(t.Assembly); - TempAssemblyCacheKey key = new TempAssemblyCacheKey(ns, t); - lock (this) { TempAssembly? tempAssembly = this[ns, t]; - if (tempAssembly == assembly) return; + AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(t.Assembly); + TempAssemblyCacheKey key = new TempAssemblyCacheKey(ns, t); + Dictionary? cache; + if (alc != null && alc.IsCollectible) { - Dictionary? collectibleCache; - if (!_collectibleCaches.TryGetValue(alc, out collectibleCache)) - { - collectibleCache = new Dictionary(); - _collectibleCaches.Add(alc, collectibleCache); - } - - collectibleCache.Add(key, assembly); + 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.Add(key, assembly); + cache = new Dictionary(_fastCache); // Clone + cache[key] = assembly; + _fastCache = cache; } } } 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..b25af7bd77d1a5 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,15 +627,16 @@ 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 ConditionalWeakTable> s_setMemberValueDelegateCache = new ConditionalWeakTable>(); [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); + if (!delegateCacheForType.TryGetValue(memberName, out var result)) { MemberInfo memberInfo = ReflectionXmlSerializationHelper.GetEffectiveSetInfo(o.GetType(), memberName); Debug.Assert(memberInfo != null, "memberInfo could not be retrieved"); @@ -657,7 +658,7 @@ private static ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate Get MethodInfo getSetMemberValueDelegateWithTypeMi = getSetMemberValueDelegateWithTypeGenericMi.MakeGenericMethod(o.GetType(), memberType); var getSetMemberValueDelegateWithType = (Func)getSetMemberValueDelegateWithTypeMi.CreateDelegate(typeof(Func)); result = getSetMemberValueDelegateWithType(memberInfo); - s_setMemberValueDelegateCache.TryAdd(typeMemberNameTuple, result); + delegateCacheForType.TryAdd(memberName, result); } return result; diff --git a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs index 7ddefb7e63efad..2105bd030936d4 100644 --- a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs +++ b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs @@ -8,6 +8,7 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.CompilerServices; using System.Runtime.Loader; using System.Text; using System.Threading; @@ -1964,36 +1965,38 @@ public static void Xml_TypeWithSpecialCharacterInStringMember() [Fact] public static void Xml_TypeInCollectibleALC() { - WeakReference weakRef; - -//#if !XMLSERIALIZERGENERATORTESTS -// ExecuteAndUnload("System.Collections.Specialized", "System.Collections.Specialized.StringCollection", out weakRef); -//#else - ExecuteAndUnload("SerializableAssembly.dll", "SerializationTypes.SimpleType", out weakRef); -//#endif + ExecuteAndUnload("SerializableAssembly.dll", "SerializationTypes.SimpleType", out var weakRef); for (int i = 0; weakRef.IsAlive && i < 10; i++) { GC.Collect(); + GC.WaitForFullGCComplete(); GC.WaitForPendingFinalizers(); } Assert.True(!weakRef.IsAlive); } + [MethodImpl(MethodImplOptions.NoInlining)] private static void ExecuteAndUnload(string assemblyfile, string typename, out WeakReference wref) { - var alc = new AssemblyLoadContext("XmlSerializerTests", true); + var fullPath = Path.GetFullPath(assemblyfile); + var alc = new TestAssemblyLoadContext("XmlSerializerTests", true, fullPath); wref = new WeakReference(alc); - var asm = alc.LoadFromAssemblyPath(Path.GetFullPath(assemblyfile)); - var type = asm.GetType(typename); - var obj = Activator.CreateInstance(type); - Assert.NotEqual(AssemblyLoadContext.GetLoadContext(type.Assembly), AssemblyLoadContext.Default); + // Load assembly by path. By name, and it gets loaded in the default ALC. + var asm = alc.LoadFromAssemblyPath(fullPath); - XmlSerializer serializer = new XmlSerializer(type); - var rto = SerializeAndDeserialize(obj, null, () => serializer, true); + // 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); - Assert.NotNull(rto); + // 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)); alc.Unload(); } 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 From b25a1fe0acf339934985d8c45d7fce47097f909f Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Thu, 30 Sep 2021 15:02:26 -0700 Subject: [PATCH 05/15] Mono/wasm test fixup --- .../Common/tests/System/Runtime/Serialization/Utils.cs | 8 +++++++- .../tests/XmlSerializer/XmlSerializerTests.cs | 6 +++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/tests/System/Runtime/Serialization/Utils.cs b/src/libraries/Common/tests/System/Runtime/Serialization/Utils.cs index 23cb68d4bb08f8..2e2ac299c2c53b 100644 --- a/src/libraries/Common/tests/System/Runtime/Serialization/Utils.cs +++ b/src/libraries/Common/tests/System/Runtime/Serialization/Utils.cs @@ -360,11 +360,17 @@ internal class TestAssemblyLoadContext : AssemblyLoadContext public TestAssemblyLoadContext(string name, bool isCollectible, string mainAssemblyToLoadPath = null) : base(name, isCollectible) { - _resolver = new AssemblyDependencyResolver(mainAssemblyToLoadPath ?? Assembly.GetExecutingAssembly().Location); + 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) { diff --git a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs index 2105bd030936d4..917a59e8a29279 100644 --- a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs +++ b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs @@ -1963,6 +1963,11 @@ public static void Xml_TypeWithSpecialCharacterInStringMember() } [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 public static void Xml_TypeInCollectibleALC() { ExecuteAndUnload("SerializableAssembly.dll", "SerializationTypes.SimpleType", out var weakRef); @@ -1970,7 +1975,6 @@ public static void Xml_TypeInCollectibleALC() for (int i = 0; weakRef.IsAlive && i < 10; i++) { GC.Collect(); - GC.WaitForFullGCComplete(); GC.WaitForPendingFinalizers(); } Assert.True(!weakRef.IsAlive); From 8e58217a0ca69e3f203e8c218367f1af6de88994 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Tue, 12 Oct 2021 09:58:37 -0700 Subject: [PATCH 06/15] Ensure that serializers from XmlMappings don't run afoul of collectible ALCs. --- .../src/Resources/Strings.resx | 3 ++ .../System/Xml/Serialization/Compilation.cs | 31 +++++++++++++++++-- .../System/Xml/Serialization/XmlSerializer.cs | 3 ++ 3 files changed, 34 insertions(+), 3 deletions(-) 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/Xml/Serialization/Compilation.cs b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/Compilation.cs index 021d04b1e7d851..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 @@ -454,17 +454,25 @@ 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 != null && types.Length > 0) ? types[0] : null; + 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); - using (AssemblyLoadContext.EnterContextualReflection(mainType?.Assembly)) + using (AssemblyLoadContext.EnterContextualReflection(mainAssembly)) { + // 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 @@ -597,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) { 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 0175c9a9bf44f1..18f2f29e97785b 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 @@ -631,7 +631,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; } } From 5f1cfcf69f10851da390a1b47035b09cf5c73b4c Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Wed, 20 Oct 2021 16:40:45 -0700 Subject: [PATCH 07/15] PR feedback --- .../ReflectionXmlSerializationReader.cs | 53 +++++++++++-------- 1 file changed, 30 insertions(+), 23 deletions(-) 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 b25af7bd77d1a5..b7cd2ebf0e93ea 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,7 +627,7 @@ private static void AddObjectsIntoTargetCollection(object targetCollection, List } } - private static readonly ConditionalWeakTable> s_setMemberValueDelegateCache = new ConditionalWeakTable>(); + private static readonly ConditionalWeakTable s_setMemberValueDelegateCache = new ConditionalWeakTable(); [RequiresUnreferencedCode(XmlSerializer.TrimSerializationWarning)] private static ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate GetSetMemberValueDelegate(object o, string memberName) @@ -635,33 +635,40 @@ private static ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate Get Debug.Assert(o != null, "Object o should not be null"); Debug.Assert(!string.IsNullOrEmpty(memberName), "memberName must have a value"); Type type = o.GetType(); - var delegateCacheForType = s_setMemberValueDelegateCache.GetOrCreateValue(type); - if (!delegateCacheForType.TryGetValue(memberName, out var result)) + var delegateCacheForType = s_setMemberValueDelegateCache.GetValue(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); - delegateCacheForType.TryAdd(memberName, 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) From 4e83ee5a4506189b4f67cc18931042703cee5699 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Wed, 20 Oct 2021 16:54:24 -0700 Subject: [PATCH 08/15] Unloadable contexts not yet supported in Mono. --- .../System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs index 917a59e8a29279..3da4923bac5045 100644 --- a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs +++ b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs @@ -1968,6 +1968,7 @@ public static void Xml_TypeWithSpecialCharacterInStringMember() // 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); From 7a2861acb806c302582a7b97c08d24e851058102 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Thu, 21 Oct 2021 23:07:15 -0700 Subject: [PATCH 09/15] Fix trimming of types for XmlSerializer tests that got moved out of main test assembly. --- .../System.Xml.XmlSerializer.ReflectionOnly.Tests.csproj | 1 + .../tests/XmlSerializer/System.Xml.XmlSerializer.Tests.csproj | 1 + 2 files changed, 2 insertions(+) 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 c00abc814f673e..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 @@ -5,6 +5,7 @@ + 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 ba44fefb9b261a..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 @@ -4,6 +4,7 @@ + From 38b7db8c1d0ca08bc9af94e384de5c18799a779a Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Thu, 28 Oct 2021 10:29:00 -0700 Subject: [PATCH 10/15] Encapsulating the duality of dealing with unloadable ALC instead of rewriting similar code everywhere. --- .../src/System.Private.Xml.csproj | 1 + .../ReflectionXmlSerializationReader.cs | 4 +-- .../Serialization/XmlSerializationWriter.cs | 9 +++--- .../System/Xml/Serialization/XmlSerializer.cs | 28 ++----------------- 4 files changed, 9 insertions(+), 33 deletions(-) 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 9a1a75c662c831..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 @@ + 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 b7cd2ebf0e93ea..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,7 +627,7 @@ private static void AddObjectsIntoTargetCollection(object targetCollection, List } } - private static readonly ConditionalWeakTable s_setMemberValueDelegateCache = new ConditionalWeakTable(); + private static readonly ContextAwareTables s_setMemberValueDelegateCache = new ContextAwareTables(); [RequiresUnreferencedCode(XmlSerializer.TrimSerializationWarning)] private static ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate GetSetMemberValueDelegate(object o, string memberName) @@ -635,7 +635,7 @@ private static ReflectionXmlSerializationReaderHelper.SetMemberValueDelegate Get Debug.Assert(o != null, "Object o should not be null"); Debug.Assert(!string.IsNullOrEmpty(memberName), "memberName must have a value"); Type type = o.GetType(); - var delegateCacheForType = s_setMemberValueDelegateCache.GetValue(type, _ => new Hashtable()); + var delegateCacheForType = s_setMemberValueDelegateCache.GetOrCreateValue(type, () => new Hashtable()); var result = delegateCacheForType[memberName]; if (result == null) { 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 7048f371c1ff4e..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 @@ -1466,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 ConditionalWeakTable s_tableIsTypeDynamic = new ConditionalWeakTable(); + 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) { - s_tableIsTypeDynamic.TryGetValue(type, out object? oIsTypeDynamic); - if (oIsTypeDynamic == null) + object oIsTypeDynamic = s_tableIsTypeDynamic.GetOrCreateValue(type, () => { Assembly assembly = type.Assembly; bool isTypeDynamic = assembly.IsDynamic /*|| string.IsNullOrEmpty(assembly.Location)*/; @@ -1501,8 +1500,8 @@ internal static bool IsTypeDynamic(Type type) } } } - s_tableIsTypeDynamic.AddOrUpdate(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 18f2f29e97785b..0428b863df7320 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 @@ -162,9 +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 ConditionalWeakTable> s_xmlSerializerWeakTable = new ConditionalWeakTable>(); - + private static readonly ContextAwareTables> s_xmlSerializerTable = new ContextAwareTables>(); protected XmlSerializer() { } @@ -704,29 +702,7 @@ private static XmlSerializer[] GetSerializersFromCache(XmlMapping[] mappings, Ty Dictionary? typedMappingTable = null; AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(type.Assembly); - // Look up in the fast table if not collectible - if (alc == null || !alc.IsCollectible) - { - lock (s_xmlSerializerTable) - { - if (!s_xmlSerializerTable.TryGetValue(type, out typedMappingTable)) - { - typedMappingTable = new Dictionary(); - s_xmlSerializerTable[type] = typedMappingTable; - } - } - } - else // Otherwise, look up in the collectible-friendly table - { - lock (s_xmlSerializerWeakTable) - { - if (!s_xmlSerializerWeakTable.TryGetValue(type, out typedMappingTable)) - { - typedMappingTable = new Dictionary(); - s_xmlSerializerWeakTable.Add(type, typedMappingTable); - } - } - } + typedMappingTable = s_xmlSerializerTable.GetOrCreateValue(type, () => new Dictionary()); lock (typedMappingTable) { From cc07389b65517e48087b5eb52873071bb9554b67 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Thu, 28 Oct 2021 10:46:24 -0700 Subject: [PATCH 11/15] Missed new file in last commit. --- .../Xml/Serialization/ContextAwareTables.cs | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 src/libraries/System.Private.Xml/src/System/Xml/Serialization/ContextAwareTables.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 new file mode 100644 index 00000000000000..05643577846c4c --- /dev/null +++ b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ContextAwareTables.cs @@ -0,0 +1,62 @@ +// 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.Runtime.CompilerServices; + using System.Runtime.Loader; + + internal class ContextAwareTables where T : class? + { + private Hashtable _defaultTable; + private ConditionalWeakTable _collectibleTable; + + public ContextAwareTables() + { + _defaultTable = new Hashtable(); + _collectibleTable = new ConditionalWeakTable(); + } + + internal T GetOrCreateValue(Type t, Func f) + { + T? ret; + AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(t.Assembly); + + // Null and non-collectible load contexts use the default table + if (alc == null || !alc.IsCollectible) + { + if ((ret = (T?)_defaultTable[t]) == null) + { + 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 + { + if (!_collectibleTable.TryGetValue(t, out ret)) + { + lock (_collectibleTable) + { + if (!_collectibleTable.TryGetValue(t, out ret)) + { + ret = f(); + _collectibleTable.AddOrUpdate(t, ret); + } + } + } + } + + return ret; + } + } +} From cc0e5f7160b472e46dd099cc9fdf86b8ee61f676 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Thu, 28 Oct 2021 12:20:26 -0700 Subject: [PATCH 12/15] Compilation bug not seen locally. --- .../src/System/Xml/Serialization/ContextAwareTables.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 index 05643577846c4c..8f5a621f43e8cc 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ContextAwareTables.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ContextAwareTables.cs @@ -5,10 +5,11 @@ namespace System.Xml.Serialization { using System; using System.Collections; + using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.Loader; - internal class ContextAwareTables where T : class? + internal class ContextAwareTables<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T> where T : class? { private Hashtable _defaultTable; private ConditionalWeakTable _collectibleTable; From c95a78985148e595ecbe83107ae33ea233f4d56a Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Thu, 28 Oct 2021 17:24:35 -0700 Subject: [PATCH 13/15] Perf improvement. --- .../Xml/Serialization/ContextAwareTables.cs | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) 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 index 8f5a621f43e8cc..64e09bbc063b2c 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ContextAwareTables.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/ContextAwareTables.cs @@ -22,21 +22,27 @@ public ContextAwareTables() internal T GetOrCreateValue(Type t, Func f) { - T? ret; + // 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) { - if ((ret = (T?)_defaultTable[t]) == null) + lock (_defaultTable) { - lock (_defaultTable) + if ((ret = (T?)_defaultTable[t]) == null) { - if ((ret = (T?)_defaultTable[t]) == null) - { - ret = f(); - _defaultTable[t] = ret; - } + ret = f(); + _defaultTable[t] = ret; } } } @@ -44,15 +50,12 @@ internal T GetOrCreateValue(Type t, Func f) // Collectible load contexts should use the ConditionalWeakTable so they can be unloaded else { - if (!_collectibleTable.TryGetValue(t, out ret)) + lock (_collectibleTable) { - lock (_collectibleTable) + if (!_collectibleTable.TryGetValue(t, out ret)) { - if (!_collectibleTable.TryGetValue(t, out ret)) - { - ret = f(); - _collectibleTable.AddOrUpdate(t, ret); - } + ret = f(); + _collectibleTable.AddOrUpdate(t, ret); } } } From 58c1353a9be0120fcb298485ef56bd0531bb1c6a Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Fri, 12 Nov 2021 18:41:01 -0800 Subject: [PATCH 14/15] Remove extraneous line. Test feedback. --- .../src/System/Xml/Serialization/XmlSerializer.cs | 1 - .../System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) 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 0428b863df7320..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 @@ -700,7 +700,6 @@ private static XmlSerializer[] GetSerializersFromCache(XmlMapping[] mappings, Ty { XmlSerializer?[] serializers = new XmlSerializer?[mappings.Length]; Dictionary? typedMappingTable = null; - AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(type.Assembly); typedMappingTable = s_xmlSerializerTable.GetOrCreateValue(type, () => new Dictionary()); diff --git a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs index 3da4923bac5045..9fefff2137faa5 100644 --- a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs +++ b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs @@ -2002,6 +2002,7 @@ private static void ExecuteAndUnload(string assemblyfile, string typename, out W 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(); } From 65d317b458f1acd5534c9541efb8c8a9f41b2798 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 10 Sep 2021 19:47:24 +0200 Subject: [PATCH 15/15] 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) --- src/coreclr/vm/appdomain.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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;