From 4d0ea89a09d582f41a09aad62e44a7a866648cee Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 07:25:03 +0000 Subject: [PATCH 1/2] Initial plan From 4d65b363083d912517552bbab035da2ad492c17c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 08:17:15 +0000 Subject: [PATCH 2/2] Fix XmlSerializer collectible ALC: support inverted scenario where serializer created inside collectible ALC Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/1c148b1e-1a19-45fb-93a5-8eb9a309241f Co-authored-by: StephenMolloy <19562826+StephenMolloy@users.noreply.github.com> --- .../System/Xml/Serialization/CodeGenerator.cs | 8 +- .../System/Xml/Serialization/Compilation.cs | 99 ++++++++++++++-- .../Xml/Serialization/ContextAwareTables.cs | 73 ++++++++++-- .../tests/XmlSerializer/XmlSerializerTests.cs | 112 ++++++++++++++++++ 4 files changed, 271 insertions(+), 21 deletions(-) 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..3b9a0a6a9e7107 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,16 @@ private IfState PopIfState() } [RequiresDynamicCode(XmlSerializer.AotSerializationWarning)] - internal static AssemblyBuilder CreateAssemblyBuilder(string name) + internal static AssemblyBuilder CreateAssemblyBuilder(string name, bool collectible = false) { AssemblyName assemblyName = new AssemblyName(); assemblyName.Name = name; assemblyName.Version = new Version(1, 0, 0, 0); - return AssemblyBuilder.DefineDynamicAssembly(assemblyName, AssemblyBuilderAccess.Run); + // Use RunAndCollect when creating a collectible assembly (e.g. for types in a collectible ALC). + // The CurrentContextualReflectionContext must already be set to the target collectible ALC + // before calling this method, so that DefineDynamicAssembly places the assembly there. + 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..d3e9b066e703d3 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 @@ -445,7 +445,57 @@ internal static Assembly GenerateRefEmitAssembly(XmlMapping[] xmlMappings, Type? TypeScope[] scopes = new TypeScope[scopeTable.Keys.Count]; scopeTable.Keys.CopyTo(scopes, 0); - using (AssemblyLoadContext.EnterContextualReflection(mainAssembly)) + // Capture the caller's ALC before EnterContextualReflection may overwrite it. + // This handles the "inverted" scenario where code runs within a collectible ALC + // and creates a serializer for a type that may appear to be in the default ALC. + AssemblyLoadContext? callerAlc = AssemblyLoadContext.CurrentContextualReflectionContext; + + // Determine if any type involved requires a collectible dynamic assembly. + AssemblyLoadContext? collectibleAlc = null; + + // Check the top-level types passed in + foreach (Type? t in types) + { + if (t == null) continue; + AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(t.Assembly); + if (alc?.IsCollectible == true) + { + collectibleAlc = alc; + break; + } + } + + // Check all types discovered in the type scopes (e.g. generic type arguments, + // property types) so that types like List are handled correctly. + if (collectibleAlc == null) + { + foreach (TypeScope scope in scopes) + { + foreach (Type scopeType in scope.Types) + { + AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(scopeType.Assembly); + if (alc?.IsCollectible == true) + { + collectibleAlc = alc; + break; + } + } + if (collectibleAlc != null) break; + } + } + + // If no collectible types were found in the type graph, fall back to the caller's ALC. + // This handles the "inverted" scenario where all serialized types are in the default ALC + // but the serializer is being created from within a collectible ALC. + if (collectibleAlc == null && callerAlc?.IsCollectible == true) + collectibleAlc = callerAlc; + + // Enter contextual reflection using the effective ALC. For the inverted scenario the + // collectible ALC must be used (not mainAssembly which may be in the default ALC) so + // that DefineDynamicAssembly correctly places the generated assembly in the right ALC. + using (collectibleAlc != null + ? collectibleAlc.EnterContextualReflection() + : AssemblyLoadContext.EnterContextualReflection(mainAssembly)) { // Before generating any IL, check each mapping and supported type to make sure // they are compatible with the current ALC @@ -455,7 +505,10 @@ internal static Assembly GenerateRefEmitAssembly(XmlMapping[] xmlMappings, Type? VerifyLoadContext(mapping.Accessor.Mapping?.TypeDesc?.Type, mainAssembly); string assemblyName = "Microsoft.GeneratedCode"; - AssemblyBuilder assemblyBuilder = CodeGenerator.CreateAssemblyBuilder(assemblyName); + // Create a collectible assembly when the effective context is a collectible ALC so + // that the generated assembly can reference collectible types and will itself be + // collected when the ALC is unloaded. + AssemblyBuilder assemblyBuilder = CodeGenerator.CreateAssemblyBuilder(assemblyName, collectible: collectibleAlc != null); // Add AssemblyVersion attribute to match parent assembly version if (mainType != null) { @@ -597,9 +650,12 @@ internal static void VerifyLoadContext(Type? t, Assembly? assembly) if (typeALC == null || !typeALC.IsCollectible) return; - // Collectible types should be in the same collectible context + // Collectible types should be in the same collectible context. + // Also check CurrentContextualReflectionContext to allow the "inverted" scenario where + // the serializer is created from within a collectible ALC for a type whose assembly + // is in the default ALC (e.g. List). var baseALC = AssemblyLoadContext.GetLoadContext(assembly) ?? AssemblyLoadContext.CurrentContextualReflectionContext; - if (typeALC != baseALC) + if (typeALC != baseALC && typeALC != AssemblyLoadContext.CurrentContextualReflectionContext) throw new InvalidOperationException(SR.Format(SR.XmlTypeInBadLoadContext, t.FullName)); } @@ -692,6 +748,30 @@ internal sealed class TempAssemblyCache private readonly ConditionalWeakTable> _collectibleCaches = new ConditionalWeakTable>(); private Dictionary _fastCache = new Dictionary(); + // Returns the assembly to use as the ConditionalWeakTable key for a given type. + // + // When the type's assembly is collectible, it is used directly (original behavior). + // For the "inverted" scenario where the type appears to be in the default ALC (e.g. + // List) but the caller is executing within a collectible ALC, we use the + // first assembly from the caller's ALC as the weak key. This ensures the cache entry is + // GC'd when the caller's ALC is unloaded without creating a circular dependency between + // the ALC lifecycle and the cache entry. + private static Assembly? GetCollectibleKeyAssembly(Type t) + { + AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(t.Assembly); + if (alc?.IsCollectible == true) + return t.Assembly; + + AssemblyLoadContext? currentAlc = AssemblyLoadContext.CurrentContextualReflectionContext; + if (currentAlc?.IsCollectible == true) + { + foreach (Assembly a in currentAlc.Assemblies) + return a; + } + + return null; + } + internal TempAssembly? this[string? ns, Type t] { get @@ -702,7 +782,8 @@ internal sealed class TempAssemblyCache if (_fastCache.TryGetValue(key, out tempAssembly)) return tempAssembly; - if (_collectibleCaches.TryGetValue(t.Assembly, out var cCache)) + Assembly? keyAssembly = GetCollectibleKeyAssembly(t); + if (keyAssembly != null && _collectibleCaches.TryGetValue(keyAssembly, out var cCache)) cCache.TryGetValue(key, out tempAssembly); return tempAssembly; @@ -717,17 +798,17 @@ internal void Add(string? ns, Type t, TempAssembly assembly) if (tempAssembly == assembly) return; - AssemblyLoadContext? alc = AssemblyLoadContext.GetLoadContext(t.Assembly); + Assembly? keyAssembly = GetCollectibleKeyAssembly(t); TempAssemblyCacheKey key = new TempAssemblyCacheKey(ns, t); Dictionary? cache; - if (alc != null && alc.IsCollectible) + if (keyAssembly != null) { - cache = _collectibleCaches.TryGetValue(t.Assembly, out var c) // Clone or create + cache = _collectibleCaches.TryGetValue(keyAssembly, out var c) // Clone or create ? new Dictionary(c) : new Dictionary(); cache[key] = assembly; - _collectibleCaches.AddOrUpdate(t.Assembly, cache); + _collectibleCaches.AddOrUpdate(keyAssembly, 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..4dbd6e4f02e0cd 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 @@ -13,11 +13,33 @@ internal sealed class ContextAwareTables<[DynamicallyAccessedMembers(Dynamically { private readonly Hashtable _defaultTable; private readonly ConditionalWeakTable _collectibleTable; + // Used for the "inverted" scenario: the type being serialized is in the default ALC but + // the serializer is created from within a collectible ALC. Keyed by the first assembly + // from the caller's collectible ALC so entries are GC'd when that ALC is unloaded. + private readonly ConditionalWeakTable _collectibleAlcTable; public ContextAwareTables() { _defaultTable = new Hashtable(); _collectibleTable = new ConditionalWeakTable(); + _collectibleAlcTable = new ConditionalWeakTable(); + } + + // Returns the first assembly from the current contextual reflection context's collectible + // ALC to use as a stable weak key for the inverted caching scenario. + private static Assembly? GetInvertedAlcKeyAssembly(Type t) + { + AssemblyLoadContext? typeAlc = AssemblyLoadContext.GetLoadContext(t.Assembly); + if (typeAlc?.IsCollectible == true) + return null; // Direct case: handled by _collectibleTable keyed on the type + + AssemblyLoadContext? currentAlc = AssemblyLoadContext.CurrentContextualReflectionContext; + if (currentAlc?.IsCollectible == true) + { + foreach (Assembly a in currentAlc.Assemblies) + return a; + } + return null; } internal T GetOrCreateValue(Type t, Func f) @@ -27,35 +49,66 @@ internal T GetOrCreateValue(Type t, Func f) if (ret != null) return ret; - // Common case for collectible contexts + // Common case for collectible types (type itself is in a collectible ALC) if (_collectibleTable.TryGetValue(t, out ret)) return ret; + // Inverted scenario: type is in the default ALC but caller is in a collectible ALC + Assembly? alcKeyAssembly = GetInvertedAlcKeyAssembly(t); + if (alcKeyAssembly != null && _collectibleAlcTable.TryGetValue(alcKeyAssembly, out Hashtable? alcTable)) + { + ret = (T?)alcTable[t]; + if (ret != null) + 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 (alc?.IsCollectible == true) { - lock (_defaultTable) + // Type is in a collectible ALC — use the ConditionalWeakTable so entries can be + // collected when the ALC is unloaded (keyed on the type itself) + lock (_collectibleTable) { - if ((ret = (T?)_defaultTable[t]) == null) + if (!_collectibleTable.TryGetValue(t, out ret)) { ret = f(t); - _defaultTable[t] = ret; + _collectibleTable.AddOrUpdate(t, ret); } } } + else if (alcKeyAssembly != null) + { + // Inverted scenario: type appears to be in the default ALC but the caller is in a + // collectible ALC. Use a per-ALC hashtable so entries are GC'd when the ALC unloads. + lock (_collectibleAlcTable) + { + if (!_collectibleAlcTable.TryGetValue(alcKeyAssembly, out alcTable)) + { + alcTable = new Hashtable(); + _collectibleAlcTable.Add(alcKeyAssembly, alcTable); + } - // Collectible load contexts should use the ConditionalWeakTable so they can be unloaded + lock (alcTable) + { + if ((ret = (T?)alcTable[t]) == null) + { + ret = f(t); + alcTable[t] = ret; + } + } + } + } else { - lock (_collectibleTable) + // Null and non-collectible load contexts use the default table + lock (_defaultTable) { - if (!_collectibleTable.TryGetValue(t, out ret)) + if ((ret = (T?)_defaultTable[t]) == null) { ret = f(t); - _collectibleTable.AddOrUpdate(t, ret); + _defaultTable[t] = ret; } } } diff --git a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs index fc285987efc134..ae91a845a2a06e 100644 --- a/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs +++ b/src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs @@ -2656,6 +2656,118 @@ public static void Xml_TypeInCollectibleALC() Assert.True(!weakRef.IsAlive); } +#if !ReflectionOnly && !XMLSERIALIZERGENERATORTESTS + // Tests for the "inverted" scenario: serializer is created from within a collectible ALC. + // This tests the ILGen path specifically; the reflection-based path already handles this. + + [ConditionalFact(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))] + public static void Xml_SerializerCreatedInsideCollectibleALC() + { + // Inverted scenario: create XmlSerializer from within a collectible ALC context for a + // type that lives in that same collectible ALC. Verify the ALC can still be unloaded. + ExecuteAndUnloadWithContext("SerializableAssembly.dll", "SerializationTypes.SimpleType", 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.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))] + public static void Xml_FromTypesInsideCollectibleALC() + { + // Verify that XmlSerializer.FromTypes works when called from within a collectible ALC + // and that the ALC can be unloaded afterwards. + ExecuteAndUnloadFromTypes("SerializableAssembly.dll", "SerializationTypes.SimpleType", 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.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))] + public static void Xml_MultipleCollectibleALCsCanUnloadIndependently() + { + // Verify that multiple collectible ALCs each get their own cache partition and can + // unload independently without affecting each other. + ExecuteAndUnloadWithContext("SerializableAssembly.dll", "SerializationTypes.SimpleType", out var weakRef1); + ExecuteAndUnloadWithContext("SerializableAssembly.dll", "SerializationTypes.SimpleType", out var weakRef2); + + for (int i = 0; (weakRef1.IsAlive || weakRef2.IsAlive) && i < 10; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + Assert.True(!weakRef1.IsAlive); + Assert.True(!weakRef2.IsAlive); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ExecuteAndUnloadWithContext(string assemblyfile, string typename, out WeakReference wref) + { + var fullPath = Path.GetFullPath(assemblyfile); + var alc = new TestAssemblyLoadContext("XmlSerializerTests_Context", true, fullPath); + wref = new WeakReference(alc); + + var asm = alc.LoadFromAssemblyPath(fullPath); + var type = asm.GetType(typename); + Assert.NotNull(type); + Assert.Equal(AssemblyLoadContext.GetLoadContext(type.Assembly), alc); + + // Simulate code running within the collectible ALC (the "inverted" scenario) + using (alc.EnterContextualReflection()) + { + 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(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ExecuteAndUnloadFromTypes(string assemblyfile, string typename, out WeakReference wref) + { + var fullPath = Path.GetFullPath(assemblyfile); + var alc = new TestAssemblyLoadContext("XmlSerializerTests_FromTypes", true, fullPath); + wref = new WeakReference(alc); + + var asm = alc.LoadFromAssemblyPath(fullPath); + var type = asm.GetType(typename); + Assert.NotNull(type); + + // Simulate code running within the collectible ALC using FromTypes + using (alc.EnterContextualReflection()) + { + var serializers = XmlSerializer.FromTypes(new[] { type }); + Assert.Single(serializers); + var obj = Activator.CreateInstance(type); + var rtobj = SerializeAndDeserialize(obj, null, () => serializers[0], true); + Assert.NotNull(rtobj); + Assert.True(rtobj.Equals(obj)); + } + + alc.Unload(); + } +#endif + + [Fact] public static void ValidateXElement() {