From 56fd769602697fc4f6eba1c730420e3e7277345b Mon Sep 17 00:00:00 2001 From: Gan Keyu Date: Sat, 23 Sep 2023 15:57:37 +0800 Subject: [PATCH 1/8] minimal entirety lock to fix bug --- .../System/ComponentModel/TypeDescriptor.cs | 79 ++++++++++--------- 1 file changed, 42 insertions(+), 37 deletions(-) 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 2efd53e6223ae7..ba0b88f1d3a9e6 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs @@ -1446,58 +1446,62 @@ public static Type GetReflectionType(object instance) private static TypeDescriptionNode NodeFor(Type type, bool createDelegator) { Debug.Assert(type != null, "Caller should validate"); - CheckDefaultProvider(type); - // First, check our provider type table to see if we have a matching - // provider for this type. The provider type table is a cache that - // matches types to providers. When a new provider is added or - // an existing one removed, the provider type table is torn - // down and automatically rebuilt on demand. - // - TypeDescriptionNode? node = null; - Type searchType = type; - - while (node == null) + lock (s_internalSyncObject) { - node = (TypeDescriptionNode?)s_providerTypeTable[searchType] ?? - (TypeDescriptionNode?)s_providerTable[searchType]; + CheckDefaultProvider(type); - if (node == null) + // First, check our provider type table to see if we have a matching + // provider for this type. The provider type table is a cache that + // matches types to providers. When a new provider is added or + // an existing one removed, the provider type table is torn + // down and automatically rebuilt on demand. + // + TypeDescriptionNode? node = null; + Type searchType = type; + + 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]; + lock (s_providerTable) + { + 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; + } + } + } + else if (createDelegator) + { + node = new TypeDescriptionNode(new DelegatingTypeDescriptionProvider(baseType)); + lock (s_providerTable) { - // 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_providerTypeTable[searchType] = node; } } - } - else if (createDelegator) - { - node = new TypeDescriptionNode(new DelegatingTypeDescriptionProvider(baseType)); - lock (s_providerTable) + else { - s_providerTypeTable[searchType] = node; + // Continue our search + searchType = baseType; } } - else - { - // Continue our search - searchType = baseType; - } } - } - return node; + return node; + } } /// @@ -1560,7 +1564,8 @@ private static TypeDescriptionNode NodeFor(object instance, bool createDelegator } else { - node = NodeFor(type); + //lock (s_internalSyncObject) + node = NodeFor(type); } } From 47d9fe658c624599e159ad9a1ba0e27d919dd7c1 Mon Sep 17 00:00:00 2001 From: Gan Keyu Date: Sat, 23 Sep 2023 16:00:42 +0800 Subject: [PATCH 2/8] remove unused code --- .../src/System/ComponentModel/TypeDescriptor.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 ba0b88f1d3a9e6..1273dec3d1ab87 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs @@ -1447,7 +1447,7 @@ private static TypeDescriptionNode NodeFor(Type type, bool createDelegator) { Debug.Assert(type != null, "Caller should validate"); - lock (s_internalSyncObject) + lock (s_providerTable) { CheckDefaultProvider(type); @@ -1564,8 +1564,7 @@ private static TypeDescriptionNode NodeFor(object instance, bool createDelegator } else { - //lock (s_internalSyncObject) - node = NodeFor(type); + node = NodeFor(type); } } From 7b743c3799679b0e2d68a4593a62f16e0360ca1f Mon Sep 17 00:00:00 2001 From: Gan Keyu Date: Sat, 23 Sep 2023 16:18:05 +0800 Subject: [PATCH 3/8] minimize lock range --- .../System/ComponentModel/TypeDescriptor.cs | 116 ++++++++---------- 1 file changed, 54 insertions(+), 62 deletions(-) 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 1273dec3d1ab87..96d26259c8151e 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs @@ -262,10 +262,7 @@ public static void AddProviderTransparent(TypeDescriptionProvider provider, obje /// private static void CheckDefaultProvider(Type type) { - if (s_defaultProviders.ContainsKey(type)) - { - return; - } + bool providerAdded = false; lock (s_internalSyncObject) { @@ -278,25 +275,24 @@ private static void CheckDefaultProvider(Type type) // and it starts messing around with type information, // this could infinitely recurse. s_defaultProviders[type] = null; - } - // 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; + // 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 precedence. + object[] attrs = type.GetCustomAttributes(typeof(TypeDescriptionProviderAttribute), 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; + } } } @@ -1446,62 +1442,58 @@ public static Type GetReflectionType(object instance) private static TypeDescriptionNode NodeFor(Type type, bool createDelegator) { Debug.Assert(type != null, "Caller should validate"); + CheckDefaultProvider(type); - lock (s_providerTable) - { - CheckDefaultProvider(type); + // First, check our provider type table to see if we have a matching + // provider for this type. The provider type table is a cache that + // matches types to providers. When a new provider is added or + // an existing one removed, the provider type table is torn + // down and automatically rebuilt on demand. + // + TypeDescriptionNode? node = null; + Type searchType = type; - // First, check our provider type table to see if we have a matching - // provider for this type. The provider type table is a cache that - // matches types to providers. When a new provider is added or - // an existing one removed, the provider type table is torn - // down and automatically rebuilt on demand. - // - TypeDescriptionNode? node = null; - Type searchType = type; + while (node == null) + { + node = (TypeDescriptionNode?)s_providerTypeTable[searchType] ?? + (TypeDescriptionNode?)s_providerTable[searchType]; - while (node == null) + if (node == null) { - node = (TypeDescriptionNode?)s_providerTypeTable[searchType] ?? - (TypeDescriptionNode?)s_providerTable[searchType]; + Type? baseType = GetNodeForBaseType(searchType); - if (node == null) + if (searchType == typeof(object) || baseType == null) { - Type? baseType = GetNodeForBaseType(searchType); - - if (searchType == typeof(object) || baseType == null) + lock (s_providerTable) { - lock (s_providerTable) - { - node = (TypeDescriptionNode?)s_providerTable[searchType]; + node = (TypeDescriptionNode?)s_providerTable[searchType]; - 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; - } - } - } - else if (createDelegator) - { - node = new TypeDescriptionNode(new DelegatingTypeDescriptionProvider(baseType)); - lock (s_providerTable) + if (node == null) { - s_providerTypeTable[searchType] = node; + // 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; } } - else + } + else if (createDelegator) + { + node = new TypeDescriptionNode(new DelegatingTypeDescriptionProvider(baseType)); + lock (s_providerTable) { - // Continue our search - searchType = baseType; + s_providerTypeTable[searchType] = node; } } + else + { + // Continue our search + searchType = baseType; + } } - - return node; } + + return node; } /// From ba1e17688e83e4f8d76364cf307985b2ee180b79 Mon Sep 17 00:00:00 2001 From: Gan Keyu Date: Sat, 23 Sep 2023 16:57:41 +0800 Subject: [PATCH 4/8] add test --- .../tests/ConcurrentTypeDescriptorTests.cs | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 src/libraries/System.ComponentModel.TypeConverter/tests/ConcurrentTypeDescriptorTests.cs diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/ConcurrentTypeDescriptorTests.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/ConcurrentTypeDescriptorTests.cs new file mode 100644 index 00000000000000..2d9dec0eb85675 --- /dev/null +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/ConcurrentTypeDescriptorTests.cs @@ -0,0 +1,110 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Xunit; + +namespace System.ComponentModel.Tests +{ + [Collection(nameof(DisableParallelization))] // manipulates cache + public class ConcurrentTypeDescriptorTests + { + private long _error = 0; + private bool Error + { + get => Interlocked.Read(ref _error) == 1; + set => Interlocked.Exchange(ref _error, value ? 1 : 0); + } + private void ConcurrentTest(SomeType instance) + { + var properties = TypeDescriptor.GetProperties(instance); + Thread.Sleep(10); + if (properties.Count > 0) + { + Error = true; + } + } + + [Fact] + public void GetProperties_ReturnsExpected() + { + const int Timeout = 60000; + int concurrentCount = Environment.ProcessorCount * 2; + + using var finished = new CountdownEvent(concurrentCount); + + var instances = new SomeType[concurrentCount]; + for (int i = 0; i < concurrentCount; i++) + { + instances[i] = new SomeType(); + } + + for (int i = 0; i < concurrentCount; i++) + { + int i2 = i; + new Thread(() => + { + ConcurrentTest(instances[i2]); + finished.Signal(); + }).Start(); + } + + finished.Wait(Timeout); + + if (finished.CurrentCount != 0) + { + Assert.Fail("Timeout. Possible deadlock."); + } + else + { + Assert.False(Error, "Fallback type descriptor is used."); + } + } + internal class SomeTypeProvider : TypeDescriptionProvider + { + public static ThreadLocal Constructed = new ThreadLocal(); + public static ThreadLocal GetPropertiesCalled = new ThreadLocal(); + private class CTD : ICustomTypeDescriptor + { + public AttributeCollection GetAttributes() => AttributeCollection.Empty; + public string? GetClassName() => null; + public string? GetComponentName() => null; + public TypeConverter GetConverter() => new TypeConverter(); + public EventDescriptor? GetDefaultEvent() => null; + public PropertyDescriptor? GetDefaultProperty() => null; + public object? GetEditor(Type editorBaseType) => null; + public EventDescriptorCollection GetEvents() => EventDescriptorCollection.Empty; + public EventDescriptorCollection GetEvents(Attribute[]? attributes) => EventDescriptorCollection.Empty; + + public PropertyDescriptorCollection GetProperties() + { + GetPropertiesCalled.Value = true; + return PropertyDescriptorCollection.Empty; + } + + public PropertyDescriptorCollection GetProperties(Attribute[]? attributes) + { + throw new NotImplementedException(); + } + + public object? GetPropertyOwner(PropertyDescriptor? pd) => null; + } + public override ICustomTypeDescriptor GetTypeDescriptor(Type objectType, object? instance) + { + Constructed.Value = true; + return new CTD(); + } + } + + [TypeDescriptionProvider(typeof(SomeTypeProvider))] + internal sealed class SomeType + { + public int SomeProperty { get; set; } + } + } +} From 49aef1a35a65e470b1c6d8216dd5b7b25c6c7879 Mon Sep 17 00:00:00 2001 From: Gan Keyu Date: Sat, 23 Sep 2023 17:00:08 +0800 Subject: [PATCH 5/8] fix code style --- .../tests/ConcurrentTypeDescriptorTests.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/ConcurrentTypeDescriptorTests.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/ConcurrentTypeDescriptorTests.cs index 2d9dec0eb85675..e75c0210c6e67b 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/ConcurrentTypeDescriptorTests.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/ConcurrentTypeDescriptorTests.cs @@ -20,6 +20,7 @@ private bool Error get => Interlocked.Read(ref _error) == 1; set => Interlocked.Exchange(ref _error, value ? 1 : 0); } + private void ConcurrentTest(SomeType instance) { var properties = TypeDescriptor.GetProperties(instance); @@ -65,7 +66,8 @@ public void GetProperties_ReturnsExpected() Assert.False(Error, "Fallback type descriptor is used."); } } - internal class SomeTypeProvider : TypeDescriptionProvider + + private class SomeTypeProvider : TypeDescriptionProvider { public static ThreadLocal Constructed = new ThreadLocal(); public static ThreadLocal GetPropertiesCalled = new ThreadLocal(); @@ -102,7 +104,7 @@ public override ICustomTypeDescriptor GetTypeDescriptor(Type objectType, object? } [TypeDescriptionProvider(typeof(SomeTypeProvider))] - internal sealed class SomeType + private sealed class SomeType { public int SomeProperty { get; set; } } From a8a259c626a401d8df2e681901ddada4f324e7b4 Mon Sep 17 00:00:00 2001 From: Gan Keyu Date: Sat, 23 Sep 2023 18:22:35 +0800 Subject: [PATCH 6/8] Make `TypeDescriptor` thread safe with custom providers Originally a race condition exists in `CheckDefaultProvider` and leads to wrong results when many methods are called simultaneously. The PR fixes that by extending the lock statement. Fix #92934 --- .../src/System/ComponentModel/TypeDescriptor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 96d26259c8151e..b3a6851487d486 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs @@ -1456,7 +1456,7 @@ private static TypeDescriptionNode NodeFor(Type type, bool createDelegator) while (node == null) { node = (TypeDescriptionNode?)s_providerTypeTable[searchType] ?? - (TypeDescriptionNode?)s_providerTable[searchType]; + (TypeDescriptionNode?)s_providerTable[searchType]; if (node == null) { From e4c4296078ea36931550b0029c0f70f16be18ef6 Mon Sep 17 00:00:00 2001 From: Gan Keyu Date: Sat, 23 Sep 2023 18:47:52 +0800 Subject: [PATCH 7/8] Originally a race condition exists in `CheckDefaultProvider` and leads to wrong results when many methods are called simultaneously. The PR fixes that by extending the lock statement. Fix #92394 --- .../tests/ConcurrentTypeDescriptorTests.cs | 46 +++++++++---------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/ConcurrentTypeDescriptorTests.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/ConcurrentTypeDescriptorTests.cs index e75c0210c6e67b..2d125e7d261d1f 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/ConcurrentTypeDescriptorTests.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/ConcurrentTypeDescriptorTests.cs @@ -21,7 +21,7 @@ private bool Error set => Interlocked.Exchange(ref _error, value ? 1 : 0); } - private void ConcurrentTest(SomeType instance) + private void ConcurrentTest(TypeWithProperty instance) { var properties = TypeDescriptor.GetProperties(instance); Thread.Sleep(10); @@ -39,10 +39,10 @@ public void GetProperties_ReturnsExpected() using var finished = new CountdownEvent(concurrentCount); - var instances = new SomeType[concurrentCount]; + var instances = new TypeWithProperty[concurrentCount]; for (int i = 0; i < concurrentCount; i++) { - instances[i] = new SomeType(); + instances[i] = new TypeWithProperty(); } for (int i = 0; i < concurrentCount; i++) @@ -63,50 +63,48 @@ public void GetProperties_ReturnsExpected() } else { - Assert.False(Error, "Fallback type descriptor is used."); + Assert.False(Error, "Fallback type descriptor is used. Possible race condition."); } } - private class SomeTypeProvider : TypeDescriptionProvider + public sealed class EmptyPropertiesTypeProvider : TypeDescriptionProvider { - public static ThreadLocal Constructed = new ThreadLocal(); - public static ThreadLocal GetPropertiesCalled = new ThreadLocal(); - private class CTD : ICustomTypeDescriptor + private sealed class EmptyPropertyListDescriptor : ICustomTypeDescriptor { public AttributeCollection GetAttributes() => AttributeCollection.Empty; + public string? GetClassName() => null; + public string? GetComponentName() => null; - public TypeConverter GetConverter() => new TypeConverter(); + + public TypeConverter? GetConverter() => new TypeConverter(); + public EventDescriptor? GetDefaultEvent() => null; + public PropertyDescriptor? GetDefaultProperty() => null; + public object? GetEditor(Type editorBaseType) => null; + public EventDescriptorCollection GetEvents() => EventDescriptorCollection.Empty; - public EventDescriptorCollection GetEvents(Attribute[]? attributes) => EventDescriptorCollection.Empty; - public PropertyDescriptorCollection GetProperties() - { - GetPropertiesCalled.Value = true; - return PropertyDescriptorCollection.Empty; - } + public EventDescriptorCollection GetEvents(Attribute[]? attributes) => GetEvents(); - public PropertyDescriptorCollection GetProperties(Attribute[]? attributes) - { - throw new NotImplementedException(); - } + public PropertyDescriptorCollection GetProperties() => PropertyDescriptorCollection.Empty; + + public PropertyDescriptorCollection GetProperties(Attribute[]? attributes) => GetProperties(); public object? GetPropertyOwner(PropertyDescriptor? pd) => null; } public override ICustomTypeDescriptor GetTypeDescriptor(Type objectType, object? instance) { - Constructed.Value = true; - return new CTD(); + return new EmptyPropertyListDescriptor(); } } - [TypeDescriptionProvider(typeof(SomeTypeProvider))] - private sealed class SomeType + [TypeDescriptionProvider(typeof(EmptyPropertiesTypeProvider))] + public sealed class TypeWithProperty { - public int SomeProperty { get; set; } + public int OneProperty { get; set; } } } } From b50fdd78313ff54cecfe8eb4ff30650fc13e27f3 Mon Sep 17 00:00:00 2001 From: Gan Keyu Date: Sun, 24 Sep 2023 11:40:50 +0800 Subject: [PATCH 8/8] minimize lock use in CheckDefaultProvider --- .../System/ComponentModel/TypeDescriptor.cs | 110 ++++++++++++++---- 1 file changed, 85 insertions(+), 25 deletions(-) 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 b3a6851487d486..680bd87f12d717 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs @@ -189,6 +189,30 @@ public static void AddProvider(TypeDescriptionProvider provider, Type type) Refresh(type); } + /// + /// Specialized version for calls from . + /// Code is adjusted to minimize lock in the aforementioned method. + /// + /// + /// + private static void AddProvider_FewerLock(TypeDescriptionProvider provider, Type type) + { + Debug.Assert(provider != null); + Debug.Assert(type != null); + + lock (s_providerTable) + { + // Get the root node, hook it up, and stuff it back into + // the provider cache. + TypeDescriptionNode node = NodeFor_LockFreeWithCreateDelegator(type); + var head = new TypeDescriptionNode(provider) { Next = node }; + s_providerTable[type] = head; + s_providerTypeTable.Clear(); + } + + Refresh(type); + } + /// /// Adds a type description provider that will be called on to provide /// type information for a single object instance. A provider added @@ -264,36 +288,33 @@ private static void CheckDefaultProvider(Type type) { bool providerAdded = false; - lock (s_internalSyncObject) + if (s_defaultProviders.ContainsKey(type)) + { + return; + } + + // 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 precedence. + object[] attrs = type.GetCustomAttributes(typeof(TypeDescriptionProviderAttribute), false); + for (int idx = attrs.Length - 1; idx >= 0; idx--) { - if (s_defaultProviders.ContainsKey(type)) + TypeDescriptionProviderAttribute pa = (TypeDescriptionProviderAttribute)attrs[idx]; + Type? providerType = Type.GetType(pa.TypeName); + if (providerType != null && typeof(TypeDescriptionProvider).IsAssignableFrom(providerType)) { - return; + TypeDescriptionProvider prov = (TypeDescriptionProvider)Activator.CreateInstance(providerType)!; + AddProvider_FewerLock(prov, type); + providerAdded = true; } + } - // Immediately clear this. If we find a default provider - // and it starts messing around with type information, - // this could infinitely recurse. + lock (s_internalSyncObject) + { s_defaultProviders[type] = null; - - // 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 precedence. - object[] attrs = type.GetCustomAttributes(typeof(TypeDescriptionProviderAttribute), 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 we did not add a provider, check the base class. @@ -1496,6 +1517,45 @@ private static TypeDescriptionNode NodeFor(Type type, bool createDelegator) return node; } + /// + /// Specialized version for calls from and . + /// Code is adjusted to minimize lock. + /// + /// + /// + private static TypeDescriptionNode NodeFor_LockFreeWithCreateDelegator(Type type) + { + // lock is in the outer method. + + var node = (TypeDescriptionNode?)(s_providerTypeTable[type] ?? s_providerTable[type]); + if (node != null) + { + return node; + } + + Type? baseType = GetNodeForBaseType(type); + + if (type == typeof(object) || baseType == null) + { + node = (TypeDescriptionNode?)s_providerTable[type]; + + 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[type] = node; + } + } + else + { + node = new TypeDescriptionNode(new DelegatingTypeDescriptionProvider(baseType)); + s_providerTypeTable[type] = node; + } + + return node; + } + /// /// Retrieves the head type description node for an instance. /// Instance-based node lists are rare. If a node list is not