Enhance collectible assembly handling in XmlSerializer (#127695)

This pull request improves the handling of collectible assemblies and
types within the XML serialization infrastructure, particularly when
dealing with generic containers (like `List<T>` or arrays) that may
contain collectible types. The changes ensure that serializers and
related caches correctly associate with the appropriate collectible
AssemblyLoadContext (ALC), preventing memory leaks and enabling proper
unloading of collectible assemblies. Additionally, new tests have been
added to verify these scenarios.

**Improvements to collectible assembly handling:**

* Added the `FindCollectibleAssembly` helper method in `Compilation.cs`
to recursively locate the correct collectible assembly for a given type,
including generic arguments and element types. This ensures that
serializer generation and caching are tied to the correct ALC,
especially for generic or array types whose main assembly may not be
collectible.
* Updated serializer generation (`GenerateRefEmitAssembly`), cache
lookup (`TempAssemblyCache.GetTempAssembly`), and cache insertion
(`TempAssemblyCache.Add`) logic to use the collectible assembly found
via `FindCollectibleAssembly`, rather than relying solely on
`t.Assembly`. This change ensures that collectible types within generic
containers are handled correctly and can be unloaded.
[[1]](diffhunk://#diff-e4d354039ba8ec739e290407b136c6af32d4d38e07fa9420078f662590ea0da2L448-R488)
[[2]](diffhunk://#diff-e4d354039ba8ec739e290407b136c6af32d4d38e07fa9420078f662590ea0da2L705-R746)
[[3]](diffhunk://#diff-e4d354039ba8ec739e290407b136c6af32d4d38e07fa9420078f662590ea0da2L720-R776)

**Improvements to context-aware tables:**

* Adjusted logic in `ContextAwareTables.GetOrCreateValue` to use the
default table only for non-collectible types and non-collectible load
contexts, and to use a `ConditionalWeakTable` for collectible types or
collectible load contexts, improving resource management for collectible
scenarios.
[[1]](diffhunk://#diff-ddd028dcb1c03a409ec83ec7247b1c4793ebbee56e379a0dab16696fedf1503dL37-R38)
[[2]](diffhunk://#diff-ddd028dcb1c03a409ec83ec7247b1c4793ebbee56e379a0dab16696fedf1503dL50-R50)

**Testing enhancements:**

* Added new tests (`Xml_CollectibleTypeInGenericContainer`) to verify
that collectible types inside generic containers (like `List<T>` or
arrays) are correctly handled and can be unloaded, preventing memory
leaks. This includes a helper method
`ExecuteCollectibleContainerAndUnload` that exercises these scenarios.
[[1]](diffhunk://#diff-cb25d8cca02e9c7959514c29f15823cfbdcae1b51cd148ddc69e2a8a6fb780e4R2623-R2646)
[[2]](diffhunk://#diff-cb25d8cca02e9c7959514c29f15823cfbdcae1b51cd148ddc69e2a8a6fb780e4R2823-R2883)

**Minor changes:**

* Added a missing `using System.Diagnostics;` directive to support new
assertions.

These changes collectively improve the reliability and correctness of
XML serialization in scenarios involving collectible assemblies and
types, especially when used in generic containers.

Fixes #100518

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Author: StephenMolloy

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;
@@ -2701,6 +2702,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()
     {
@@ -2877,6 +2918,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)