Bugfix/30024 problem with concurrency type descriptor getconverter

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;
@@ -28,7 +29,7 @@ public sealed class TypeDescriptor
  // class load anyway.
  private static readonly WeakHashtable s_providerTable = new WeakHashtable(); // mapping of type or object hash to a provider list
  private static readonly Hashtable s_providerTypeTable = new Hashtable(); // A direct mapping from type to provider.
- private static readonly Hashtable s_defaultProviders = new Hashtable(); // A table of type -> default provider to track DefaultTypeDescriptionProviderAttributes.
+ private static readonly Dictionary<Type, ResetEventContext> s_processedTypes = new Dictionary<Type, ResetEventContext>();
  private static WeakHashtable? s_associationTable;
  private static int s_metadataVersion; // a version stamp for our metadata. Used by property descriptors to know when to rebuild attributes.
 
@@ -75,7 +76,8 @@ public sealed class TypeDescriptor
  Guid.NewGuid() // events
  };
 
- private static readonly object s_internalSyncObject = new object();
+ private static readonly object s_processedTypesLock = new object();
+ private static readonly ReaderWriterLockSlim s_providerTableLock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);
 
  private TypeDescriptor()
  {
@@ -176,7 +178,8 @@ public static void AddProvider(TypeDescriptionProvider provider, Type type)
  ArgumentNullException.ThrowIfNull(provider);
  ArgumentNullException.ThrowIfNull(type);
 
- lock (s_providerTable)
+ s_providerTableLock.EnterWriteLock();
+ try
  {
  // Get the root node, hook it up, and stuff it back into
  // the provider cache.
@@ -185,6 +188,10 @@ public static void AddProvider(TypeDescriptionProvider provider, Type type)
  s_providerTable[type] = head;
  s_providerTypeTable.Clear();
  }
+ finally
+ {
+ s_providerTableLock.ExitWriteLock();
+ }
 
  Refresh(type);
  }
