diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/CodeGenerator.cs b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/CodeGenerator.cs index 7b432dad15380f..8fed6c4cf7b69f 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/Serialization/CodeGenerator.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/Serialization/CodeGenerator.cs @@ -1276,12 +1276,14 @@ private IfState PopIfState() } [RequiresDynamicCode(XmlSerializer.AotSerializationWarning)] - internal static AssemblyBuilder CreateAssemblyBuilder(string name) + internal static AssemblyBuilder CreateAssemblyBuilder(string name, bool collectible) { AssemblyName assemblyName = new AssemblyName(); assemblyName.Name = name; assemblyName.Version = new Version(1, 0, 0, 0); - return AssemblyBuilder.DefineDynamicAssembly(assemblyName, AssemblyBuilderAccess.Run); + return AssemblyBuilder.DefineDynamicAssembly( + assemblyName, + collectible ? AssemblyBuilderAccess.RunAndCollect : AssemblyBuilderAccess.Run); } internal static ModuleBuilder CreateModuleBuilder(AssemblyBuilder assemblyBuilder, string name) 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 4438f36172250c..57087cdb7d0d11 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 @@ -3,6 +3,7 @@ using System.Collections; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.IO; @@ -433,6 +434,32 @@ internal static bool GenerateSerializerToStream(XmlMapping[] xmlMappings, Type?[ return true; } + internal static Assembly? FindCollectibleAssembly(Type t) + { + // Shortcut the common case + if (!t.IsCollectible) + return null; + + if (t.Assembly.IsCollectible) + return t.Assembly; + + if (t.IsGenericType && !t.IsGenericTypeDefinition) + { + foreach (Type arg in t.GenericTypeArguments) + { + Assembly? found = FindCollectibleAssembly(arg); + if (found is not null) + return found; + } + } + else if (t.HasElementType) + { + return FindCollectibleAssembly(t.GetElementType()!); + } + + return null; + } + [RequiresUnreferencedCode("calls GenerateElement")] [RequiresDynamicCode(XmlSerializer.AotSerializationWarning)] internal static Assembly GenerateRefEmitAssembly(XmlMapping[] xmlMappings, Type?[] types) @@ -445,17 +472,37 @@ internal static Assembly GenerateRefEmitAssembly(XmlMapping[] xmlMappings, Type? TypeScope[] scopes = new TypeScope[scopeTable.Keys.Count]; scopeTable.Keys.CopyTo(scopes, 0); - using (AssemblyLoadContext.EnterContextualReflection(mainAssembly)) + // Make sure we enter the correct ALC. If we have any collectible types, we should enter + // the ALC of those types. The mainType's assembly might not satisfy that requirement. + Assembly? collectibleAssembly = null; + foreach (var t in types) + { + if (t?.IsCollectible == true) + { + collectibleAssembly = FindCollectibleAssembly(t); + Debug.Assert(collectibleAssembly != null); + break; + } + } + + // Validate collectible types against the same assembly whose ALC we enter below, so + // that a collectible type combined with a non-collectible mainAssembly is checked + // against the collectible context rather than the default one. + Assembly? contextAssembly = collectibleAssembly ?? mainAssembly; + + using (AssemblyLoadContext.EnterContextualReflection(contextAssembly)) { // 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); + VerifyLoadContext(types[i], contextAssembly); foreach (var mapping in xmlMappings) - VerifyLoadContext(mapping.Accessor.Mapping?.TypeDesc?.Type, mainAssembly); + VerifyLoadContext(mapping.Accessor.Mapping?.TypeDesc?.Type, contextAssembly); string assemblyName = "Microsoft.GeneratedCode"; - AssemblyBuilder assemblyBuilder = CodeGenerator.CreateAssemblyBuilder(assemblyName); + AssemblyBuilder assemblyBuilder = CodeGenerator.CreateAssemblyBuilder( + assemblyName, + collectible: collectibleAssembly is not null); // Add AssemblyVersion attribute to match parent assembly version if (mainType != null) { @@ -702,7 +749,8 @@ internal sealed class TempAssemblyCache if (_fastCache.TryGetValue(key, out tempAssembly)) return tempAssembly; - if (_collectibleCaches.TryGetValue(t.Assembly, out var cCache)) + Assembly lookupAssembly = TempAssembly.FindCollectibleAssembly(t) ?? t.Assembly; + if (_collectibleCaches.TryGetValue(lookupAssembly, out var cCache)) cCache.TryGetValue(key, out tempAssembly); return tempAssembly; @@ -717,17 +765,22 @@ internal void Add(string? ns, Type t, TempAssembly assembly) if (tempAssembly == assembly) return; - AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(t.Assembly); TempAssemblyCacheKey key = new TempAssemblyCacheKey(ns, t); Dictionary? cache; - if (alc != null && alc.IsCollectible) + // Use the collectible cache for any collectible type so that the cache entry + // can be released when the collectible ALC is unloaded. For generic types like + // List where Bar is collectible, t.Assembly is the default ALC assembly + // (System.Collections), so we need to find the actual collectible assembly from + // the type's generic arguments or element type. + Assembly? collectibleAssembly = TempAssembly.FindCollectibleAssembly(t); + if (collectibleAssembly != null) { - cache = _collectibleCaches.TryGetValue(t.Assembly, out var c) // Clone or create + cache = _collectibleCaches.TryGetValue(collectibleAssembly, out var c) ? new Dictionary(c) : new Dictionary(); cache[key] = assembly; - _collectibleCaches.AddOrUpdate(t.Assembly, cache); + _collectibleCaches.AddOrUpdate(collectibleAssembly, cache); } else { 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 3fedcb0d77bf9c..cab5a2351ec20e 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 @@ -34,8 +34,8 @@ internal T GetOrCreateValue(Type t, Func f) // 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) + // Use the default table for types that are not collectible and whose load context is either null or not collectible + if (!t.IsCollectible && !(alc?.IsCollectible ?? false)) { lock (_defaultTable) { @@ -47,7 +47,7 @@ internal T GetOrCreateValue(Type t, Func f) } } - // Collectible load contexts should use the ConditionalWeakTable so they can be unloaded + // Collectible types or load contexts should use the ConditionalWeakTable so they can be unloaded else { lock (_collectibleTable) diff --git a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs index 5a7b42eb092487..8a92a06ffd317a 100644 --- a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs +++ b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs @@ -10,6 +10,7 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Reflection.Emit; using System.Runtime.CompilerServices; using System.Runtime.Loader; using System.Runtime.Serialization.Tests; @@ -2620,6 +2621,46 @@ public static void Xml_TypeInCollectibleALC() Assert.True(!weakRef.IsAlive); } +#if !XMLSERIALIZERGENERATORTESTS + // The Generator tests only cover pre-generated serializers that are based on the types in SerializableAssembly.dll. + // For this test, we're testing a scenario where one of those types is used inside a generic container - aka, List. + // Since 'List<>' is *not* defined in SerializableAssembly.dll, the pre-generated serializers do not include serializers for List, + // Therefore, this test is excluded from the Generator tests. + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.HasAssemblyFiles))] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/95928", typeof(PlatformDetection), nameof(PlatformDetection.IsReadyToRunCompiled))] + [ActiveIssue("https://github.com/dotnet/runtime/issues/124344", typeof(PlatformDetection), nameof(PlatformDetection.IsAppleMobile), nameof(PlatformDetection.IsCoreCLR))] + [InlineData("List")] + [InlineData("Array")] + public static void Xml_CollectibleTypeInGenericContainer(string containerKind) + { + ExecuteCollectibleContainerAndUnload("SerializableAssembly.dll", "SerializationTypes.SimpleType", containerKind, out var weakRef); + + for (int i = 0; weakRef.IsAlive && i < 10; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + Assert.True(!weakRef.IsAlive); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsReflectionEmitSupported))] + // Mono doesn't support collectible assemblies: RuntimeType.IsCollectible always returns false, + // so the serializer never engages its collectible weak-caching path. See dotnet/runtime#34072. + [ActiveIssue("https://github.com/dotnet/runtime/issues/34072", TestRuntimes.Mono)] + public static void Xml_RunAndCollectType_DoesNotRemainCached() + { + ExecuteRunAndCollectAndRelease(out WeakReference weakRef); + + for (int i = 0; weakRef.IsAlive && i < 10; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + Assert.True(!weakRef.IsAlive); + } +#endif + [Fact] public static void ValidateXElement() { @@ -2796,6 +2837,150 @@ private static void ExecuteAndUnload(string assemblyfile, string typename, out W alc.Unload(); } + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ExecuteCollectibleContainerAndUnload(string assemblyfile, string typename, string containerKind, out WeakReference wref) + { + var fullPath = Path.GetFullPath(assemblyfile); + var alc = new TestAssemblyLoadContext("XmlSerializerTests", true, fullPath); + wref = new WeakReference(alc); + + var asm = alc.LoadFromAssemblyPath(fullPath); + var baseType = asm.GetType(typename); + Assert.Equal(alc, AssemblyLoadContext.GetLoadContext(baseType.Assembly)); + Assert.NotEqual(alc, AssemblyLoadContext.Default); + + Type type; + object obj; + switch (containerKind) + { + case "List": + type = typeof(List<>).MakeGenericType(baseType); + // The type is collectible because it has a collectible type argument, but its + // Assembly is in the default ALC since that's where List<> is defined. + Assert.True(type.IsCollectible); + Assert.Equal(AssemblyLoadContext.Default, AssemblyLoadContext.GetLoadContext(type.Assembly)); + + obj = Activator.CreateInstance(type); + object listItem = Activator.CreateInstance(baseType); + baseType.GetProperty("P1").SetValue(listItem, "test"); + baseType.GetProperty("P2").SetValue(listItem, 42); + type.GetMethod("Add").Invoke(obj, [listItem]); + break; + + case "Array": + type = baseType.MakeArrayType(); + Assert.True(type.IsCollectible); + + var array = Array.CreateInstance(baseType, 1); + object arrayItem = Activator.CreateInstance(baseType); + baseType.GetProperty("P1").SetValue(arrayItem, "test"); + baseType.GetProperty("P2").SetValue(arrayItem, 42); + array.SetValue(arrayItem, 0); + obj = array; + break; + + default: + throw new ArgumentException($"Unknown container kind: {containerKind}"); + } + + XmlSerializer serializer = new XmlSerializer(type); + using var ms = new MemoryStream(); + serializer.Serialize(ms, obj); + ms.Position = 0; + var rtobj = serializer.Deserialize(ms); + Assert.NotNull(rtobj); + + if (rtobj is Array rtArray) + Assert.Equal(1, rtArray.Length); + else if (rtobj is ICollection rtCollection) + Assert.Equal(1, rtCollection.Count); + else + Assert.Fail($"Expected deserialized object to be an Array or ICollection, but got {rtobj.GetType()}."); + + alc.Unload(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ExecuteRunAndCollectAndRelease(out WeakReference wref) + { + string uniqueName = "XmlSerializerTests.RunAndCollect.Type_" + Guid.NewGuid().ToString("N"); + var assemblyName = new AssemblyName(uniqueName); + AssemblyBuilder assemblyBuilder = AssemblyBuilder.DefineDynamicAssembly( + assemblyName, + AssemblyBuilderAccess.RunAndCollect); + + ModuleBuilder moduleBuilder = assemblyBuilder.DefineDynamicModule(uniqueName); + TypeBuilder typeBuilder = moduleBuilder.DefineType( + uniqueName + ".SimpleType", + TypeAttributes.Public | TypeAttributes.Class); + + typeBuilder.DefineDefaultConstructor(MethodAttributes.Public); + DefineAutoProperty(typeBuilder, "P1", typeof(string)); + DefineAutoProperty(typeBuilder, "P2", typeof(int)); + + Type type = typeBuilder.CreateTypeInfo()!.AsType(); + + Assert.True(type.IsCollectible); + Assert.True(type.Assembly.IsCollectible); + + wref = new WeakReference(type.Assembly); + + object obj = Activator.CreateInstance(type)!; + type.GetProperty("P1")!.SetValue(obj, "test"); + type.GetProperty("P2")!.SetValue(obj, 42); + + XmlSerializer serializer = new XmlSerializer(type); + using var ms = new MemoryStream(); + serializer.Serialize(ms, obj); + + ms.Position = 0; + object? rtobj = serializer.Deserialize(ms); + Assert.NotNull(rtobj); + Assert.Equal("test", type.GetProperty("P1")!.GetValue(rtobj)); + Assert.Equal(42, type.GetProperty("P2")!.GetValue(rtobj)); + } + + private static void DefineAutoProperty(TypeBuilder typeBuilder, string name, Type propertyType) + { + FieldBuilder field = typeBuilder.DefineField("_" + name, propertyType, FieldAttributes.Private); + + MethodAttributes methodAttributes = + MethodAttributes.Public | + MethodAttributes.SpecialName | + MethodAttributes.HideBySig; + + MethodBuilder getter = typeBuilder.DefineMethod( + "get_" + name, + methodAttributes, + propertyType, + Type.EmptyTypes); + + ILGenerator getterIl = getter.GetILGenerator(); + getterIl.Emit(OpCodes.Ldarg_0); + getterIl.Emit(OpCodes.Ldfld, field); + getterIl.Emit(OpCodes.Ret); + + MethodBuilder setter = typeBuilder.DefineMethod( + "set_" + name, + methodAttributes, + null, + new[] { propertyType }); + + ILGenerator setterIl = setter.GetILGenerator(); + setterIl.Emit(OpCodes.Ldarg_0); + setterIl.Emit(OpCodes.Ldarg_1); + setterIl.Emit(OpCodes.Stfld, field); + setterIl.Emit(OpCodes.Ret); + + PropertyBuilder property = typeBuilder.DefineProperty( + name, + PropertyAttributes.None, + propertyType, + null); + + property.SetGetMethod(getter); + property.SetSetMethod(setter); + } 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)