From 4647d16b39205bf6fba8557a667ba81c910e8987 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Fri, 10 Sep 2021 02:48:54 -0700 Subject: [PATCH 01/13] 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 619a6c4fbb715..13969e02c0c06 100644 --- a/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj +++ b/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj @@ -564,6 +564,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 8fdeed7b60b77..7c432f4d3c799 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 46877f900e902..7048f371c1ff4 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 20f9d18343cbb..189f85dd1cd45 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 55eb88b961e2c6d1cca9b43d4476284ff86c576b Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Fri, 10 Sep 2021 04:01:19 -0700 Subject: [PATCH 02/13] 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 c918520a19c8f..b59b7e9886f3a 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 d9e96dcc37d45133204889e49af64a20af6d7e48 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Tue, 28 Sep 2021 15:43:13 -0700 Subject: [PATCH 03/13] 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 7c432f4d3c799..ee9e1e875211a 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 189f85dd1cd45..0175c9a9bf44f 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 53abcbd395702..c00abc814f673 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 dbc8447b87c1c..ba44fefb9b261 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 b59b7e9886f3a..7ddefb7e63efa 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 b6f110a7c339fa5421d496c501f44aa518326c35 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Wed, 29 Sep 2021 23:10:05 -0700 Subject: [PATCH 04/13] 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 ca01381a6dff1..23cb68d4bb08f 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 ee9e1e875211a..021d04b1e7d85 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 0c55638a27a66..b25af7bd77d1a 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 7ddefb7e63efa..2105bd030936d 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 bc10a370b033c..747f661129a94 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 8616e2934a3fc6a6fdaec9cf3bc99c6ce0354d27 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Thu, 30 Sep 2021 15:02:26 -0700 Subject: [PATCH 05/13] 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 23cb68d4bb08f..2e2ac299c2c53 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 2105bd030936d..917a59e8a2927 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 88778c466d3a47e8effccc8179308b8bf53cda74 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Tue, 12 Oct 2021 09:58:37 -0700 Subject: [PATCH 06/13] 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 d19dc6ec4eb31..112359dfecba8 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 021d04b1e7d85..c3df77a5a2bc3 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 0175c9a9bf44f..18f2f29e97785 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 15b7517040600719770019f9f93d8ba17065fbab Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Wed, 20 Oct 2021 16:40:45 -0700 Subject: [PATCH 07/13] 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 b25af7bd77d1a..b7cd2ebf0e93e 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 2c8216538de69a830112415a390712026c8c5d81 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Wed, 20 Oct 2021 16:54:24 -0700 Subject: [PATCH 08/13] 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 917a59e8a2927..3da4923bac504 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 da3c8c113381c6c06fc3dfde940780669f8c499b Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Thu, 21 Oct 2021 23:07:15 -0700 Subject: [PATCH 09/13] 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 c00abc814f673..d7d1f3af3bb87 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 ba44fefb9b261..7818cd132825c 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 0d37491bde5ce903976aec61ba45eb9334bad610 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Thu, 28 Oct 2021 10:29:00 -0700 Subject: [PATCH 10/13] 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 13969e02c0c06..142abf9f9ccb0 100644 --- a/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj +++ b/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj @@ -445,6 +445,7 @@ + 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 b7cd2ebf0e93e..2477f283b01f1 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 7048f371c1ff4..7ebc4462dfb7a 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 18f2f29e97785..0428b863df732 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 6ab0bcd5678fdd20974fb7b24b87894441e01617 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Thu, 28 Oct 2021 10:46:24 -0700 Subject: [PATCH 11/13] 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 0000000000000..05643577846c4 --- /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 f160c1b9ad3b18ad38dec1f63931355e31affbfa Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Thu, 28 Oct 2021 12:20:26 -0700 Subject: [PATCH 12/13] 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 05643577846c4..8f5a621f43e8c 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 454a9fee80f2b3f3b67a6a906841245179e12ef8 Mon Sep 17 00:00:00 2001 From: Steve Molloy Date: Thu, 28 Oct 2021 17:24:35 -0700 Subject: [PATCH 13/13] 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 8f5a621f43e8c..64e09bbc063b2 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); } } }