@@ -204,15 +211,26 @@ public static void AddProvider(TypeDescriptionProvider provider, object instance
 
  bool refreshNeeded;
 
- // Get the root node, hook it up, and stuff it back into
- // the provider cache.
- lock (s_providerTable)
+ s_providerTableLock.EnterUpgradeableReadLock();
+ try
  {
  refreshNeeded = s_providerTable.ContainsKey(instance);
  TypeDescriptionNode node = NodeFor(instance, true);
  var head = new TypeDescriptionNode(provider) { Next = node };
- s_providerTable.SetWeak(instance, head);
- s_providerTypeTable.Clear();
+ s_providerTableLock.EnterWriteLock();
+ try
+ {
+ s_providerTable.SetWeak(instance, head);
+ s_providerTypeTable.Clear();
+ }
+ finally
+ {
+ s_providerTableLock.ExitWriteLock();
+ }
+ }
+ finally
+ {
+ s_providerTableLock.ExitUpgradeableReadLock();
  }
 
  if (refreshNeeded)
@@ -262,51 +280,49 @@ public static void AddProviderTransparent(TypeDescriptionProvider provider, obje
  /// </summary>
  private static void CheckDefaultProvider(Type type)
  {
- if (s_defaultProviders.ContainsKey(type))
+ if (!s_processedTypes.TryGetValue(type, out ResetEventContext? context))
  {
- return;
- }
-
- lock (s_internalSyncObject)
- {
- if (s_defaultProviders.ContainsKey(type))
+ lock (s_processedTypesLock)
  {
- return;
- }
+ if (!s_processedTypes.ContainsKey(type))
+ {
+ ManualResetEvent resetEventForAdd = new ManualResetEvent(false);
+ ResetEventContext contextForAdd = new ResetEventContext(resetEventForAdd, Environment.CurrentManagedThreadId);
+ s_processedTypes.Add(type, contextForAdd);
 
- // Immediately clear this. If we find a default provider
- // and it starts messing around with type information,
- // this could infinitely recurse.
- s_defaultProviders[type] = null;
- }
+ var providerAttr = type.GetCustomAttribute<TypeDescriptionProviderAttribute>(false);
+ bool providerAdded = false;
 
- // Always use core reflection when checking for
- // the default provider attribute. If there is a
- // provider, we probably don't want to build up our
- // own cache state against the type. There shouldn't be
- // more than one of these, but walk anyway. Walk in
- // reverse order so that the most derived takes precidence.
- object[] attrs = type.GetCustomAttributes(typeof(TypeDescriptionProviderAttribute), false);
- bool providerAdded = false;
- for (int idx = attrs.Length - 1; idx >= 0; idx--)
- {
- TypeDescriptionProviderAttribute pa = (TypeDescriptionProviderAttribute)attrs[idx];
- Type? providerType = Type.GetType(pa.TypeName);
- if (providerType != null && typeof(TypeDescriptionProvider).IsAssignableFrom(providerType))
- {
- TypeDescriptionProvider prov = (TypeDescriptionProvider)Activator.CreateInstance(providerType)!;
- AddProvider(prov, type);
- providerAdded = true;
+ if (providerAttr != null)
+ {
+ Type? providerType = Type.GetType(providerAttr.TypeName);
+ if (providerType != null && typeof(TypeDescriptionProvider).IsAssignableFrom(providerType))
+ {
+ TypeDescriptionProvider provider = (TypeDescriptionProvider)Activator.CreateInstance(providerType)!;
+ AddProvider(provider, type);
+ providerAdded = true;
+ }
+ }
+
+ // If we did not add a provider, check the base class.
+ if (!providerAdded)
+ {
+ Type? baseType = type.BaseType;
+ if (baseType != null && baseType != type)
+ {
+ CheckDefaultProvider(baseType);
+ }
+ }
+
+ resetEventForAdd.Set();
+ }
  }
  }
-
- // If we did not add a provider, check the base class.
- if (!providerAdded)
+ else
  {
- Type? baseType = type.BaseType;
- if (baseType != null && baseType != type)
+ if (context.OwnerId != Environment.CurrentManagedThreadId)
  {
-CheckDefaultProvider(baseType);
+context.ResetEvent.WaitOne();
  }
  }
  }
@@ -1457,45 +1473,63 @@ private static TypeDescriptionNode NodeFor(Type type, bool createDelegator)
  TypeDescriptionNode? node = null;
  Type searchType = type;
 
- while (node == null)
+ s_providerTableLock.EnterUpgradeableReadLock();
+ try
  {
- node = (TypeDescriptionNode?)s_providerTypeTable[searchType] ??
- (TypeDescriptionNode?)s_providerTable[searchType];
-
- if (node == null)
+ while (node == null)
  {
- Type? baseType = GetNodeForBaseType(searchType);
+ node = (TypeDescriptionNode?)s_providerTypeTable[searchType] ??
+ (TypeDescriptionNode?)s_providerTable[searchType];
 
- if (searchType == typeof(object) || baseType == null)
+ if (node == null)
  {
- lock (s_providerTable)
+ Type? baseType = GetNodeForBaseType(searchType);
+
+ if (searchType == typeof(object) || baseType == null)
  {
- node = (TypeDescriptionNode?)s_providerTable[searchType];
+ s_providerTableLock.EnterWriteLock();
+ try
+ {
+ node = (TypeDescriptionNode?)s_providerTable[searchType];
 
- if (node == null)
+ if (node == null)
+ {
+ // The reflect type description provider is a default provider that
+ // can provide type information for all objects.
+ node = new TypeDescriptionNode(new ReflectTypeDescriptionProvider());
+ s_providerTable[searchType] = node;
+ }
+ }
+ finally
  {
- // The reflect type description provider is a default provider that
- // can provide type information for all objects.
- node = new TypeDescriptionNode(new ReflectTypeDescriptionProvider());
- s_providerTable[searchType] = node;
+ s_providerTableLock.ExitWriteLock();
  }
  }
- }
- else if (createDelegator)
- {
- node = new TypeDescriptionNode(new DelegatingTypeDescriptionProvider(baseType));
- lock (s_providerTable)
+ else if (createDelegator)
  {
- s_providerTypeTable[searchType] = node;
+ node = new TypeDescriptionNode(new DelegatingTypeDescriptionProvider(baseType));
+ s_providerTableLock.EnterWriteLock();
+ try
+ {
+ s_providerTypeTable[searchType] = node;
+ }
+ finally
+ {
+ s_providerTableLock.ExitWriteLock();
+ }
+ }
+ else
+ {
+ // Continue our search
+ searchType = baseType;
  }
- }
- else
- {
- // Continue our search
- searchType = baseType;
  }
  }
  }
+ finally
+ {
+ s_providerTableLock.ExitUpgradeableReadLock();
+ }
 
  return node;
  }
@@ -1587,7 +1621,8 @@ private static TypeDescriptionNode NodeFor(object instance, bool createDelegator
  /// </summary>
  private static void NodeRemove(object key, TypeDescriptionProvider provider)
  {
- lock (s_providerTable)
+ s_providerTableLock.EnterUpgradeableReadLock();
+ try
  {
  TypeDescriptionNode? head = (TypeDescriptionNode?)s_providerTable[key];
  TypeDescriptionNode? target = head;
@@ -1623,7 +1658,15 @@ private static void NodeRemove(object key, TypeDescriptionProvider provider)
  if (target == head && target.Provider is DelegatingTypeDescriptionProvider)
  {
  Debug.Assert(target.Next == null, "Delegating provider should always be the last provider in the chain.");
- s_providerTable.Remove(key);
+ s_providerTableLock.EnterWriteLock();
+ try
+ {
+ s_providerTable.Remove(key);
+ }
+ finally
+ {
+ s_providerTableLock.ExitWriteLock();
+ }
  }
  }
  else if (target != head)
@@ -1645,14 +1688,26 @@ private static void NodeRemove(object key, TypeDescriptionProvider provider)
  }
  else
  {
- s_providerTable.Remove(key);
+ s_providerTableLock.EnterWriteLock();
+ try
+ {
+ s_providerTable.Remove(key);
+ }
+ finally
+ {
+ s_providerTableLock.ExitWriteLock();
+ }
  }
 
  // Finally, clear our cache of provider types; it might be invalid
  // now.
  s_providerTypeTable.Clear();
  }
  }
