Enhance collectible assembly handling in XmlSerializer

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)

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<TempAssemblyCacheKey, TempAssembly>? 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<Bar> 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<TempAssemblyCacheKey, TempAssembly>(c)
                         : new Dictionary<TempAssemblyCacheKey, TempAssembly>();
                     cache[key] = assembly;
-                    _collectibleCaches.AddOrUpdate(t.Assembly, cache);
+                    _collectibleCaches.AddOrUpdate(collectibleAssembly, cache);
                 }
                 else
                 {

src/libraries/System.Private.Xml/src/System/Xml/Serialization/ContextAwareTables.cs

@@ -34,8 +34,8 @@ internal T GetOrCreateValue(Type t, Func<Type, T> 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<Type, T> 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)

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<SerializationType>.
+    // Since 'List<>' is *not* defined in SerializableAssembly.dll, the pre-generated serializers do not include serializers for List<SerializationType>,
+    // 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<T>(object requestBodyValue, string memberName, string baseline, bool skipStringCompare = false, string wrapperName = null)