In the System.ComponentModel.PropertyDescriptor class, the AddValueChanged and RemoveValueChanged methods mutate private Dictionary<object, EventHandler?>? _valueChangedHandlers without locking. This causes errors if multiple threads add or remove value-changed event handlers for different components in parallel. Because TypeDescriptor caches the PropertyDescriptor instances, it is normal that multiple threads use the same PropertyDescriptor instance.
Reproduction Steps
This demo program attempts to trigger the bug in one of three ways, depending on the command line:
PropertyDescriptor: Hits the bug, but might not be a realistic scenario.
BindingSource: Hits the bug, but BindingSource is defined in Windows Forms and would typically be used from a UI thread only. In the Debug configuration, the dictionary corruption causes an assertion failure before the InvalidOperationException.
BindingList\<T>: Doesn't hit the bug because it checks for INotifyPropertyChange, rather than adding event handlers via PropertyDescriptor.
using System;
using System.ComponentModel;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics;
using System.IO;
using System.Threading.Tasks;
#if UseWindowsForms
using System.Windows.Forms;
#endif
namespace ConsoleApp1
{
internal static class Program
{
private static int Main(string[] args)
{
if (args.Length != 2
|| !long.TryParse(args[1], out long count))
{
ShowUsage(Console.Error);
return 1;
}
Console.WriteLine("Initial check.");
PropertyDescriptor property = TypeDescriptor.GetDefaultProperty(componentType: typeof(Notifier));
Debug.Assert(property.SupportsChangeEvents, "Must support change events.");
switch (args[0])
{
case "BindingList":
// No problem.
Console.WriteLine("Parallel looping with BindingList...");
Parallel.For(0, count, _ => PokeViaBindingList());
Console.WriteLine("Finished.");
break;
case "BindingSource":
#if UseWindowsForms
Console.WriteLine("Parallel looping with BindingSource...");
Parallel.For(0, count, _ => PokeViaBindingSource());
Console.WriteLine("Finished.");
break;
#else
Console.Error.WriteLine("BindingSource requires Windows Forms.");
return 1;
#endif
case "PropertyDescriptor":
// Not thread-safe.
Console.WriteLine("Parallel looping with PropertyDescriptor...");
Parallel.For(0, count, _ => PokeViaPropertyDescriptor());
Console.WriteLine("Finished.");
break;
default:
ShowUsage(Console.Error);
return 1;
}
return 0;
}
private static void ShowUsage(TextWriter textWriter)
{
string appName = System.Reflection.Assembly.GetEntryAssembly().GetName().Name;
textWriter.WriteLine($"Usage: {appName} (BindingList | BindingSource | PropertyDescriptor) count");
}
// No problems here.
private static void PokeViaBindingList()
{
BindingList<Notifier> bindingList = new BindingList<Notifier>();
Debug.Assert(((IRaiseItemChangedEvents)bindingList).RaisesItemChangedEvents, "Must raise ItemChanged events.");
int itemChangeCount = 0;
ListChangedEventHandler listChangedHandler = (sender, e) =>
{
if (e.ListChangedType == ListChangedType.ItemChanged)
{
itemChangeCount++;
}
};
bindingList.ListChanged += listChangedHandler;
Notifier notifier = new Notifier();
bindingList.Add(notifier);
Debug.Assert(itemChangeCount == 0, "Expected itemChangeCount == 0");
notifier.Number = 42;
Debug.Assert(itemChangeCount == 1, "Expected itemChangeCount == 1");
bindingList.ListChanged -= listChangedHandler;
bindingList.Clear();
}
#if UseWindowsForms
private static void PokeViaBindingSource()
{
using (BindingSource bindingSource = new BindingSource())
{
// Because List<T> implements neither IRaiseItemChangedEvents nor IBindingList,
// BindingSource will hook the property-change events of the current item only.
bindingSource.DataSource = new List<Notifier>();
bindingSource.CurrencyManager.PositionChanged += (sender, e) => { };
int itemChangeCount = 0;
ListChangedEventHandler listChangedHandler = (sender, e) =>
{
if (e.ListChangedType == ListChangedType.ItemChanged)
{
itemChangeCount++;
}
};
bindingSource.ListChanged += listChangedHandler;
Notifier notifier = new Notifier();
bindingSource.Add(notifier);
Debug.Assert(bindingSource.Current == notifier, "The item should have become current");
Debug.Assert(itemChangeCount == 0, "Expected itemChangeCount == 0", $"Is actually {itemChangeCount}");
notifier.Number = 42;
Debug.Assert(itemChangeCount == 1, "Expected itemChangeCount == 1", $"Is actually {itemChangeCount}");
bindingSource.ListChanged -= listChangedHandler;
}
}
#endif // UseWindowsForms
private static void PokeViaPropertyDescriptor()
{
Notifier component = new Notifier();
PropertyDescriptor property = TypeDescriptor.GetDefaultProperty(component: component);
Debug.Assert(property.SupportsChangeEvents, "Must support change events.");
EventHandler valueChangedHandler = (sender, e) => { };
property.AddValueChanged(component, valueChangedHandler);
property.RemoveValueChanged(component, valueChangedHandler);
}
}
[DefaultProperty(nameof(Number))]
internal sealed class Notifier : INotifyPropertyChanged
{
private int number;
public int Number
{
get => this.number;
set
{
this.number = value;
this.PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(this.Number)));
}
}
public event PropertyChangedEventHandler PropertyChanged;
}
}
Run
dotnet run --configuration=Release --framework=net9.0-windows -- PropertyDescriptor 1000000
Expected behavior
Should not throw any exceptions. Indeed it doesn't throw, if the parallel loop count is only 1.
Initial check.
Parallel looping with PropertyDescriptor...
Finished.
Actual behavior
Throws InvalidOperationException because invalid parallel access corrupts the dictionary.
Initial check.
Parallel looping with PropertyDescriptor...
Unhandled exception. System.AggregateException: One or more errors occurred. (Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.) (Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.)
---> System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
at System.Collections.Generic.Dictionary`2.FindValue(TKey key)
at System.Collections.Generic.Dictionary`2.TryGetValue(TKey key, TValue& value)
at System.Collections.Generic.CollectionExtensions.GetValueOrDefault[TKey,TValue](IReadOnlyDictionary`2 dictionary, TKey key, TValue defaultValue)
at System.ComponentModel.ReflectPropertyDescriptor.AddValueChanged(Object component, EventHandler handler)
at ConsoleApp1.Program.PokeViaPropertyDescriptor() in [REDACTED]\ConsoleApp1\Program.cs:line 136
at ConsoleApp1.Program.<>c.<Main>b__0_2(Int64 _) in [REDACTED]\ConsoleApp1\Program.cs:line 53
at System.Threading.Tasks.Parallel.<>c__DisplayClass19_0`2.<ForWorker>b__1(RangeWorker& currentWorker, Int64 timeout, Boolean& replicationDelegateYieldedBeforeCompletion)
--- End of stack trace from previous location ---
at System.Threading.Tasks.Parallel.<>c__DisplayClass19_0`2.<ForWorker>b__1(RangeWorker& currentWorker, Int64 timeout, Boolean& replicationDelegateYieldedBeforeCompletion)
at System.Threading.Tasks.TaskReplicator.Replica.Execute()
--- End of inner exception stack trace ---
at System.Threading.Tasks.TaskReplicator.Run[TState](ReplicatableUserAction`1 action, ParallelOptions options, Boolean stopOnFirstFailure)
at System.Threading.Tasks.Parallel.ForWorker[TLocal,TInt](TInt fromInclusive, TInt toExclusive, ParallelOptions parallelOptions, Action`1 body, Action`2 bodyWithState, Func`4 bodyWithLocal, Func`1 localInit, Action`1 localFinally)
--- End of stack trace from previous location ---
at System.Threading.Tasks.Parallel.ForWorker[TLocal,TInt](TInt fromInclusive, TInt toExclusive, ParallelOptions parallelOptions, Action`1 body, Action`2 bodyWithState, Func`4 bodyWithLocal, Func`1 localInit, Action`1 localFinally)
at System.Threading.Tasks.Parallel.For(Int64 fromInclusive, Int64 toExclusive, Action`1 body)
at ConsoleApp1.Program.Main(String[] args) in [REDACTED]\ConsoleApp1\Program.cs:line 53
---> (Inner Exception #1) System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
at System.Collections.Generic.Dictionary`2.FindValue(TKey key)
at System.Collections.Generic.Dictionary`2.TryGetValue(TKey key, TValue& value)
at System.Collections.Generic.CollectionExtensions.GetValueOrDefault[TKey,TValue](IReadOnlyDictionary`2 dictionary, TKey key, TValue defaultValue)
at System.ComponentModel.ReflectPropertyDescriptor.AddValueChanged(Object component, EventHandler handler)
at ConsoleApp1.Program.PokeViaPropertyDescriptor() in [REDACTED]\ConsoleApp1\Program.cs:line 136
at ConsoleApp1.Program.<>c.<Main>b__0_2(Int64 _) in [REDACTED]\ConsoleApp1\Program.cs:line 53
at System.Threading.Tasks.Parallel.<>c__DisplayClass19_0`2.<ForWorker>b__1(RangeWorker& currentWorker, Int64 timeout, Boolean& replicationDelegateYieldedBeforeCompletion)
--- End of stack trace from previous location ---
at System.Threading.Tasks.Parallel.<>c__DisplayClass19_0`2.<ForWorker>b__1(RangeWorker& currentWorker, Int64 timeout, Boolean& replicationDelegateYieldedBeforeCompletion)
at System.Threading.Tasks.TaskReplicator.Replica.Execute()<---
Regression?
No, it doesn't work in .NET Framework either.
Known Workarounds
No response
Configuration
.NET 9.0.0-preview.3.24172.9 on Windows 10.
The thread-unsafety of PropertyDescriptor.AddValueChange is not specific to Windows. In the .NET Runtime however, only WPF and Windows Forms seem to call this method; that may make the bug less likely to affect applications on other operating systems.
Description
In the System.ComponentModel.PropertyDescriptor class, the AddValueChanged and RemoveValueChanged methods mutate
private Dictionary<object, EventHandler?>? _valueChangedHandlers
without locking. This causes errors if multiple threads add or remove value-changed event handlers for different components in parallel. Because TypeDescriptor caches the PropertyDescriptor instances, it is normal that multiple threads use the same PropertyDescriptor instance.Reproduction Steps
This demo program attempts to trigger the bug in one of three ways, depending on the command line:
ConsoleApp1.csproj
Program.cs
Run
dotnet run --configuration=Release --framework=net9.0-windows -- PropertyDescriptor 1000000
Expected behavior
Should not throw any exceptions. Indeed it doesn't throw, if the parallel loop count is only 1.
Actual behavior
Throws InvalidOperationException because invalid parallel access corrupts the dictionary.
Regression?
No, it doesn't work in .NET Framework either.
Known Workarounds
No response
Configuration
.NET 9.0.0-preview.3.24172.9 on Windows 10.
The thread-unsafety of PropertyDescriptor.AddValueChange is not specific to Windows. In the .NET Runtime however, only WPF and Windows Forms seem to call this method; that may make the bug less likely to affect applications on other operating systems.
Other information
Related to https://github.com/dotnet/runtime/issues/30024 and https://github.com/dotnet/runtime/issues/92394. The bug corrupts this dictionary:
https://github.com/dotnet/runtime/blob/9e6ba1f68c6a9c7206dacdf1e4cac67ea19931eb/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/PropertyDescriptor.cs#L19