Added a test for custom provider and updated WeakHashTable to use ConditionalWeakTable to ensure values can be also colelcted

Author: alexey-zakharov

src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ContextAwareHashtable.cs

@@ -8,9 +8,12 @@
 namespace System.ComponentModel
 {
     /// <summary>
-    /// 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 <see cref="MemberInfo"/> object key to an associated value.
+    /// <para>
+    /// For keys where <see cref="MemberInfo.IsCollectible"/> is <c>false</c>, a standard <see cref="Hashtable"/> is used.
+    /// For keys where <see cref="MemberInfo.IsCollectible"/> is <c>true</c>, a <see cref="ConditionalWeakTable{TKey, TValue}"/> is used.
+    /// This ensures that collectible <see cref="MemberInfo"/> instances (such as those from collectible assemblies) do not prevent their assemblies from being unloaded.
+    /// </para>
     /// </summary>
     internal sealed class ContextAwareHashtable
     {

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;
@@ -288,7 +289,7 @@ public static void AddProvider(TypeDescriptionProvider provider, object instance
             // the provider cache.
             lock (s_commonSyncObject)
             {
-                refreshNeeded = s_providerTable.ContainsKey(instance);
+                refreshNeeded = s_providerTable.Contains(instance);
                 TypeDescriptionNode node = NodeFor(instance, true);
                 var head = new TypeDescriptionNode(provider) { Next = node };
                 s_providerTable[instance] = head;
@@ -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)
                 {
@@ -2334,14 +2335,12 @@ private static void Refresh(object component, bool refreshReflectionProvider)
                     // 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<object, object?> 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;
@@ -2418,14 +2417,12 @@ public static void Refresh(Type type)
                 // 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<object, object?> 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 +2475,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<object, object?> 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 +2622,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)
             {

src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/WeakHashtable.cs

@@ -3,252 +3,32 @@
 
 using System.Collections;
 using System.Collections.Generic;
+using System.Linq;
+using System.Runtime.CompilerServices;
 
 namespace System.ComponentModel
 {
     /// <summary>
-    /// 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 <see cref="ConditionalWeakTable{TKey, TValue}"/>
+    /// and is especially useful for associating data with objects from unloadable assemblies.
     /// </summary>
-    internal sealed class WeakHashtable : Hashtable
+    internal sealed class WeakHashtable : IEnumerable<KeyValuePair<object, object?>>
     {
-        private static readonly IEqualityComparer s_comparer = new WeakKeyComparer();
+        private readonly ConditionalWeakTable<object, object?> _hashtable = new ConditionalWeakTable<object, object?>();
 
-        private long _lastGlobalMem;
-        private int _lastHashCount;
-
-        internal WeakHashtable() : base(s_comparer)
-        {
-        }
-
-        /// <summary>
-        /// Adds a key-value pair to the hashtable, wrapping the key in a weak reference.
-        /// </summary>
-        public override void Add(object key, object? value)
-        {
-            ScavengeKeys();
-            base.Add(new EqualityWeakReference(key), value);
-        }
-
-        /// <summary>
-        /// Gets or sets the value associated with the specified key.
-        /// </summary>
-        public override object? this[object key]
-        {
-            get
-            {
-                return base[key];
-            }
-
-            set
-            {
-                ScavengeKeys();
-                base[new EqualityWeakReference(key)] = value;
-            }
-        }
-
-        /// <summary>
-        /// Override of GetEnumerator that returns enumerator which skips not alive keys
-        /// during enumeration.
-        /// </summary>
-        public override IDictionaryEnumerator GetEnumerator() => new WeakHashtableEnumerator(base.GetEnumerator());
-
-        /// <summary>
-        /// 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).
-        /// </summary>
-        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<object>? cleanupList = null;
-                foreach (object o in Keys)
-                {
-                    if (o is WeakReference wr && !wr.IsAlive)
-                    {
-                        cleanupList ??= new List<object>();
-                        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();
-        }
-
-        /// <summary>
-        /// 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.
-        /// </summary>
-        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);
         }
 
-        /// <summary>
-        /// A wrapper around the base HashTable enumerator that skips
-        /// collected keys and captures the key value to prevent it
-        /// from being collected during enumeration.
-        /// </summary>
-        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 Contains(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<KeyValuePair<object, object?>> GetEnumerator() => ((IEnumerable<KeyValuePair<object, object?>>)_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<KeyValuePair<object, object?>>)_hashtable).GetEnumerator();
     }
 }