Enhance collectible assembly handling in XmlSerializer#127695
Open
StephenMolloy wants to merge 4 commits intodotnet:mainfrom
Open
Enhance collectible assembly handling in XmlSerializer#127695StephenMolloy wants to merge 4 commits intodotnet:mainfrom
StephenMolloy wants to merge 4 commits intodotnet:mainfrom
Conversation
…mbly when entering the ALC for the generated serializer, fixing cases like `List<T>` where the outer type may live in the default ALC but a type argument is collectible. Also switch ContextAwareTables to use the collectible table whenever Type.IsCollectible is true, not only when the type's own ALC is collectible.
…yCache - Expose FindCollectibleAssembly method to determine the correct collectible assembly for types. - Update caching logic to utilize the collectible assembly for any collectible type, ensuring proper cache entry release when the collectible ALC is unloaded.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves XmlSerializer behavior for collectible types embedded in non-collectible container types (e.g., List<T> or T[] where T comes from a collectible AssemblyLoadContext). It ensures serializer generation and caching are associated with the correct collectible context/assembly, avoiding cache roots that would otherwise prevent unloading.
Changes:
- Added a helper to locate an appropriate collectible assembly by walking generic arguments / element types, and used it to pick the contextual reflection scope for serializer generation.
- Updated
TempAssemblyCachelookups and inserts to key collectible entries off the discovered collectible assembly instead oft.Assembly(important for container types whose defining assembly is in the default ALC). - Updated
ContextAwareTablesto use the weak table for collectible types even when theirAssemblyLoadContextis non-collectible (again, container-type scenario), and added coverage tests forList<T>/ arrays.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs | Adds test coverage for collectible types inside List<> / array containers and verifies unload behavior. |
| src/libraries/System.Private.Xml/src/System/Xml/Serialization/ContextAwareTables.cs | Routes collectible types (not just collectible ALCs) into the weak-table-backed storage to avoid rooting collectible Type instances. |
| src/libraries/System.Private.Xml/src/System/Xml/Serialization/Compilation.cs | Introduces FindCollectibleAssembly and uses it to select the contextual reflection ALC and to key collectible temp-assembly caches appropriately. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
FindCollectibleAssemblyhelper method inCompilation.csto 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.GenerateRefEmitAssembly), cache lookup (TempAssemblyCache.GetTempAssembly), and cache insertion (TempAssemblyCache.Add) logic to use the collectible assembly found viaFindCollectibleAssembly, rather than relying solely ont.Assembly. This change ensures that collectible types within generic containers are handled correctly and can be unloaded. [1] [2] [3]Improvements to context-aware tables:
ContextAwareTables.GetOrCreateValueto use the default table only for non-collectible types and non-collectible load contexts, and to use aConditionalWeakTablefor collectible types or collectible load contexts, improving resource management for collectible scenarios. [1] [2]Testing enhancements:
Xml_CollectibleTypeInGenericContainer) to verify that collectible types inside generic containers (likeList<T>or arrays) are correctly handled and can be unloaded, preventing memory leaks. This includes a helper methodExecuteCollectibleContainerAndUnloadthat exercises these scenarios. [1] [2]Minor changes:
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