TypeDescriptor.GetProperties(object instance) is not thread-safe

[seanhalliday]

Description

If multiple threads call TypeDescriptor.GetProperties(object instance) at the same time, they can get different results if the instance has a TypeDescriptorProvider attribute. I believe this is because of missing locks around reading the WeakHashTables.

Reproduction Steps

using System.ComponentModel;
using System.Threading;

namespace TypeDescriptionProvider_GetProperties_repro
{
	public class SomeTypeProvider : TypeDescriptionProvider
	{
		public static ThreadLocal<bool> Constructed = new ThreadLocal<bool>();
		public static ThreadLocal<bool> GetPropertiesCalled = new ThreadLocal<bool>();
		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))]
	public sealed class SomeType
	{
		public int SomeProperty { get; set; }
	}

	public static class Program
	{
		public static void ConcurrentTest(SomeType instance)
		{
			var properties = TypeDescriptor.GetProperties(instance);
			Console.WriteLine($"Constructed = {SomeTypeProvider.Constructed.Value} GetPropertiesCalled={SomeTypeProvider.GetPropertiesCalled.Value} properties.Count = {properties.Count} (should be 0)");
		}

		public static void Main()
		{
			const int threadCount = 10;
			// Uncomment and it will get the correct properties for all threads
			// TypeDescriptor.GetProperties(new SomeType());
			using (var finished = new CountdownEvent(threadCount))
			{
				SomeType[] instances = new SomeType[threadCount];
				for (var i = 0; i < threadCount; i++)
				{
					instances[i] = new SomeType();
				}
				for (var i = 0; i < threadCount; i++)
				{
					int _i = i;
					new Thread(() =>
					{
						ConcurrentTest(instances[_i]);
						finished.Signal();
					}).Start();
				}

				finished.Wait();
			}
		}
	}
}

### Expected behavior

Every thread should call the provider and get an empty list of properties.

### Actual behavior

Some threads will not call the provider and get the default properties based on reflection

### Regression?

Not that I know.

### Known Workarounds

_No response_

### Configuration

_No response_

### Other information

_No response_

[ghost]

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

If multiple threads call TypeDescriptor.GetProperties(object instance) at the same time, they can get different results if the instance has a TypeDescriptorProvider attribute. I believe this is because of missing locks around reading the WeakHashTables.

Reproduction Steps

using System.ComponentModel;
using System.Threading;

namespace TypeDescriptionProvider_GetProperties_repro
{
	public class SomeTypeProvider : TypeDescriptionProvider
	{
		public static ThreadLocal<bool> Constructed = new ThreadLocal<bool>();
		public static ThreadLocal<bool> GetPropertiesCalled = new ThreadLocal<bool>();
		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))]
	public sealed class SomeType
	{
		public int SomeProperty { get; set; }
	}

	public static class Program
	{
		public static void ConcurrentTest(SomeType instance)
		{
			var properties = TypeDescriptor.GetProperties(instance);
			Console.WriteLine($"Constructed = {SomeTypeProvider.Constructed.Value} GetPropertiesCalled={SomeTypeProvider.GetPropertiesCalled.Value} properties.Count = {properties.Count} (should be 0)");
		}

		public static void Main()
		{
			const int threadCount = 10;
			// Uncomment and it will get the correct properties for all threads
			// TypeDescriptor.GetProperties(new SomeType());
			using (var finished = new CountdownEvent(threadCount))
			{
				SomeType[] instances = new SomeType[threadCount];
				for (var i = 0; i < threadCount; i++)
				{
					instances[i] = new SomeType();
				}
				for (var i = 0; i < threadCount; i++)
				{
					int _i = i;
					new Thread(() =>
					{
						ConcurrentTest(instances[_i]);
						finished.Signal();
					}).Start();
				}

				finished.Wait();
			}
		}
	}
}

### Expected behavior

Every thread should call the provider and get an empty list of properties.

### Actual behavior

Some threads will not call the provider and get the default properties based on reflection

### Regression?

Not that I know.

### Known Workarounds

_No response_

### Configuration

_No response_

### Other information

_No response_

<table>
  <tr>
    <th align="left">Author:</th>
    <td>seanhalliday</td>
  </tr>
  <tr>
    <th align="left">Assignees:</th>
    <td>-</td>
  </tr>
  <tr>
    <th align="left">Labels:</th>
    <td>

`area-System.ComponentModel`, `untriaged`

</td>
  </tr>
  <tr>
    <th align="left">Milestone:</th>
    <td>-</td>
  </tr>
</table>
</details>

[steveharter]

Verified repro; the threads after the first do not call TypeDescriptor.GetProperties and instead appear to use fall-back reflection logic. Once the original thread is done initializing the TypeDescriptor, then additional threads will correctly call TypeDescriptor.GetProperties.

[karakasa]

The minimal "simple" lock I found is in the method of CheckDefaultProvider.

https://github.com/karakasa/runtime/blob/7b743c3799679b0e2d68a4593a62f16e0360ca1f/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs#L263-L297

I believe the reason is:

• First thread finds no mapping in s_defaultProviders, hence sets s_defaultProviders[Type] = null. However it hasn't completed the method of CheckDefaultProvider.
• Other threads think providers exist because s_defaultProviders[Type] exists but fails to retrieve one because the first thread hasn't finished the method, therefore later-coming threads revert to the fallback approach.

I have tested the modified code with more concurrent threads and find no deadlocks.

[seanhalliday]

We are working around this by calling TypeDescriptor.GetProvider(type) for every type that has a TypeProviderAttribute. We do this at application startup before we do any parallel queries for it.

[steveharter]

Note there is a newer PR for this now: #92521

[steveharter]

This long-standing threading issues in TypeDescriptor have been addressed in .NET 9 Preview 1, so any existing work-arounds can be removed. Workarounds included pre-populating internal caches such as by calling TypeDescriptor.GetProvider(type) for every type that has a TypeProviderAttribute, and by wrapping the access to the affected APIs with a lock statement.

Also see Concurrency issue in TypeDescriptor.GetConverter(type).

The threading issues also exist in .NET Framework. If you are affected by this issue in .NET Framework, please add feedback here so we can determine the priority of porting the fix to .NET Framework.