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 0747e7ff01e95e..4967187d006446 100644
--- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs
+++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs
@@ -27,7 +27,12 @@ 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 Hashtable s_defaultProviderInitialized = new Hashtable(); // A table of type -> object to track DefaultTypeDescriptionProviderAttributes.
+ // A value of `null` indicates initialization is in progress.
+ // A value of s_initializedDefaultProvider indicates the provider is initialized.
+ private static readonly object s_initializedDefaultProvider = new object();
+
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.
@@ -74,8 +79,6 @@ public sealed class TypeDescriptor
Guid.NewGuid() // events
};
- private static readonly object s_internalSyncObject = new object();
-
private TypeDescriptor()
{
}
@@ -302,32 +305,43 @@ public static void AddProviderTransparent(TypeDescriptionProvider provider, obje
///
private static void CheckDefaultProvider(Type type)
{
- if (s_defaultProviders.ContainsKey(type))
+ if (s_defaultProviderInitialized[type] == s_initializedDefaultProvider)
{
return;
}
- lock (s_internalSyncObject)
+ // Lock on s_providerTable even though s_providerTable is not modified here.
+ // Using a single lock prevents deadlocks since other methods that call into or are called
+ // by this method also lock on s_providerTable and the ordering of the locks may be different.
+ lock (s_providerTable)
{
- if (s_defaultProviders.ContainsKey(type))
- {
- return;
- }
+ AddDefaultProvider(type);
+ }
+ }
+
+ ///
+ /// Add the default provider, if it exists.
+ /// For threading, this is always called under a 'lock (s_providerTable)'.
+ ///
+ private static void AddDefaultProvider(Type type)
+ {
+ bool providerAdded = false;
- // 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;
+ if (s_defaultProviderInitialized.ContainsKey(type))
+ {
+ // Either another thread finished initializing for this type, or we are recursing on the same thread.
+ 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 precidence.
+ // Immediately set this to null to indicate we are in progress setting the default provider for a type.
+ // This prevents re-entrance to this method.
+ s_defaultProviderInitialized[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);
- bool providerAdded = false;
for (int idx = attrs.Length - 1; idx >= 0; idx--)
{
TypeDescriptionProviderAttribute pa = (TypeDescriptionProviderAttribute)attrs[idx];
@@ -339,24 +353,25 @@ private static void CheckDefaultProvider(Type 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);
+ AddDefaultProvider(baseType);
}
}
+
+ s_defaultProviderInitialized[type] = s_initializedDefaultProvider;
}
///
- /// The CreateAssocation method creates an association between two objects.
+ /// The CreateAssociation method creates an association between two objects.
/// Once an association is created, a designer or other filtering mechanism
/// can add properties that route to either object into the primary object's
/// property set. When a property invocation is made against the primary
- /// object, GetAssocation will be called to resolve the actual object
+ /// object, GetAssociation will be called to resolve the actual object
/// instance that is related to its type parameter.
///
[EditorBrowsable(EditorBrowsableState.Advanced)]
diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs
index a142fe5e5c7d0f..08dd7107ff82f1 100644
--- a/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs
+++ b/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs
@@ -5,6 +5,10 @@
using System.Collections.Generic;
using System.ComponentModel.Design;
using System.Globalization;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.DotNet.RemoteExecutor;
using Moq;
using Xunit;
@@ -61,7 +65,7 @@ public void AddProvider_InvokeObjectMultipleTimes_Refreshes()
.Setup(p => p.GetCache(instance))
.Returns(new Dictionary())
.Verifiable();
-
+
int callCount = 0;
RefreshEventHandler handler = (e) =>
{
@@ -147,7 +151,7 @@ public void AddProvider_InvokeTypeMultipleTimes_Refreshes()
.Setup(p => p.GetCache(type))
.Returns(new Dictionary())
.Verifiable();
-
+
int callCount = 0;
RefreshEventHandler handler = (e) =>
{
@@ -253,7 +257,7 @@ public void AddProviderTransparent_InvokeObjectMultipleTimes_Refreshes()
.Setup(p => p.GetCache(instance))
.Returns(new Dictionary())
.Verifiable();
-
+
int callCount = 0;
RefreshEventHandler handler = (e) =>
{
@@ -338,7 +342,7 @@ public void AddProviderTransparent_InvokeTypeMultipleTimes_Refreshes()
.Setup(p => p.GetCache(type))
.Returns(new Dictionary())
.Verifiable();
-
+
int callCount = 0;
RefreshEventHandler handler = (e) =>
{
@@ -1226,5 +1230,248 @@ protected DerivedCultureInfo() : base("hello")
class TwiceDerivedCultureInfo : DerivedCultureInfo
{
}
+
+ private long _concurrentError = 0;
+ private bool ConcurrentError
+ {
+ get => Interlocked.Read(ref _concurrentError) == 1;
+ set => Interlocked.Exchange(ref _concurrentError, value ? 1 : 0);
+ }
+
+ private void ConcurrentTest(TypeWithProperty instance)
+ {
+ var properties = TypeDescriptor.GetProperties(instance);
+ Thread.Sleep(10);
+ if (properties.Count > 0)
+ {
+ ConcurrentError = true;
+ }
+ }
+
+ [SkipOnPlatform(TestPlatforms.Browser, "Thread.Start is not supported on browsers.")]
+ [Fact]
+ public void ConcurrentGetProperties_ReturnsExpected()
+ {
+ const int Timeout = 60000;
+ int concurrentCount = Environment.ProcessorCount * 2;
+
+ using var finished = new CountdownEvent(concurrentCount);
+
+ var instances = new TypeWithProperty[concurrentCount];
+ for (int i = 0; i < concurrentCount; i++)
+ {
+ instances[i] = new TypeWithProperty();
+ }
+
+ 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.True(false, "Timeout. Possible deadlock.");
+ }
+ else
+ {
+ Assert.False(ConcurrentError, "Fallback type descriptor is used. Possible race condition.");
+ }
+ }
+
+ [SkipOnPlatform(TestPlatforms.Browser, "Thread.Start is not supported on browsers.")]
+ [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+ public static void ConcurrentAddProviderAndGetProvider()
+ {
+ // Use a timeout value lower than RemoteExecutor in order to get a nice Fail message.
+ const int Timeout = 50000;
+
+ RemoteInvokeOptions options = new()
+ {
+ TimeOut = 60000
+ };
+
+ RemoteExecutor.Invoke(() =>
+ {
+ using var finished = new CountdownEvent(2);
+ Thread t1 = new Thread(() =>
+ {
+ ConcurrentAddProvider();
+ finished.Signal();
+ });
+ Thread t2 = new Thread(() =>
+ {
+ ConcurrentGetProvider();
+ finished.Signal();
+ });
+ t1.Start();
+ t2.Start();
+ finished.Wait(Timeout);
+ if (finished.CurrentCount != 0)
+ {
+ Assert.True(false, "Timeout. Possible deadlock.");
+ }
+ }, options).Dispose();
+
+ static void ConcurrentAddProvider()
+ {
+ var provider = new EmptyPropertiesTypeProvider();
+ TypeDescriptor.AddProvider(provider, typeof(MyClass));
+
+ // This test primarily verifies no deadlock, but verify the values anyway.
+ Assert.True(TypeDescriptor.GetProvider(typeof(MyClass)).IsSupportedType(typeof(MyClass)));
+ }
+
+ static void ConcurrentGetProvider()
+ {
+ TypeDescriptionProvider provider = TypeDescriptor.GetProvider(typeof(TypeWithProperty));
+
+ // This test primarily verifies no deadlock, but verify the values anyway.
+ Assert.True(provider.IsSupportedType(typeof(TypeWithProperty)));
+ }
+ }
+
+ public sealed class EmptyPropertiesTypeProvider : TypeDescriptionProvider
+ {
+ private sealed class EmptyPropertyListDescriptor : 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) => GetEvents();
+
+ 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)
+ {
+ return new EmptyPropertyListDescriptor();
+ }
+ }
+
+ [TypeDescriptionProvider(typeof(EmptyPropertiesTypeProvider))]
+ public sealed class TypeWithProperty
+ {
+ public int OneProperty { get; set; }
+ }
+
+ public static IEnumerable