Skip to content

Bugfix/30024 problem with concurrency type descriptor getconverter #85156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

Bugfix/30024 problem with concurrency type descriptor getconverter #85156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6360220
fix(30024): Fix problem with concurrency of TypeDescriptor.
Apr 21, 2023
7d1336d
fix(30024): Little performance improvements.
Maximys May 13, 2023
12c9c82
fix(30024): One more performance improvements.
Jun 30, 2023
5f80bd5
fix(30024): Switching to iterating over array.
Jul 24, 2023
1f20655
fix(30024): Removing needless loop.
Jul 24, 2023
f912fda
refactor(30024): Simplifying value declarations.
Aug 7, 2023
f7deebe
refactor(30024): Changing "GetCustomAttributes" to "GetCustomAttribute".
Aug 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
235 changes: 159 additions & 76 deletions ...libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.

Expand Down Expand Up @@ -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()
{
Expand Down Expand Up @@ -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.
Expand All @@ -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);
}
Expand All @@ -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)
Expand Down Expand Up @@ -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();
}
}
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
Loading