Use AssemblyLoadContext-aware caches in TypeDescriptor to support unloading of assemblies cached by TypeDescriptor

[alexey-zakharov]

Fix cleanup of cached types in TypeDescriptor.

Motivation

MetadataUpdateHandler attribute is used to facilitate .NET HotReload and used in Unity to ensure Type caches are cleared on AssemblyLoadContext unloading.
However ReflectTypeDescriptionProvider currently does not fully clear types that were requested to be removed.
While the cached properties for the type are cleared, the Type itself is kept in the _typeData ConcurrentDictionary.
This prevents AssemblyLoadContext from unloading.

Changes

V1
The PR changes the ReflectionCachesUpdateHandler behavior to completely remove the requested types from the provider cache.

V2

• The PR introduces ContextAwareConcurrentDictionary and ContextAwareHashtable helper classes that wrap key and values with WeakReference for collectible types.
• The context aware tables are use in TypeDescription and ReflectTypeDescriptionProvider implementations to ensure there are no strong references from static caches to types from unloadable assemblies.

The PR addresses #114620 #30656 issue.

@simonferquel mentioned that the potential alternative could be to use WeakHashtable instead. However this would come with performance costs, so if cache clearing is fundamentally ok and doesn't break functionality I would prefer to keep it.

Testing

• Added TypeDescriptorTests.TypeDescriptor_DoesNotKeepUnloadableTypes test to validate caching for collectible types.
• Validated System.ComponentModel.TypeConverter.Tests locally.
• Validated no ALC leaks are present using a custom reporting tool.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

[Copilot]

Copilot reviewed 6 out of 9 changed files in this pull request and generated no comments.

Files not reviewed (3)

• src/libraries/System.ComponentModel.TypeConverter/System.ComponentModel.TypeConverter.sln: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/tests/System.ComponentModel.TypeConverter.Tests.csproj: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.csproj: Language not supported

Comments suppressed due to low confidence (1)

src/libraries/System.ComponentModel.TypeConverter/tests/ReflectionCachesUpdateHandlerTests.cs:104

• Consider increasing the number of garbage collection attempts or using a more robust timeout mechanism to ensure that the assembly unloads reliably across different environments.

for (int i = 0; weakRef.IsAlive && i < 10; i++)

[jkotas]

caches are cleared on AssemblyLoadContext unloading.

MetadataUpdateHandler is not meant to be used to prepare caches for AssemblyLoadContext unloading.

[jkotas]

the potential alternative could be to use WeakHashtable instead. However this would come with performance costs

The simplest way to avoid the overhead is to skip the caching completely for collectible types. We do that in number of places in this repo.

[alexey-zakharov]

The simplest way to avoid the overhead is to skip the caching completely for collectible types. We do that in number of places in this repo.

thanks for the suggestion!
we could skip caching completely, but I would expect potential json serialization performance regression for types from collectible assemblies
I'll update the PR to do this instead

[jkotas]

If the performance regression is not acceptable, it is an option to have two caches - one for collectible types and one for non-collectible types. It avoids the performance overhead of weak handles for non-colllectible types.

[alexey-zakharov]

If the performance regression is not acceptable, it is an option to have two caches - one for collectible types and one for non-collectible types. It avoids the performance overhead of weak handles for non-colllectible types.

@jkotas to clarify - would you be ok with a solution that keeps existing cache for types in non-collectible ALCs and WeakHashtable for the types in collectible ALCs (for both TypeDescriptor and ReflectTypeDescriptionProvider cache levels), correct?

I like this approach, as it gives correctness and performance at a relatively low cost of calling Type.IsCollectible 😄

[steveharter]

This and the other ClearCache() below will need to lock on:

lock (s_commonSyncObject)

Adding items to s_defaultProviderInitialized is already protected this way and so should removing.

[alexey-zakharov]

