From ef4361620d654a68e5a5cb6542e5aa887fac260b Mon Sep 17 00:00:00 2001 From: Alexey Zakharov Date: Mon, 14 Apr 2025 10:59:14 +0200 Subject: [PATCH 1/4] =?UTF-8?q?=EF=BB=BFUse=20separate=20hashtables=20for?= =?UTF-8?q?=20collectible=20types=20to=20support=20assembly=20unloadabilit?= =?UTF-8?q?y=20for=20the=20TypeDescriptor=20class?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../System.ComponentModel.TypeConverter.slnx | 3 + ...System.ComponentModel.TypeConverter.csproj | 2 + .../ContextAwareConcurrentHashtable.cs | 137 ++++++++++++++++++ .../ComponentModel/ContextAwareHashtable.cs | 40 +++++ .../ReflectTypeDescriptionProvider.cs | 33 ++--- .../System/ComponentModel/TypeDescriptor.cs | 12 +- .../System/ComponentModel/WeakHashtable.cs | 90 +++++++++++- ....ComponentModel.TypeConverter.Tests.csproj | 3 + .../tests/TypeDescriptorTests.cs | 101 +++++++++++++ .../UnloadableTestTypes.cs | 22 +++ .../UnloadableTestTypes.csproj | 10 ++ 11 files changed, 425 insertions(+), 28 deletions(-) create mode 100644 src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareConcurrentHashtable.cs create mode 100644 src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs create mode 100644 src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.cs create mode 100644 src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.csproj diff --git a/src/libraries/System.ComponentModel.TypeConverter/System.ComponentModel.TypeConverter.slnx b/src/libraries/System.ComponentModel.TypeConverter/System.ComponentModel.TypeConverter.slnx index 77bb7634e1ca2d..50b35ddc2d9bea 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/System.ComponentModel.TypeConverter.slnx +++ b/src/libraries/System.ComponentModel.TypeConverter/System.ComponentModel.TypeConverter.slnx @@ -263,6 +263,9 @@ + + + diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj b/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj index 88b57d1c749a32..e8adf9e090d880 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj @@ -15,6 +15,7 @@ + @@ -44,6 +45,7 @@ + diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareConcurrentHashtable.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareConcurrentHashtable.cs new file mode 100644 index 00000000000000..5496274cd89cff --- /dev/null +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareConcurrentHashtable.cs @@ -0,0 +1,137 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Reflection; +using System.Runtime.CompilerServices; + +namespace System.ComponentModel +{ + /// + /// Concurrent dictionary that maps MemberInfo object key to an object. + /// Uses ConditionalWeakTable for the collectible keys (if MemberInfo.IsCollectible is true) and + /// ConcurrentDictionary for non-collectible keys. + /// + internal sealed class ContextAwareConcurrentHashtable : IEnumerable> + where TKey : MemberInfo + where TValue : class? + { + private readonly ConcurrentDictionary _defaultTable = new ConcurrentDictionary(); + private readonly ConditionalWeakTable _collectibleTable = new ConditionalWeakTable(); + + public TValue? this[TKey key] + { + get + { + return TryGetValue(key, out TValue? value) ? value : default; + } + + set + { + if (!key.IsCollectible) + { + _defaultTable[key] = value!; + } + else + { + _collectibleTable.AddOrUpdate(key, value!); + } + } + } + + public bool Contains(TKey key) + { + return !key.IsCollectible ? _defaultTable.ContainsKey(key) : _collectibleTable.TryGetValue(key, out _); + } + + public bool TryGetValue(TKey key, [MaybeNullWhen(false)] out TValue value) + { + if (!key.IsCollectible) + return _defaultTable.TryGetValue(key, out value); + + if (_collectibleTable.TryGetValue(key, out object? valueObj) && valueObj != null) + { + value = (TValue)valueObj; + return true; + } + + value = default; + return false; + } + + public bool TryAdd(TKey key, TValue value) + { + return !key.IsCollectible + ? _defaultTable.TryAdd(key, value) + : _collectibleTable.TryAdd(key, value); + } + + public void Clear() + { + _defaultTable.Clear(); + _collectibleTable.Clear(); + } + + public IEnumerator> GetEnumerator() => + new Enumerator(_defaultTable.GetEnumerator(), ((IEnumerable>)_collectibleTable).GetEnumerator()); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + + private sealed class Enumerator : IEnumerator> + { + private readonly IEnumerator> _defaultEnumerator; + private readonly IEnumerator> _collectibleEnumerator; + private bool _enumeratingCollectibleEnumerator; + + public Enumerator(IEnumerator> defaultEnumerator, IEnumerator> collectibleEnumerator) + { + _defaultEnumerator = defaultEnumerator; + _collectibleEnumerator = collectibleEnumerator; + _enumeratingCollectibleEnumerator = false; + } + + public KeyValuePair Current { get; private set; } + + object IEnumerator.Current => Current; + + public void Dispose() + { + _defaultEnumerator.Dispose(); + _collectibleEnumerator.Dispose(); + } + + public bool MoveNext() + { + if (!_enumeratingCollectibleEnumerator && _defaultEnumerator.MoveNext()) + { + Current = _defaultEnumerator.Current; + return true; + } + + _enumeratingCollectibleEnumerator = true; + + while (_collectibleEnumerator.MoveNext()) + { + if (_collectibleEnumerator.Current.Value is TValue value) + { + Current = new KeyValuePair(_collectibleEnumerator.Current.Key, value); + return true; + } + } + + Current = default; + return false; + } + + public void Reset() + { + _defaultEnumerator.Reset(); + _collectibleEnumerator.Reset(); + Current = default; + } + } + } +} diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs new file mode 100644 index 00000000000000..a3bfd60709368e --- /dev/null +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs @@ -0,0 +1,40 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections; +using System.Reflection; +using System.Runtime.CompilerServices; + +namespace System.ComponentModel +{ + /// + /// Hashtable that maps MemberInfo object key to an object and + /// uses a WeakReference for the collectible keys (if MemberInfo.IsCollectible is true). + /// Uses a Hashtable for non-collectible keys and WeakHashtable for the collectible keys. + /// + internal sealed class ContextAwareHashtable + { + private readonly Hashtable _defaultTable = new Hashtable(); + private readonly ConditionalWeakTable _collectibleTable = new ConditionalWeakTable(); + + public object? this[MemberInfo key] + { + get + { + return !key.IsCollectible ? _defaultTable[key] : (_collectibleTable.TryGetValue(key, out object? value) ? value : null); + } + + set + { + if (!key.IsCollectible) + { + _defaultTable[key] = value!; + } + else + { + _collectibleTable.AddOrUpdate(key, value!); + } + } + } + } +} diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs index 60e69f6cb92c2a..dac561167c9d98 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs @@ -3,7 +3,6 @@ using System.Collections; using System.Collections.Generic; -using System.Collections.Concurrent; using System.ComponentModel.Design; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; @@ -24,7 +23,7 @@ namespace System.ComponentModel internal sealed partial class ReflectTypeDescriptionProvider : TypeDescriptionProvider { // ReflectedTypeData contains all of the type information we have gathered for a given type. - private readonly ConcurrentDictionary _typeData = new ConcurrentDictionary(); + private readonly ContextAwareConcurrentHashtable _typeData = new ContextAwareConcurrentHashtable(); // This is the signature we look for when creating types that are generic, but // want to know what type they are dealing with. Enums are a good example of this; @@ -49,10 +48,10 @@ internal sealed partial class ReflectTypeDescriptionProvider : TypeDescriptionPr // on Control, Component and object are also automatically filled // in. The keys to the property and event caches are types. // The keys to the attribute cache are either MemberInfos or types. - private static Hashtable? s_propertyCache; - private static Hashtable? s_eventCache; - private static Hashtable? s_attributeCache; - private static Hashtable? s_extendedPropertyCache; + private static ContextAwareHashtable? s_propertyCache; + private static ContextAwareHashtable? s_eventCache; + private static ContextAwareHashtable? s_attributeCache; + private static ContextAwareHashtable? s_extendedPropertyCache; // These are keys we stuff into our object cache. We use this // cache data to store extender provider info for an object. @@ -193,13 +192,13 @@ private static Dictionary IntrinsicTypeConve Justification = "IntrinsicTypeConverters is marked with RequiresUnreferencedCode. It is the only place that should call this.")] private static NullableConverter CreateNullableConverter(Type type) => new NullableConverter(type); - private static Hashtable PropertyCache => LazyInitializer.EnsureInitialized(ref s_propertyCache, () => new Hashtable()); + private static ContextAwareHashtable PropertyCache => LazyInitializer.EnsureInitialized(ref s_propertyCache, () => new ContextAwareHashtable()); - private static Hashtable EventCache => LazyInitializer.EnsureInitialized(ref s_eventCache, () => new Hashtable()); + private static ContextAwareHashtable EventCache => LazyInitializer.EnsureInitialized(ref s_eventCache, () => new ContextAwareHashtable()); - private static Hashtable AttributeCache => LazyInitializer.EnsureInitialized(ref s_attributeCache, () => new Hashtable()); + private static ContextAwareHashtable AttributeCache => LazyInitializer.EnsureInitialized(ref s_attributeCache, () => new ContextAwareHashtable()); - private static Hashtable ExtendedPropertyCache => LazyInitializer.EnsureInitialized(ref s_extendedPropertyCache, () => new Hashtable()); + private static ContextAwareHashtable ExtendedPropertyCache => LazyInitializer.EnsureInitialized(ref s_extendedPropertyCache, () => new ContextAwareHashtable()); /// Clear the global caches this maintains on top of reflection. internal static void ClearReflectionCaches() @@ -982,14 +981,14 @@ private ReflectedTypeData GetTypeDataFromRegisteredType(Type type) { Type componentType = typeof(T); - if (_typeData.ContainsKey(componentType)) + if (_typeData.Contains(componentType)) { return; } lock (TypeDescriptor.s_commonSyncObject) { - if (_typeData.ContainsKey(componentType)) + if (_typeData.Contains(componentType)) { return; } @@ -1096,7 +1095,7 @@ internal bool IsPopulated(Type type) /// internal static Attribute[] ReflectGetAttributes(Type type) { - Hashtable attributeCache = AttributeCache; + ContextAwareHashtable attributeCache = AttributeCache; Attribute[]? attrs = (Attribute[]?)attributeCache[type]; if (attrs != null) { @@ -1124,7 +1123,7 @@ internal static Attribute[] ReflectGetAttributes(Type type) /// internal static Attribute[] ReflectGetAttributes(MemberInfo member) { - Hashtable attributeCache = AttributeCache; + ContextAwareHashtable attributeCache = AttributeCache; Attribute[]? attrs = (Attribute[]?)attributeCache[member]; if (attrs != null) { @@ -1152,7 +1151,7 @@ internal static Attribute[] ReflectGetAttributes(MemberInfo member) /// private static EventDescriptor[] ReflectGetEvents(Type type) { - Hashtable eventCache = EventCache; + ContextAwareHashtable eventCache = EventCache; EventDescriptor[]? events = (EventDescriptor[]?)eventCache[type]; if (events != null) { @@ -1252,7 +1251,7 @@ private static PropertyDescriptor[] ReflectGetExtendedProperties(IExtenderProvid // property store. // Type providerType = provider.GetType(); - Hashtable extendedPropertyCache = ExtendedPropertyCache; + ContextAwareHashtable extendedPropertyCache = ExtendedPropertyCache; ReflectPropertyDescriptor[]? extendedProperties = (ReflectPropertyDescriptor[]?)extendedPropertyCache[providerType]; if (extendedProperties == null) { @@ -1337,7 +1336,7 @@ private static PropertyDescriptor[] ReflectGetPropertiesFromRegisteredType(Type private static PropertyDescriptor[] ReflectGetPropertiesImpl(Type type) { - Hashtable propertyCache = PropertyCache; + ContextAwareHashtable propertyCache = PropertyCache; PropertyDescriptor[]? properties = (PropertyDescriptor[]?)propertyCache[type]; if (properties != null) { diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs index fae894f519902a..6d4b7ab06ae2c6 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections; -using System.Collections.Concurrent; -using System.Collections.Generic; using System.Collections.Specialized; using System.ComponentModel.Design; using System.Diagnostics; @@ -67,12 +65,12 @@ public sealed class TypeDescriptor internal static readonly object s_commonSyncObject = new object(); // A direct mapping from type to provider. - private static readonly ConcurrentDictionary s_providerTypeTable = new ConcurrentDictionary(); + private static readonly ContextAwareConcurrentHashtable s_providerTypeTable = new ContextAwareConcurrentHashtable(); // Tracks DefaultTypeDescriptionProviderAttributes. // A value of `null` indicates initialization is in progress. // A value of s_initializedDefaultProvider indicates the provider is initialized. - private static readonly ConcurrentDictionary s_defaultProviderInitialized = new ConcurrentDictionary(); + private static readonly ContextAwareConcurrentHashtable s_defaultProviderInitialized = new ContextAwareConcurrentHashtable(); private static readonly object s_initializedDefaultProvider = new object(); @@ -293,7 +291,7 @@ public static void AddProvider(TypeDescriptionProvider provider, object instance refreshNeeded = s_providerTable.ContainsKey(instance); TypeDescriptionNode node = NodeFor(instance, true); var head = new TypeDescriptionNode(provider) { Next = node }; - s_providerTable.SetWeak(instance, head); + s_providerTable[instance] = head; s_providerTypeTable.Clear(); } @@ -363,7 +361,7 @@ private static void AddDefaultProvider(Type type) { bool providerAdded = false; - if (s_defaultProviderInitialized.ContainsKey(type)) + if (s_defaultProviderInitialized.Contains(type)) { // Either another thread finished initializing for this type, or we are recursing on the same thread. return; @@ -433,7 +431,7 @@ public static void CreateAssociation(object primary, object secondary) if (associations == null) { associations = new ArrayList(4); - associationTable.SetWeak(primary, associations); + associationTable[primary] = associations; } } } diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/WeakHashtable.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/WeakHashtable.cs index 35fc1c8e523dca..c2478bed6915ee 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/WeakHashtable.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/WeakHashtable.cs @@ -23,15 +23,37 @@ internal WeakHashtable() : base(s_comparer) } /// - /// Override of Item that wraps a weak reference around the - /// key and performs a scavenge. + /// Adds a key-value pair to the hashtable, wrapping the key in a weak reference. /// - public void SetWeak(object key, object value) + public override void Add(object key, object? value) { ScavengeKeys(); - this[new EqualityWeakReference(key)] = value; + base.Add(new EqualityWeakReference(key), value); } + /// + /// Gets or sets the value associated with the specified key. + /// + public override object? this[object key] + { + get + { + return base[key]; + } + + set + { + ScavengeKeys(); + base[new EqualityWeakReference(key)] = value; + } + } + + /// + /// Override of GetEnumerator that returns enumerator which skips not alive keys + /// during enumeration. + /// + public override IDictionaryEnumerator GetEnumerator() => new WeakHashtableEnumerator(base.GetEnumerator()); + /// /// This method checks to see if it is necessary to /// scavenge keys, and if it is it performs a scan @@ -168,5 +190,65 @@ public override bool Equals(object? o) public override int GetHashCode() => _hashCode; } + + /// + /// A wrapper around the base HashTable enumerator that skips + /// collected keys and captures the key value to prevent it + /// from being collected during enumeration. + /// + private sealed class WeakHashtableEnumerator : IDictionaryEnumerator + { + private readonly IDictionaryEnumerator _baseEnumerator; + // Captured key of the WeakHashtable so that it is not collected during enumeration + private object? _currentKey; + + public WeakHashtableEnumerator(IDictionaryEnumerator baseEnumerator) + { + _baseEnumerator = baseEnumerator; + } + + public DictionaryEntry Entry => new DictionaryEntry(Key, Value); + + public object Key + { + get + { + object key = _baseEnumerator.Key; + return ((EqualityWeakReference)key).Target!; + } + } + + public object? Value => _baseEnumerator.Value; + + public object Current => Entry; + + public bool MoveNext() + { + while (_baseEnumerator.MoveNext()) + { + // The key must always be an EqualityWeakReference by design. + EqualityWeakReference key = (EqualityWeakReference)_baseEnumerator.Key; + + // If the weak reference is alive, capture it and return true. + _currentKey = key.Target; + if (_currentKey != null) + { + return true; + } + + // Otherwise, the key has been collected and we need to skip it. + } + + // Cleanup the captured key and return false if there is nothing more in the table. + _currentKey = null; + return false; + } + + public void Reset() + { + _baseEnumerator.Reset(); + _currentKey = null; + } + } } } diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/System.ComponentModel.TypeConverter.Tests.csproj b/src/libraries/System.ComponentModel.TypeConverter/tests/System.ComponentModel.TypeConverter.Tests.csproj index 1a9f2c778e6647..3aeb17c8844aae 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/System.ComponentModel.TypeConverter.Tests.csproj +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/System.ComponentModel.TypeConverter.Tests.csproj @@ -182,6 +182,9 @@ TestResx.Designer.cs + + + diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs index 6fb9f57edc479f..db0bf099067751 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs @@ -5,7 +5,11 @@ using System.Collections.Generic; using System.ComponentModel.Design; using System.Globalization; +using System.IO; using System.Linq; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.Loader; using System.Threading; using System.Threading.Tasks; using Microsoft.DotNet.RemoteExecutor; @@ -1523,5 +1527,102 @@ public class MyTypeConverter : TypeConverter public class MyInheritedClassWithCustomTypeDescriptionProviderConverter : TypeConverter { } + + private class TestAssemblyLoadContext : AssemblyLoadContext + { + private AssemblyDependencyResolver _resolver; + + public TestAssemblyLoadContext(string name, bool isCollectible, string mainAssemblyToLoadPath = null) : base(name, isCollectible) + { + if (!PlatformDetection.IsBrowser) + _resolver = new AssemblyDependencyResolver(mainAssemblyToLoadPath ?? Assembly.GetExecutingAssembly().Location); + } + + protected override Assembly Load(AssemblyName name) + { + if (PlatformDetection.IsBrowser) + { + return base.Load(name); + } + + string assemblyPath = _resolver.ResolveAssemblyToPath(name); + if (assemblyPath != null) + { + return LoadFromAssemblyPath(assemblyPath); + } + + return null; + } + } + + // This method must be not inlined to ensure that the ALC is not captured on the caller's stack. + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)] + private static void ExecuteAndUnload(string assemblyfile, Action assemblyAction, out WeakReference alcWeakRef) + { + var fullPath = Path.GetFullPath(assemblyfile); + var alc = new TestAssemblyLoadContext("TypeDescriptorTests", true, fullPath); + alcWeakRef = new WeakReference(alc); + + // Load assembly by path. By name, and it gets loaded in the default ALC. + var assembly = alc.LoadFromAssemblyPath(fullPath); + Assert.NotEqual(AssemblyLoadContext.GetLoadContext(assembly), AssemblyLoadContext.Default); + + // Perform action on the assembly from ALC + assemblyAction(assembly); + + // Unload the ALC + alc.Unload(); + } + + [Fact] + // Lack of AssemblyDependencyResolver results in assemblies that are not loaded by path to get + // loaded in the default ALC, which causes problems for this test. + [SkipOnPlatform(TestPlatforms.Browser, "AssemblyDependencyResolver not supported in wasm")] + [ActiveIssue("34072", TestRuntimes.Mono)] + public static void TypeDescriptor_DoesNotKeepUnloadableTypes() + { + ExecuteAndUnload("UnloadableTestTypes.dll", + static (assembly) => + { + // Ensure the type loaded in the intended non-Default ALC + Type? collectibleType = assembly.GetType("UnloadableTestTypes.SimpleType"); + Assert.NotNull(collectibleType); + Type? collectibleAttributeType = assembly.GetType("UnloadableTestTypes.SimpleTypeAttribute"); + Assert.NotNull(collectibleAttributeType); + Attribute? collectibleAttribute = collectibleType.GetCustomAttribute(collectibleAttributeType); + Assert.NotNull(collectibleAttribute); + + // Cache the type's cachable entities + AttributeCollection attributes = TypeDescriptor.GetAttributes(collectibleType); + Assert.True(attributes.Contains(collectibleAttribute)); + + EventDescriptorCollection events = TypeDescriptor.GetEvents(collectibleType); + Assert.Equal(1, events.Count); + + PropertyDescriptorCollection properties = TypeDescriptor.GetProperties(collectibleType); + Assert.Equal(2, properties.Count); + + TypeDescriptionProvider provider = TypeDescriptor.GetProvider(collectibleType); + Assert.NotNull(provider); + + // Refresh the assembly causing the ReflectTypeDescriptionProvider caches to be purged. + TypeDescriptor.Refresh(assembly); + + // Tests that the new collections are different + // from the old ones, and that the old ones are not kept alive. + Assert.NotSame(attributes, TypeDescriptor.GetAttributes(collectibleType)); + Assert.NotSame(events, TypeDescriptor.GetEvents(collectibleType)); + Assert.NotSame(properties, TypeDescriptor.GetProperties(collectibleType)); + }, + out var weakRef); + + for (int i = 0; weakRef.IsAlive && i < 10; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + Assert.True(!weakRef.IsAlive); + } + } } diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.cs new file mode 100644 index 00000000000000..3a8ec9b74de776 --- /dev/null +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.cs @@ -0,0 +1,22 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace UnloadableTestTypes +{ + [SimpleType] + public class SimpleType + { + public string P1 { get; set; } + public int P2 { get; set; } + public event Action ActionEvent; + public void OnActionEvent() + { + ActionEvent?.Invoke(); + } + } + + [AttributeUsage(AttributeTargets.All)] + public sealed class SimpleTypeAttribute : Attribute { } +} diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.csproj b/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.csproj new file mode 100644 index 00000000000000..de72d469f359c1 --- /dev/null +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.csproj @@ -0,0 +1,10 @@ + + + netstandard2.1 + + + + + + + From 93dfcf1a94c63cc01aba16fc6a5f8b5c4f22416a Mon Sep 17 00:00:00 2001 From: Alexey Zakharov Date: Thu, 10 Jul 2025 19:53:24 +0200 Subject: [PATCH 2/4] Addressed codereview feedback --- .../System.ComponentModel.TypeConverter.slnx | 5 +++++ .../System.ComponentModel.TypeConverter.csproj | 2 +- ...cs => CollectibleKeyConcurrentHashtable.cs} | 7 ++++--- .../ComponentModel/ContextAwareHashtable.cs | 6 +++--- .../ReflectTypeDescriptionProvider.cs | 6 +++--- .../System/ComponentModel/TypeDescriptor.cs | 6 +++--- .../tests/TypeDescriptorTests.cs | 9 +++++++++ .../UnloadableTestTypes/UnloadableTestTypes.cs | 18 ++++++++++++++++++ 8 files changed, 46 insertions(+), 13 deletions(-) rename src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/{ContextAwareConcurrentHashtable.cs => CollectibleKeyConcurrentHashtable.cs} (92%) diff --git a/src/libraries/System.ComponentModel.TypeConverter/System.ComponentModel.TypeConverter.slnx b/src/libraries/System.ComponentModel.TypeConverter/System.ComponentModel.TypeConverter.slnx index 50b35ddc2d9bea..249a2812bd98d9 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/System.ComponentModel.TypeConverter.slnx +++ b/src/libraries/System.ComponentModel.TypeConverter/System.ComponentModel.TypeConverter.slnx @@ -265,6 +265,11 @@ + + + + + diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj b/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj index e8adf9e090d880..0214ba20d330b5 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj @@ -15,7 +15,7 @@ - + diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareConcurrentHashtable.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CollectibleKeyConcurrentHashtable.cs similarity index 92% rename from src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareConcurrentHashtable.cs rename to src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CollectibleKeyConcurrentHashtable.cs index 5496274cd89cff..8665996aa5e176 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareConcurrentHashtable.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CollectibleKeyConcurrentHashtable.cs @@ -15,7 +15,7 @@ namespace System.ComponentModel /// Uses ConditionalWeakTable for the collectible keys (if MemberInfo.IsCollectible is true) and /// ConcurrentDictionary for non-collectible keys. /// - internal sealed class ContextAwareConcurrentHashtable : IEnumerable> + internal sealed class CollectibleKeyConcurrentHashtable : IEnumerable> where TKey : MemberInfo where TValue : class? { @@ -37,12 +37,12 @@ public TValue? this[TKey key] } else { - _collectibleTable.AddOrUpdate(key, value!); + _collectibleTable.AddOrUpdate(key, value); } } } - public bool Contains(TKey key) + public bool ContainsKey(TKey key) { return !key.IsCollectible ? _defaultTable.ContainsKey(key) : _collectibleTable.TryGetValue(key, out _); } @@ -130,6 +130,7 @@ public void Reset() { _defaultEnumerator.Reset(); _collectibleEnumerator.Reset(); + _enumeratingCollectibleEnumerator = false; Current = default; } } diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs index a3bfd60709368e..70179732256819 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs @@ -15,7 +15,7 @@ namespace System.ComponentModel internal sealed class ContextAwareHashtable { private readonly Hashtable _defaultTable = new Hashtable(); - private readonly ConditionalWeakTable _collectibleTable = new ConditionalWeakTable(); + private readonly ConditionalWeakTable _collectibleTable = new ConditionalWeakTable(); public object? this[MemberInfo key] { @@ -28,11 +28,11 @@ public object? this[MemberInfo key] { if (!key.IsCollectible) { - _defaultTable[key] = value!; + _defaultTable[key] = value; } else { - _collectibleTable.AddOrUpdate(key, value!); + _collectibleTable.AddOrUpdate(key, value); } } } diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs index dac561167c9d98..44c12d51184eff 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs @@ -23,7 +23,7 @@ namespace System.ComponentModel internal sealed partial class ReflectTypeDescriptionProvider : TypeDescriptionProvider { // ReflectedTypeData contains all of the type information we have gathered for a given type. - private readonly ContextAwareConcurrentHashtable _typeData = new ContextAwareConcurrentHashtable(); + private readonly CollectibleKeyConcurrentHashtable _typeData = new CollectibleKeyConcurrentHashtable(); // This is the signature we look for when creating types that are generic, but // want to know what type they are dealing with. Enums are a good example of this; @@ -981,14 +981,14 @@ private ReflectedTypeData GetTypeDataFromRegisteredType(Type type) { Type componentType = typeof(T); - if (_typeData.Contains(componentType)) + if (_typeData.ContainsKey(componentType)) { return; } lock (TypeDescriptor.s_commonSyncObject) { - if (_typeData.Contains(componentType)) + if (_typeData.ContainsKey(componentType)) { return; } diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs index 6d4b7ab06ae2c6..475669b1549821 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs @@ -65,12 +65,12 @@ public sealed class TypeDescriptor internal static readonly object s_commonSyncObject = new object(); // A direct mapping from type to provider. - private static readonly ContextAwareConcurrentHashtable s_providerTypeTable = new ContextAwareConcurrentHashtable(); + private static readonly CollectibleKeyConcurrentHashtable s_providerTypeTable = new CollectibleKeyConcurrentHashtable(); // Tracks DefaultTypeDescriptionProviderAttributes. // A value of `null` indicates initialization is in progress. // A value of s_initializedDefaultProvider indicates the provider is initialized. - private static readonly ContextAwareConcurrentHashtable s_defaultProviderInitialized = new ContextAwareConcurrentHashtable(); + private static readonly CollectibleKeyConcurrentHashtable s_defaultProviderInitialized = new CollectibleKeyConcurrentHashtable(); private static readonly object s_initializedDefaultProvider = new object(); @@ -361,7 +361,7 @@ private static void AddDefaultProvider(Type type) { bool providerAdded = false; - if (s_defaultProviderInitialized.Contains(type)) + if (s_defaultProviderInitialized.ContainsKey(type)) { // Either another thread finished initializing for this type, or we are recursing on the same thread. return; diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs index db0bf099067751..6ffe772892939b 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs @@ -1591,6 +1591,8 @@ public static void TypeDescriptor_DoesNotKeepUnloadableTypes() Assert.NotNull(collectibleAttributeType); Attribute? collectibleAttribute = collectibleType.GetCustomAttribute(collectibleAttributeType); Assert.NotNull(collectibleAttribute); + Type? collectibleProviderType = assembly.GetType("UnloadableTestTypes.SimpleTypeDescriptionProvider"); + Assert.NotNull(collectibleProviderType); // Cache the type's cachable entities AttributeCollection attributes = TypeDescriptor.GetAttributes(collectibleType); @@ -1602,8 +1604,10 @@ public static void TypeDescriptor_DoesNotKeepUnloadableTypes() PropertyDescriptorCollection properties = TypeDescriptor.GetProperties(collectibleType); Assert.Equal(2, properties.Count); + // Test that the provider is the expected collectible one TypeDescriptionProvider provider = TypeDescriptor.GetProvider(collectibleType); Assert.NotNull(provider); + Assert.IsType(collectibleProviderType, provider); // Refresh the assembly causing the ReflectTypeDescriptionProvider caches to be purged. TypeDescriptor.Refresh(assembly); @@ -1613,14 +1617,19 @@ public static void TypeDescriptor_DoesNotKeepUnloadableTypes() Assert.NotSame(attributes, TypeDescriptor.GetAttributes(collectibleType)); Assert.NotSame(events, TypeDescriptor.GetEvents(collectibleType)); Assert.NotSame(properties, TypeDescriptor.GetProperties(collectibleType)); + Assert.NotSame(provider, TypeDescriptor.GetProvider(collectibleType)); }, out var weakRef); + // Force garbage collection to ensure the ALC is unloaded and the types are collected. for (int i = 0; weakRef.IsAlive && i < 10; i++) { GC.Collect(); GC.WaitForPendingFinalizers(); } + + // Assert that the weak reference to the ALC is no longer alive, + // indicating that the unloaded AssemblyLoadContext has been at least collected. Assert.True(!weakRef.IsAlive); } diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.cs index 3a8ec9b74de776..a427010ece6586 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.cs @@ -2,10 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.ComponentModel; namespace UnloadableTestTypes { [SimpleType] + [TypeDescriptionProvider(typeof(SimpleTypeDescriptionProvider))] public class SimpleType { public string P1 { get; set; } @@ -19,4 +21,20 @@ public void OnActionEvent() [AttributeUsage(AttributeTargets.All)] public sealed class SimpleTypeAttribute : Attribute { } + + public sealed class SimpleTypeDescriptionProvider : TypeDescriptionProvider + { + public SimpleTypeDescriptionProvider() { } + + public override ICustomTypeDescriptor GetTypeDescriptor(Type objectType, object instance) + { + var baseDescriptor = base.GetTypeDescriptor(objectType, instance); + return new SimpleTypeDescriptor(baseDescriptor); + } + + private sealed class SimpleTypeDescriptor : CustomTypeDescriptor + { + public SimpleTypeDescriptor(ICustomTypeDescriptor parent) : base(parent) { } + } + } } From 7467087479a0a4da8263630b594dc5a435cd8c45 Mon Sep 17 00:00:00 2001 From: Alexey Zakharov Date: Thu, 10 Jul 2025 19:55:03 +0200 Subject: [PATCH 3/4] Added a test for custom provider and updated WeakHashTable to use ConditionalWeakTable to ensure values can be also colelcted --- .../ComponentModel/ContextAwareHashtable.cs | 9 +- .../System/ComponentModel/TypeDescriptor.cs | 34 +-- .../System/ComponentModel/WeakHashtable.cs | 247 +----------------- .../tests/TypeDescriptorTests.cs | 74 ++++-- .../UnloadableTestTypes.cs | 14 +- 5 files changed, 91 insertions(+), 287 deletions(-) diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs index 70179732256819..c650e2d6c587a3 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs @@ -8,9 +8,12 @@ namespace System.ComponentModel { /// - /// Hashtable that maps MemberInfo object key to an object and - /// uses a WeakReference for the collectible keys (if MemberInfo.IsCollectible is true). - /// Uses a Hashtable for non-collectible keys and WeakHashtable for the collectible keys. + /// Hashtable that maps a object key to an associated value. + /// + /// For keys where is false, a standard is used. + /// For keys where is true, a is used. + /// This ensures that collectible instances (such as those from collectible assemblies) do not prevent their assemblies from being unloaded. + /// /// internal sealed class ContextAwareHashtable { diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs index 475669b1549821..28b3c08e31061a 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections; +using System.Collections.Generic; using System.Collections.Specialized; using System.ComponentModel.Design; using System.Diagnostics; @@ -622,7 +623,7 @@ public static object GetAssociation(Type type, object primary) if (!type.IsInstanceOfType(primary)) { // Check our association table for a match. - Hashtable assocTable = AssociationTable; + WeakHashtable assocTable = AssociationTable; IList? associations = (IList?)assocTable?[primary]; if (associations != null) { @@ -2332,16 +2333,12 @@ private static void Refresh(object component, bool refreshReflectionProvider) // ReflectTypeDescritionProvider is only bound to object, but we // need go to through the entire table to try to find custom // providers. If we find one, will clear our cache. - // Manual use of IDictionaryEnumerator instead of foreach to avoid - // DictionaryEntry box allocations. - IDictionaryEnumerator e = s_providerTable.GetEnumerator(); - while (e.MoveNext()) + foreach (KeyValuePair kvp in s_providerTable) { - DictionaryEntry de = e.Entry; - Type? nodeType = de.Key as Type; + Type? nodeType = kvp.Key as Type; if (nodeType != null && type.IsAssignableFrom(nodeType) || nodeType == typeof(object)) { - TypeDescriptionNode? node = (TypeDescriptionNode?)de.Value; + TypeDescriptionNode? node = (TypeDescriptionNode?)kvp.Value; while (node != null && !(node.Provider is ReflectTypeDescriptionProvider)) { found = true; @@ -2416,16 +2413,12 @@ public static void Refresh(Type type) // ReflectTypeDescritionProvider is only bound to object, but we // need go to through the entire table to try to find custom // providers. If we find one, will clear our cache. - // Manual use of IDictionaryEnumerator instead of foreach to avoid - // DictionaryEntry box allocations. - IDictionaryEnumerator e = s_providerTable.GetEnumerator(); - while (e.MoveNext()) + foreach (KeyValuePair kvp in s_providerTable) { - DictionaryEntry de = e.Entry; - Type? nodeType = de.Key as Type; + Type? nodeType = kvp.Key as Type; if (nodeType != null && type.IsAssignableFrom(nodeType) || nodeType == typeof(object)) { - TypeDescriptionNode? node = (TypeDescriptionNode?)de.Value; + TypeDescriptionNode? node = (TypeDescriptionNode?)kvp.Value; while (node != null && !(node.Provider is ReflectTypeDescriptionProvider)) { found = true; @@ -2478,15 +2471,12 @@ public static void Refresh(Module module) lock (s_commonSyncObject) { - // Manual use of IDictionaryEnumerator instead of foreach to avoid DictionaryEntry box allocations. - IDictionaryEnumerator e = s_providerTable.GetEnumerator(); - while (e.MoveNext()) + foreach (KeyValuePair kvp in s_providerTable) { - DictionaryEntry de = e.Entry; - Type? nodeType = de.Key as Type; + Type? nodeType = kvp.Key as Type; if (nodeType != null && nodeType.Module.Equals(module) || nodeType == typeof(object)) { - TypeDescriptionNode? node = (TypeDescriptionNode?)de.Value; + TypeDescriptionNode? node = (TypeDescriptionNode?)kvp.Value; while (node != null && !(node.Provider is ReflectTypeDescriptionProvider)) { refreshedTypes ??= new Hashtable(); @@ -2628,7 +2618,7 @@ public static void RemoveAssociation(object primary, object secondary) ArgumentNullException.ThrowIfNull(primary); ArgumentNullException.ThrowIfNull(secondary); - Hashtable assocTable = AssociationTable; + WeakHashtable assocTable = AssociationTable; IList? associations = (IList?)assocTable?[primary]; if (associations != null) { diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/WeakHashtable.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/WeakHashtable.cs index c2478bed6915ee..80bba8546a055f 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/WeakHashtable.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/WeakHashtable.cs @@ -3,252 +3,31 @@ using System.Collections; using System.Collections.Generic; +using System.Runtime.CompilerServices; namespace System.ComponentModel { /// - /// This is a hashtable that stores object keys as weak references. - /// It monitors memory usage and will periodically scavenge the - /// hash table to clean out dead references. + /// Provides a hashtable-like collection that stores keys and values using weak references. + /// This class is a thin wrapper around + /// and is especially useful for associating data with objects from unloadable assemblies. /// - internal sealed class WeakHashtable : Hashtable + internal sealed class WeakHashtable : IEnumerable> { - private static readonly IEqualityComparer s_comparer = new WeakKeyComparer(); + private readonly ConditionalWeakTable _hashtable = new ConditionalWeakTable(); - private long _lastGlobalMem; - private int _lastHashCount; - - internal WeakHashtable() : base(s_comparer) - { - } - - /// - /// Adds a key-value pair to the hashtable, wrapping the key in a weak reference. - /// - public override void Add(object key, object? value) - { - ScavengeKeys(); - base.Add(new EqualityWeakReference(key), value); - } - - /// - /// Gets or sets the value associated with the specified key. - /// - public override object? this[object key] - { - get - { - return base[key]; - } - - set - { - ScavengeKeys(); - base[new EqualityWeakReference(key)] = value; - } - } - - /// - /// Override of GetEnumerator that returns enumerator which skips not alive keys - /// during enumeration. - /// - public override IDictionaryEnumerator GetEnumerator() => new WeakHashtableEnumerator(base.GetEnumerator()); - - /// - /// This method checks to see if it is necessary to - /// scavenge keys, and if it is it performs a scan - /// of all keys to see which ones are no longer valid. - /// To determine if we need to scavenge keys we need to - /// try to track the current GC memory. Our rule of - /// thumb is that if GC memory is decreasing and our - /// key count is constant we need to scavenge. We - /// will need to see if this is too often for extreme - /// use cases like the CompactFramework (they add - /// custom type data for every object at design time). - /// - private void ScavengeKeys() - { - int hashCount = Count; - - if (hashCount == 0) - { - return; - } - - if (_lastHashCount == 0) - { - _lastHashCount = hashCount; - return; - } - - long globalMem = GC.GetTotalMemory(false); - - if (_lastGlobalMem == 0) - { - _lastGlobalMem = globalMem; - return; - } - - float memDelta = (globalMem - _lastGlobalMem) / (float)_lastGlobalMem; - float hashDelta = (hashCount - _lastHashCount) / (float)_lastHashCount; - - if (memDelta < 0 && hashDelta >= 0) - { - // Perform a scavenge through our keys, looking - // for dead references. - List? cleanupList = null; - foreach (object o in Keys) - { - if (o is WeakReference wr && !wr.IsAlive) - { - cleanupList ??= new List(); - cleanupList.Add(wr); - } - } - - if (cleanupList != null) - { - foreach (object o in cleanupList) - { - Remove(o); - } - } - } - - _lastGlobalMem = globalMem; - _lastHashCount = hashCount; - } - - private sealed class WeakKeyComparer : IEqualityComparer + public object? this[object key] { - bool IEqualityComparer.Equals(object? x, object? y) - { - if (x == null) - { - return y == null; - } - if (y != null && x.GetHashCode() == y.GetHashCode()) - { - if (x is WeakReference wX) - { - if (!wX.IsAlive) - { - return false; - } - x = wX.Target; - } - - if (y is WeakReference wY) - { - if (!wY.IsAlive) - { - return false; - } - y = wY.Target; - } - - return object.ReferenceEquals(x, y); - } - - return false; - } - - int IEqualityComparer.GetHashCode(object obj) => obj.GetHashCode(); - } - - /// - /// A subclass of WeakReference that overrides GetHashCode and - /// Equals so that the weak reference returns the same equality - /// semantics as the object it wraps. This will always return - /// the object's hash code and will return True for a Equals - /// comparison of the object it is wrapping. If the object - /// it is wrapping has finalized, Equals always returns false. - /// - private sealed class EqualityWeakReference : WeakReference - { - private readonly int _hashCode; - - internal EqualityWeakReference(object o) : base(o) - { - _hashCode = o.GetHashCode(); - } - - public override bool Equals(object? o) - { - if (o?.GetHashCode() != _hashCode) - { - return false; - } - - if (o == this || (IsAlive && ReferenceEquals(o, Target))) - { - return true; - } - - return false; - } - - public override int GetHashCode() => _hashCode; + get => _hashtable.TryGetValue(key, out object? value) ? value : null; + set => _hashtable.AddOrUpdate(key, value); } - /// - /// A wrapper around the base HashTable enumerator that skips - /// collected keys and captures the key value to prevent it - /// from being collected during enumeration. - /// - private sealed class WeakHashtableEnumerator : IDictionaryEnumerator - { - private readonly IDictionaryEnumerator _baseEnumerator; - // Captured key of the WeakHashtable so that it is not collected during enumeration - private object? _currentKey; - - public WeakHashtableEnumerator(IDictionaryEnumerator baseEnumerator) - { - _baseEnumerator = baseEnumerator; - } - - public DictionaryEntry Entry => new DictionaryEntry(Key, Value); - - public object Key - { - get - { - object key = _baseEnumerator.Key; - return ((EqualityWeakReference)key).Target!; - } - } + public bool ContainsKey(object key) => _hashtable.TryGetValue(key, out object? _); - public object? Value => _baseEnumerator.Value; + public void Remove(object key) => _hashtable.Remove(key); - public object Current => Entry; + public IEnumerator> GetEnumerator() => ((IEnumerable>)_hashtable).GetEnumerator(); - public bool MoveNext() - { - while (_baseEnumerator.MoveNext()) - { - // The key must always be an EqualityWeakReference by design. - EqualityWeakReference key = (EqualityWeakReference)_baseEnumerator.Key; - - // If the weak reference is alive, capture it and return true. - _currentKey = key.Target; - if (_currentKey != null) - { - return true; - } - - // Otherwise, the key has been collected and we need to skip it. - } - - // Cleanup the captured key and return false if there is nothing more in the table. - _currentKey = null; - return false; - } - - public void Reset() - { - _baseEnumerator.Reset(); - _currentKey = null; - } - } + IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable>)_hashtable).GetEnumerator(); } } diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs index 6ffe772892939b..00fb9b0b3a5bfd 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs @@ -1567,8 +1567,11 @@ private static void ExecuteAndUnload(string assemblyfile, Action assem var assembly = alc.LoadFromAssemblyPath(fullPath); Assert.NotEqual(AssemblyLoadContext.GetLoadContext(assembly), AssemblyLoadContext.Default); - // Perform action on the assembly from ALC - assemblyAction(assembly); + using (AssemblyLoadContext.ContextualReflectionScope scope = alc.EnterContextualReflection()) + { + // Perform action on the assembly from ALC inside the reflection scope + assemblyAction(assembly); + } // Unload the ALC alc.Unload(); @@ -1579,7 +1582,7 @@ private static void ExecuteAndUnload(string assemblyfile, Action assem // loaded in the default ALC, which causes problems for this test. [SkipOnPlatform(TestPlatforms.Browser, "AssemblyDependencyResolver not supported in wasm")] [ActiveIssue("34072", TestRuntimes.Mono)] - public static void TypeDescriptor_DoesNotKeepUnloadableTypes() + public static void TypeDescriptor_WithDefaultProvider_UnloadsUnloadableTypes() { ExecuteAndUnload("UnloadableTestTypes.dll", static (assembly) => @@ -1591,8 +1594,8 @@ public static void TypeDescriptor_DoesNotKeepUnloadableTypes() Assert.NotNull(collectibleAttributeType); Attribute? collectibleAttribute = collectibleType.GetCustomAttribute(collectibleAttributeType); Assert.NotNull(collectibleAttribute); - Type? collectibleProviderType = assembly.GetType("UnloadableTestTypes.SimpleTypeDescriptionProvider"); - Assert.NotNull(collectibleProviderType); + Type? collectibleTypeDescriptionProviderType = assembly.GetType("UnloadableTestTypes.SimpleTypeDescriptionProvider"); + Assert.NotNull(collectibleTypeDescriptionProviderType); // Cache the type's cachable entities AttributeCollection attributes = TypeDescriptor.GetAttributes(collectibleType); @@ -1603,21 +1606,59 @@ public static void TypeDescriptor_DoesNotKeepUnloadableTypes() PropertyDescriptorCollection properties = TypeDescriptor.GetProperties(collectibleType); Assert.Equal(2, properties.Count); + }, + out var weakRef); + + // Force garbage collection to ensure the ALC is unloaded and the types are collected. + for (int i = 0; weakRef.IsAlive && i < 10; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + + // Assert that the weak reference to the ALC is no longer alive, + // indicating that the unloaded AssemblyLoadContext has been at least collected. + Assert.True(!weakRef.IsAlive); + } + + [Fact] + // Lack of AssemblyDependencyResolver results in assemblies that are not loaded by path to get + // loaded in the default ALC, which causes problems for this test. + [SkipOnPlatform(TestPlatforms.Browser, "AssemblyDependencyResolver not supported in wasm")] + [ActiveIssue("34072", TestRuntimes.Mono)] + public static void TypeDescriptor_WithCustomProvider_UnloadsUnloadableTypes() + { + ExecuteAndUnload("UnloadableTestTypes.dll", + static (assembly) => + { + // Ensure the type loaded in the intended non-Default ALC + Type? collectibleType = assembly.GetType("UnloadableTestTypes.SimpleType"); + Assert.NotNull(collectibleType); + Type? collectibleAttributeType = assembly.GetType("UnloadableTestTypes.SimpleTypeAttribute"); + Assert.NotNull(collectibleAttributeType); + Attribute? collectibleAttribute = collectibleType.GetCustomAttribute(collectibleAttributeType); + Assert.NotNull(collectibleAttribute); + Type? collectibleTypeDescriptionProviderType = assembly.GetType("UnloadableTestTypes.SimpleTypeDescriptionProvider"); + Assert.NotNull(collectibleTypeDescriptionProviderType); + + // Add provider to ensure it is registered and stored in the TypeDescriptor cache + TypeDescriptionProvider collectibleProvider = (TypeDescriptionProvider)Activator.CreateInstance(collectibleTypeDescriptionProviderType); + TypeDescriptor.AddProvider(collectibleProvider, collectibleType); // Test that the provider is the expected collectible one - TypeDescriptionProvider provider = TypeDescriptor.GetProvider(collectibleType); - Assert.NotNull(provider); - Assert.IsType(collectibleProviderType, provider); + AttributeCollection attributes = TypeDescriptor.GetAttributes(collectibleType); + Assert.Empty(attributes); + + EventDescriptorCollection events = TypeDescriptor.GetEvents(collectibleType); + Assert.Empty(events); - // Refresh the assembly causing the ReflectTypeDescriptionProvider caches to be purged. - TypeDescriptor.Refresh(assembly); + PropertyDescriptorCollection properties = TypeDescriptor.GetProperties(collectibleType); + Assert.Empty(properties); - // Tests that the new collections are different - // from the old ones, and that the old ones are not kept alive. - Assert.NotSame(attributes, TypeDescriptor.GetAttributes(collectibleType)); - Assert.NotSame(events, TypeDescriptor.GetEvents(collectibleType)); - Assert.NotSame(properties, TypeDescriptor.GetProperties(collectibleType)); - Assert.NotSame(provider, TypeDescriptor.GetProvider(collectibleType)); + TypeDescriptionProvider provider = TypeDescriptor.GetProvider(collectibleType); + Assert.NotNull(provider); + Assert.True(provider.IsSupportedType(collectibleType)); + Assert.False(provider.IsSupportedType(typeof(int))); }, out var weakRef); @@ -1632,6 +1673,5 @@ public static void TypeDescriptor_DoesNotKeepUnloadableTypes() // indicating that the unloaded AssemblyLoadContext has been at least collected. Assert.True(!weakRef.IsAlive); } - } } diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.cs index a427010ece6586..0547ce9dd5b3e9 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/UnloadableTestTypes/UnloadableTestTypes.cs @@ -7,7 +7,6 @@ namespace UnloadableTestTypes { [SimpleType] - [TypeDescriptionProvider(typeof(SimpleTypeDescriptionProvider))] public class SimpleType { public string P1 { get; set; } @@ -24,17 +23,10 @@ public sealed class SimpleTypeAttribute : Attribute { } public sealed class SimpleTypeDescriptionProvider : TypeDescriptionProvider { - public SimpleTypeDescriptionProvider() { } + public override bool IsSupportedType(Type type) => type.AssemblyQualifiedName == typeof(SimpleType).AssemblyQualifiedName; - public override ICustomTypeDescriptor GetTypeDescriptor(Type objectType, object instance) - { - var baseDescriptor = base.GetTypeDescriptor(objectType, instance); - return new SimpleTypeDescriptor(baseDescriptor); - } + public override ICustomTypeDescriptor GetTypeDescriptor(Type objectType, object instance) => new SimpleTypeDescriptor(); - private sealed class SimpleTypeDescriptor : CustomTypeDescriptor - { - public SimpleTypeDescriptor(ICustomTypeDescriptor parent) : base(parent) { } - } + public sealed class SimpleTypeDescriptor : CustomTypeDescriptor { } } } From 43e50c681bd7bbf893888f2de9f1cf2bd0f28bdd Mon Sep 17 00:00:00 2001 From: Alexey Zakharov Date: Fri, 11 Jul 2025 09:30:53 +0200 Subject: [PATCH 4/4] Renamed ContextAwareHashtable to CollectibleKeyHashtable --- ...System.ComponentModel.TypeConverter.csproj | 2 +- ...ashtable.cs => CollectibleKeyHashtable.cs} | 2 +- .../ReflectTypeDescriptionProvider.cs | 26 +++++++++---------- 3 files changed, 15 insertions(+), 15 deletions(-) rename src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/{ContextAwareHashtable.cs => CollectibleKeyHashtable.cs} (96%) diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj b/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj index 0214ba20d330b5..685abb005cc8fe 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj @@ -45,7 +45,7 @@ - + diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CollectibleKeyHashtable.cs similarity index 96% rename from src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs rename to src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CollectibleKeyHashtable.cs index c650e2d6c587a3..ebd0e85f57ff37 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/CollectibleKeyHashtable.cs @@ -15,7 +15,7 @@ namespace System.ComponentModel /// This ensures that collectible instances (such as those from collectible assemblies) do not prevent their assemblies from being unloaded. /// /// - internal sealed class ContextAwareHashtable + internal sealed class CollectibleKeyHashtable { private readonly Hashtable _defaultTable = new Hashtable(); private readonly ConditionalWeakTable _collectibleTable = new ConditionalWeakTable(); diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs index 44c12d51184eff..a889ae78692168 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs @@ -48,10 +48,10 @@ internal sealed partial class ReflectTypeDescriptionProvider : TypeDescriptionPr // on Control, Component and object are also automatically filled // in. The keys to the property and event caches are types. // The keys to the attribute cache are either MemberInfos or types. - private static ContextAwareHashtable? s_propertyCache; - private static ContextAwareHashtable? s_eventCache; - private static ContextAwareHashtable? s_attributeCache; - private static ContextAwareHashtable? s_extendedPropertyCache; + private static CollectibleKeyHashtable? s_propertyCache; + private static CollectibleKeyHashtable? s_eventCache; + private static CollectibleKeyHashtable? s_attributeCache; + private static CollectibleKeyHashtable? s_extendedPropertyCache; // These are keys we stuff into our object cache. We use this // cache data to store extender provider info for an object. @@ -192,13 +192,13 @@ private static Dictionary IntrinsicTypeConve Justification = "IntrinsicTypeConverters is marked with RequiresUnreferencedCode. It is the only place that should call this.")] private static NullableConverter CreateNullableConverter(Type type) => new NullableConverter(type); - private static ContextAwareHashtable PropertyCache => LazyInitializer.EnsureInitialized(ref s_propertyCache, () => new ContextAwareHashtable()); + private static CollectibleKeyHashtable PropertyCache => LazyInitializer.EnsureInitialized(ref s_propertyCache, () => new CollectibleKeyHashtable()); - private static ContextAwareHashtable EventCache => LazyInitializer.EnsureInitialized(ref s_eventCache, () => new ContextAwareHashtable()); + private static CollectibleKeyHashtable EventCache => LazyInitializer.EnsureInitialized(ref s_eventCache, () => new CollectibleKeyHashtable()); - private static ContextAwareHashtable AttributeCache => LazyInitializer.EnsureInitialized(ref s_attributeCache, () => new ContextAwareHashtable()); + private static CollectibleKeyHashtable AttributeCache => LazyInitializer.EnsureInitialized(ref s_attributeCache, () => new CollectibleKeyHashtable()); - private static ContextAwareHashtable ExtendedPropertyCache => LazyInitializer.EnsureInitialized(ref s_extendedPropertyCache, () => new ContextAwareHashtable()); + private static CollectibleKeyHashtable ExtendedPropertyCache => LazyInitializer.EnsureInitialized(ref s_extendedPropertyCache, () => new CollectibleKeyHashtable()); /// Clear the global caches this maintains on top of reflection. internal static void ClearReflectionCaches() @@ -1095,7 +1095,7 @@ internal bool IsPopulated(Type type) /// internal static Attribute[] ReflectGetAttributes(Type type) { - ContextAwareHashtable attributeCache = AttributeCache; + CollectibleKeyHashtable attributeCache = AttributeCache; Attribute[]? attrs = (Attribute[]?)attributeCache[type]; if (attrs != null) { @@ -1123,7 +1123,7 @@ internal static Attribute[] ReflectGetAttributes(Type type) /// internal static Attribute[] ReflectGetAttributes(MemberInfo member) { - ContextAwareHashtable attributeCache = AttributeCache; + CollectibleKeyHashtable attributeCache = AttributeCache; Attribute[]? attrs = (Attribute[]?)attributeCache[member]; if (attrs != null) { @@ -1151,7 +1151,7 @@ internal static Attribute[] ReflectGetAttributes(MemberInfo member) /// private static EventDescriptor[] ReflectGetEvents(Type type) { - ContextAwareHashtable eventCache = EventCache; + CollectibleKeyHashtable eventCache = EventCache; EventDescriptor[]? events = (EventDescriptor[]?)eventCache[type]; if (events != null) { @@ -1251,7 +1251,7 @@ private static PropertyDescriptor[] ReflectGetExtendedProperties(IExtenderProvid // property store. // Type providerType = provider.GetType(); - ContextAwareHashtable extendedPropertyCache = ExtendedPropertyCache; + CollectibleKeyHashtable extendedPropertyCache = ExtendedPropertyCache; ReflectPropertyDescriptor[]? extendedProperties = (ReflectPropertyDescriptor[]?)extendedPropertyCache[providerType]; if (extendedProperties == null) { @@ -1336,7 +1336,7 @@ private static PropertyDescriptor[] ReflectGetPropertiesFromRegisteredType(Type private static PropertyDescriptor[] ReflectGetPropertiesImpl(Type type) { - ContextAwareHashtable propertyCache = PropertyCache; + CollectibleKeyHashtable propertyCache = PropertyCache; PropertyDescriptor[]? properties = (PropertyDescriptor[]?)propertyCache[type]; if (properties != null) {