diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj b/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj index 588c6a970ae5e..6979742610c46 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj @@ -250,6 +250,7 @@ + diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/PropertyDescriptor.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/PropertyDescriptor.cs index 23d05036265ec..30d6a262e31db 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/PropertyDescriptor.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/PropertyDescriptor.cs @@ -2,9 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Reflection; +using System.Threading; namespace System.ComponentModel { @@ -16,10 +18,11 @@ public abstract class PropertyDescriptor : MemberDescriptor internal const string PropertyDescriptorPropertyTypeMessage = "PropertyDescriptor's PropertyType cannot be statically discovered."; private TypeConverter? _converter; - private Dictionary? _valueChangedHandlers; + private ConcurrentDictionary? _valueChangedHandlers; private object?[]? _editors; private Type[]? _editorTypes; private int _editorCount; + private object? _syncObject; /// /// Initializes a new instance of the class with the specified name and @@ -49,6 +52,8 @@ protected PropertyDescriptor(MemberDescriptor descr, Attribute[]? attrs) : base( { } + private object SyncObject => LazyInitializer.EnsureInitialized(ref _syncObject); + /// /// When overridden in a derived class, gets the type of the /// component this property is bound to. @@ -165,10 +170,11 @@ public virtual void AddValueChanged(object component, EventHandler handler) ArgumentNullException.ThrowIfNull(component); ArgumentNullException.ThrowIfNull(handler); - _valueChangedHandlers ??= new Dictionary(); - - EventHandler? h = _valueChangedHandlers.GetValueOrDefault(component, defaultValue: null); - _valueChangedHandlers[component] = (EventHandler?)Delegate.Combine(h, handler); + lock (SyncObject) + { + _valueChangedHandlers ??= new ConcurrentDictionary(concurrencyLevel: 1, capacity: 0); + _valueChangedHandlers.AddOrUpdate(component, handler, (k, v) => (EventHandler?)Delegate.Combine(v, handler)); + } } /// @@ -433,15 +439,18 @@ public virtual void RemoveValueChanged(object component, EventHandler handler) if (_valueChangedHandlers != null) { - EventHandler? h = _valueChangedHandlers.GetValueOrDefault(component, defaultValue: null); - h = (EventHandler?)Delegate.Remove(h, handler); - if (h != null) - { - _valueChangedHandlers[component] = h; - } - else + lock (SyncObject) { - _valueChangedHandlers.Remove(component); + EventHandler? h = _valueChangedHandlers.GetValueOrDefault(component, defaultValue: null); + h = (EventHandler?)Delegate.Remove(h, handler); + if (h != null) + { + _valueChangedHandlers[component] = h; + } + else + { + _valueChangedHandlers.Remove(component, out EventHandler? _); + } } } } diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs index 531989154e4c6..d7c74e719d36c 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs @@ -3,6 +3,7 @@ using System.Collections; using System.Collections.Generic; +using System.Collections.Concurrent; using System.ComponentModel.Design; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; @@ -23,7 +24,7 @@ namespace System.ComponentModel internal sealed partial class ReflectTypeDescriptionProvider : TypeDescriptionProvider { // ReflectedTypeData contains all of the type information we have gathered for a given type. - private Dictionary? _typeData; + private readonly ConcurrentDictionary _typeData = new ConcurrentDictionary(); // This is the signature we look for when creating types that are generic, but // want to know what type they are dealing with. Enums are a good example of this; @@ -281,8 +282,7 @@ internal static void AddEditorTable(Type editorBaseType, Hashtable table) public override bool? RequireRegisteredTypes => true; public override bool IsRegisteredType(Type type) { - if (_typeData != null && - _typeData.TryGetValue(type, out ReflectedTypeData? data) && + if (_typeData.TryGetValue(type, out ReflectedTypeData? data) && data.IsRegistered) { return true; @@ -862,18 +862,11 @@ internal Type[] GetPopulatedTypes(Module module) { List typeList = new List(); - lock (TypeDescriptor.s_commonSyncObject) + foreach (KeyValuePair kvp in _typeData) { - Dictionary? typeData = _typeData; - if (typeData != null) + if (kvp.Key.Module == module && kvp.Value!.IsPopulated) { - foreach (KeyValuePair kvp in typeData) - { - if (kvp.Key.Module == module && kvp.Value!.IsPopulated) - { - typeList.Add(kvp.Key); - } - } + typeList.Add(kvp.Key); } } @@ -927,9 +920,7 @@ public override Type GetReflectionType( /// private ReflectedTypeData? GetTypeData([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type type, bool createIfNeeded) { - ReflectedTypeData? td = null; - - if (_typeData != null && _typeData.TryGetValue(type, out td)) + if (_typeData.TryGetValue(type, out ReflectedTypeData? td)) { Debug.Assert(td != null); return td; @@ -937,7 +928,7 @@ public override Type GetReflectionType( lock (TypeDescriptor.s_commonSyncObject) { - if (_typeData != null && _typeData.TryGetValue(type, out td)) + if (_typeData.TryGetValue(type, out td)) { Debug.Assert(td != null); @@ -958,7 +949,6 @@ public override Type GetReflectionType( if (createIfNeeded) { td = new ReflectedTypeData(type, isRegisteredType: false); - _typeData ??= new Dictionary(); _typeData[type] = td; } } @@ -968,7 +958,7 @@ public override Type GetReflectionType( private ReflectedTypeData GetTypeDataFromRegisteredType(Type type) { - if (_typeData == null || !_typeData.TryGetValue(type, out ReflectedTypeData? td)) + if (!_typeData.TryGetValue(type, out ReflectedTypeData? td)) { if (IsIntrinsicType(type)) { @@ -991,41 +981,34 @@ private ReflectedTypeData GetTypeDataFromRegisteredType(Type type) public override void RegisterType<[DynamicallyAccessedMembers(TypeDescriptor.RegisteredTypesDynamicallyAccessedMembers)] T>() { Type componentType = typeof(T); - ReflectedTypeData? td = null; - if (_typeData != null && _typeData.ContainsKey(componentType)) + if (_typeData.ContainsKey(componentType)) { return; } lock (TypeDescriptor.s_commonSyncObject) { - if (_typeData != null && _typeData.ContainsKey(componentType)) + if (_typeData.ContainsKey(componentType)) { return; } - if (td == null) - { - td = new ReflectedTypeData(componentType, isRegisteredType: true); - _typeData ??= new Dictionary(); - _typeData[componentType] = td; - } + ReflectedTypeData td = new ReflectedTypeData(componentType, isRegisteredType: true); + _typeData[componentType] = td; } } private ReflectedTypeData GetOrRegisterType(Type type) { - ReflectedTypeData? td = null; - - if (_typeData != null && _typeData.TryGetValue(type, out td)) + if (_typeData.TryGetValue(type, out ReflectedTypeData? td)) { return td; } lock (TypeDescriptor.s_commonSyncObject) { - if (_typeData != null && _typeData.TryGetValue(type, out td)) + if (_typeData.TryGetValue(type, out td)) { return td; } @@ -1033,7 +1016,6 @@ private ReflectedTypeData GetOrRegisterType(Type type) if (td == null) { td = new ReflectedTypeData(type, isRegisteredType: true); - _typeData ??= new Dictionary(); _typeData[type] = td; } } 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 78f34efbfb73a..6b782a9fdda03 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.Specialized; using System.ComponentModel.Design; @@ -56,12 +57,12 @@ public sealed class TypeDescriptor internal static readonly object s_commonSyncObject = new object(); // A direct mapping from type to provider. - private static readonly Dictionary s_providerTypeTable = new Dictionary(); + private static readonly ConcurrentDictionary s_providerTypeTable = new ConcurrentDictionary(); // Tracks DefaultTypeDescriptionProviderAttributes. // A value of `null` indicates initialization is in progress. // A value of s_initializedDefaultProvider indicates the provider is initialized. - private static readonly Dictionary s_defaultProviderInitialized = new Dictionary(); + private static readonly ConcurrentDictionary s_defaultProviderInitialized = new ConcurrentDictionary(); private static readonly object s_initializedDefaultProvider = new object(); diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/PropertyDescriptorTests.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/PropertyDescriptorTests.cs index f791d46824f82..4786d3966a7e1 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/PropertyDescriptorTests.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/PropertyDescriptorTests.cs @@ -28,13 +28,21 @@ public void RaiseAddedValueChangedHandler() var component = new DescriptorTestComponent(); var properties = TypeDescriptor.GetProperties(component.GetType()); PropertyDescriptor propertyDescriptor = properties.Find(nameof(component.Property), false); - var handlerWasCalled = false; - EventHandler valueChangedHandler = (_, __) => handlerWasCalled = true; + int handlerCalledCount = 0; - propertyDescriptor.AddValueChanged(component, valueChangedHandler); - propertyDescriptor.SetValue(component, int.MaxValue); + EventHandler valueChangedHandler1 = (_, __) => handlerCalledCount++; + EventHandler valueChangedHandler2 = (_, __) => handlerCalledCount++; + + propertyDescriptor.AddValueChanged(component, valueChangedHandler1); + + // Add case. + propertyDescriptor.SetValue(component, int.MaxValue); // Add to delegate. + Assert.Equal(1, handlerCalledCount); - Assert.True(handlerWasCalled); + + propertyDescriptor.AddValueChanged(component, valueChangedHandler2); + propertyDescriptor.SetValue(component, int.MaxValue); // Update delegate. + Assert.Equal(3, handlerCalledCount); } [Fact] @@ -42,15 +50,25 @@ public void RemoveAddedValueChangedHandler() { var component = new DescriptorTestComponent(); var properties = TypeDescriptor.GetProperties(component.GetType()); - var handlerWasCalled = false; - EventHandler valueChangedHandler = (_, __) => handlerWasCalled = true; + int handlerCalledCount = 0; + + EventHandler valueChangedHandler1 = (_, __) => handlerCalledCount++; + EventHandler valueChangedHandler2 = (_, __) => handlerCalledCount++; + PropertyDescriptor propertyDescriptor = properties.Find(nameof(component.Property), false); - propertyDescriptor.AddValueChanged(component, valueChangedHandler); - propertyDescriptor.RemoveValueChanged(component, valueChangedHandler); + propertyDescriptor.AddValueChanged(component, valueChangedHandler1); + propertyDescriptor.AddValueChanged(component, valueChangedHandler2); + propertyDescriptor.SetValue(component, int.MaxValue); + Assert.Equal(2, handlerCalledCount); + propertyDescriptor.SetValue(component, int.MaxValue); + Assert.Equal(4, handlerCalledCount); - Assert.False(handlerWasCalled); + propertyDescriptor.RemoveValueChanged(component, valueChangedHandler1); + propertyDescriptor.RemoveValueChanged(component, valueChangedHandler2); + propertyDescriptor.SetValue(component, int.MaxValue); + Assert.Equal(4, handlerCalledCount); } [Fact]