+ finally
+ {
+ s_providerTableLock.ExitUpgradeableReadLock();
+ }
  }
 
  /// <summary>
@@ -2107,8 +2162,9 @@ private static void Refresh(object component, bool refreshReflectionProvider)
  if (refreshReflectionProvider)
  {
  Type type = component.GetType();
+ s_providerTableLock.EnterReadLock();
 
-lock (s_providerTable)
+try
  {
  // ReflectTypeDescritionProvider is only bound to object, but we
  // need go to through the entire table to try to find custom
@@ -2141,6 +2197,10 @@ private static void Refresh(object component, bool refreshReflectionProvider)
  }
  }
  }
+ finally
+ {
+ s_providerTableLock.ExitReadLock();
+ }
  }
 
  // We need to clear our filter even if no typedescriptionprovider had data.
@@ -2192,7 +2252,8 @@ public static void Refresh(Type type)
 
  bool found = false;
 
- lock (s_providerTable)
+ s_providerTableLock.EnterReadLock();
+ try
  {
  // ReflectTypeDescritionProvider is only bound to object, but we
  // need go to through the entire table to try to find custom
@@ -2225,6 +2286,10 @@ public static void Refresh(Type type)
  }
  }
  }
+ finally
+ {
+ s_providerTableLock.ExitReadLock();
+ }
 
  // We only clear our filter and fire the refresh event if there was one or
  // more type description providers that were populated with metdata.
@@ -2257,7 +2322,8 @@ public static void Refresh(Module module)
  // each of these levels.
  Hashtable? refreshedTypes = null;
 
- lock (s_providerTable)
+ s_providerTableLock.EnterReadLock();
+ try
  {
  // Manual use of IDictionaryEnumerator instead of foreach to avoid DictionaryEntry box allocations.
  IDictionaryEnumerator e = s_providerTable.GetEnumerator();
@@ -2290,6 +2356,10 @@ public static void Refresh(Module module)
  }
  }
  }
+ finally
+ {
+ s_providerTableLock.ExitReadLock();
+ }
 
  // And raise the event if types were refresh and handlers are attached.
  if (refreshedTypes != null && Refreshed != null)
@@ -2996,6 +3066,19 @@ PropertyDescriptorCollection ICustomTypeDescriptor.GetProperties(Attribute[]? at
  }
  }
 
+ private sealed class ResetEventContext
+ {
+ public ResetEventContext(ManualResetEvent resetEvent, int ownerId)
+ {
+ ResetEvent = resetEvent;
+ OwnerId = ownerId;
+ }
+
+ public ManualResetEvent ResetEvent { get; }
+
+ public int OwnerId { get; }
+ }
+
  /// <summary>
  /// This is a linked list node that is comprised of a type
  /// description provider. Each node contains a Next pointer