thanks @steveharter !
to clarify - are you ok with the approach in the PR or should I split caches into 2 - one for non-collectible types and one for collectible types (then we don't need ClearCache changes)

[steveharter]

Are these already protected since they call TypeDescriptor.Refresh(...) methods?

runtime/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs

Line 2322 in 65cda8b

lock (s_commonSyncObject)

runtime/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs

Line 2406 in 65cda8b

lock (s_commonSyncObject)

[alexey-zakharov]

Are these already protected since they call TypeDescriptor.Refresh(...) methods?

they are protected in isolation - s_commonSyncObject as I understand protects s_providerTable as well as access to the cached providers
s_defaultProviderInitialized is a ConcurrentDictionary, so it does not need lock here

but enumeration of the s_defaultProviderInitialized needs the lock since the content can be modified during enumeration

[alexey-zakharov]

if we are fine with the ClearCache change I'll fix the locks usage, however if we prefer to split the caches to have a separate WeakHashtables s_providerTypeTable and s_defaultProviderInitialized for the collectible types, then we don't need current changes

[steveharter]

I'm not sure on what issue this PR is current solving. Currently it depends on hot reload APIs to be called to clear caches, but that already works for hot reload purposes AFAIK. It doesn't solve the more general issue of unloading a collectible ALC.

[jkotas]

would you be ok with a solution that keeps existing cache for types in non-collectible ALCs and WeakHashtable for the types in collectible ALCs

It would be fine with me. We have similar split in other places.

[alexey-zakharov]

I'm not sure on what issue this PR is current solving. Currently it depends on hot reload APIs to be called to clear caches, but that already works for hot reload purposes AFAIK. It doesn't solve the more general issue of unloading a collectible ALC.

@steveharter yes, I'm going to rework it to split the caches next week.
the reason we went with reusing HotReload api is that the places that we've found leaks so far were matching the MetadataUpdateHandler usages.
I think moving to weak table would remove the need of relying on the MetadataUpdateHandler.

Although there is one case which may need a cache clear approach - ArrayPool<>.Shared holds references to the unloading ALCs in the returned pools. And while it is purged eventually, the purge speed it not sufficient to not run out of memory due to accumulating ALCs. But I'll raise this issue separately - there are multiple possible solutions to consider

[Copilot]

Pull Request Overview

This PR fixes an issue with ReflectTypeDescriptionProvider by ensuring that the cached type is completely removed during cache cleanup, allowing for proper unloading of AssemblyLoadContexts.

• Updated cache data structures to use ContextAwareConcurrentDictionary and ContextAwareHashtable.
• Added new tests to verify that unloadable types are correctly cleared from the cache.
• Refactored related helper classes (WeakHashtable, ContextAwareConcurrentDictionary, and ContextAwareHashtable) for improved cache management.

Reviewed Changes

Copilot reviewed 7 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
UnloadableTestTypes.cs	Added sample unloadable type and associated attribute for testing.
TypeDescriptorTests.cs	Introduced new tests to validate cache clearance and proper unloading.
WeakHashtable.cs	Modified weak reference handling for enumeration in caching.
TypeDescriptor.cs	Updated cache implementation to use ContextAware data structures.
ReflectTypeDescriptionProvider.cs	Refactored to use new ContextAwareConcurrentDictionary and improved threading comments.
ContextAwareHashtable.cs	Added new implementation to handle collectible types in caching.
ContextAwareConcurrentDictionary.cs	Introduced new concurrent dictionary to support collectible MemberInfo keys.

Files not reviewed (4)

• src/libraries/System.ComponentModel.TypeConverter/System.ComponentModel.TypeConverter.sln: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/tests/System.ComponentModel.TypeConverter.Tests.csproj: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.csproj: Language not supported

[Copilot]

Pull Request Overview

This PR fixes a caching issue in ReflectTypeDescriptionProvider by ensuring that collectible types are fully removed from static caches, thereby allowing AssemblyLoadContext unload. Key changes include:

• Transition from ConcurrentDictionary/Hashtable to context-aware collections (ContextAwareConcurrentDictionary and ContextAwareHashtable) for collectible types.
• Updates in TypeDescriptor and ReflectTypeDescriptionProvider to use the new collections.
• Addition of tests to confirm that unloadable types are not retained in the caches.

Reviewed Changes

Copilot reviewed 7 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/UnloadableTestTypes/UnloadableTestTypes.cs	Added sample unloadable types for testing.
tests/TypeDescriptorTests.cs	Introduced a new AssemblyLoadContext test to verify cache cleanup.
src/System/ComponentModel/WeakHashtable.cs	Updated weak referencing via indexer override.
src/System/ComponentModel/TypeDescriptor.cs	Switched to ContextAware collections and refined lock comments.
src/System/ComponentModel/ReflectTypeDescriptionProvider.cs	Modified caching to use context-aware collections and improved comments.
src/System/ComponentModel/ContextAwareHashtable.cs	New helper to conditionally use weak references for collectible keys.
src/System/ComponentModel/ContextAwareConcurrentDictionary.cs	New concurrent dictionary variant for handling collectible keys.

Files not reviewed (4)

• src/libraries/System.ComponentModel.TypeConverter/System.ComponentModel.TypeConverter.sln: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/tests/System.ComponentModel.TypeConverter.Tests.csproj: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.csproj: Language not supported

[alexey-zakharov]

@steveharter @jkotas I've updated the implementation to use WeakHashtable and ContextAware* patterns to have a separate caches for collectible types.

That give us a good balance between functional correctness and performance as:

• WeakReference ensures there are no gaps between AssemblyLoadContext unload and repopulating the cache.
• The best case performance overhead (non-unloadable assemblies) is effectively MemberInfo.IsCollectible call only.

[alexey-zakharov]

if we want to keep a non-generic version to limit the amount of jit we need to do, I'm happy to make ContextAwareHashtable non-generic

[Copilot]

Pull Request Overview

This PR fixes type caching issues to ensure unloadable assembly types are properly removed from static caches and introduces context-aware collections to hold weak references for collectible types.

• Updated tests to validate that unloadable types are cleared.
• Replaced standard collections with ContextAwareConcurrentDictionary and ContextAwareHashtable in caching logic.
• Added helper classes for weak referencing collectible types.

Reviewed Changes

Copilot reviewed 7 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/UnloadableTestTypes/UnloadableTestTypes.cs	Added simple unloadable test types.
tests/TypeDescriptorTests.cs	Added assembly load context and unload test to exercise cache clearing.
src/System/ComponentModel/WeakHashtable.cs	Updated enumerator behavior to capture keys during iteration.
src/System/ComponentModel/TypeDescriptor.cs	Migrated to context-aware collections for caching.
src/System/ComponentModel/ReflectTypeDescriptionProvider.cs	Transitioned to context-aware caches and ensured proper removal criteria.
src/System/ComponentModel/ContextAwareHashtable.cs	Introduced new hashtable for wrapping collectible keys in weak references.
src/System/ComponentModel/ContextAwareConcurrentDictionary.cs	Added a new concurrent dictionary for mapping collectible keys using ConditionalWeakTable.

Files not reviewed (4)

• src/libraries/System.ComponentModel.TypeConverter/System.ComponentModel.TypeConverter.sln: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/tests/System.ComponentModel.TypeConverter.Tests.csproj: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.csproj: Language not supported

[Copilot]

Pull Request Overview

This PR fixes an issue with ReflectTypeDescriptionProvider not completely clearing cached types and introduces context‐aware collections to support unloadable assemblies. Key changes include:

• Removal of strong references from caches via ContextAwareConcurrentDictionary and ContextAwareHashtable.
• Addition of new test types and unit tests to verify that unloadable assemblies are properly cleaned up.
• Updates to caching and enumeration logic in WeakHashtable and ReflectTypeDescriptionProvider.

Reviewed Changes

Copilot reviewed 7 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.cs	Added simple test types for unloadable assembly scenarios.
src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs	Introduced tests ensuring that cached types from collectible assemblies are released.
src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/WeakHashtable.cs	Updated to use indexer for setting weak references.
src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs	Replaced standard dictionaries/hashtables with context-aware collections.
src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs	Updated to use context-aware caching and ensure complete cache cleanup.
src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs	Added new helper class for context-aware hashtable operations.
src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareConcurrentDictionary.cs	Added new helper that uses ConditionalWeakTable for collectible keys alongside ConcurrentDictionary for others.

Files not reviewed (4)

• src/libraries/System.ComponentModel.TypeConverter/System.ComponentModel.TypeConverter.sln: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/tests/System.ComponentModel.TypeConverter.Tests.csproj: Language not supported
• src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.csproj: Language not supported

[Copilot]

ConditionalWeakTable does not have an AddOrUpdate method in the standard API. Consider checking if an update is needed by removing the key first or using an alternative pattern that is supported.

Suggested change

	_collectibleTable.AddOrUpdate(key, value!);
	_collectibleTable.Remove(key); // Remove the key if it exists
	_collectibleTable.Add(key, value!); // Add the new key-value pair

[alexey-zakharov]

Updated TypeDescriptor.s_providerTypeTable table to use weak values as those capture the LoaderAllocator too