Skip to content

Commit

Permalink
[release/6.0] ComponentModel threading fixes (#107354)
Browse files Browse the repository at this point in the history
  • Loading branch information
@steveharter
steveharter committed Sep 6, 2024
1 parent 875f45f commit e32da1e
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 93 deletions.
1 change: 1 addition & 0 deletions ...raries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@
</ItemGroup>
<ItemGroup>
<Reference Include="System.Collections" />
<Reference Include="System.Collections.Concurrent" />
<Reference Include="System.Collections.NonGeneric" />
<Reference Include="System.Collections.Specialized" />
<Reference Include="System.ComponentModel" />
Expand Down
39 changes: 23 additions & 16 deletions ...aries/System.ComponentModel.TypeConverter/src/System/ComponentModel/PropertyDescriptor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +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
{
Expand All @@ -15,10 +18,11 @@ public abstract class PropertyDescriptor : MemberDescriptor
internal const string PropertyDescriptorPropertyTypeMessage = "PropertyDescriptor's PropertyType cannot be statically discovered.";

private TypeConverter? _converter;
private Hashtable? _valueChangedHandlers;
private ConcurrentDictionary<object, EventHandler?>? _valueChangedHandlers;
private object?[]? _editors;
private Type[]? _editorTypes;
private int _editorCount;
private object? _syncObject;

/// <summary>
/// Initializes a new instance of the <see cref='System.ComponentModel.PropertyDescriptor'/> class with the specified name and
Expand Down Expand Up @@ -48,6 +52,8 @@ protected PropertyDescriptor(MemberDescriptor descr, Attribute[]? attrs) : base(
{
}

private object SyncObject => LazyInitializer.EnsureInitialized(ref _syncObject);

/// <summary>
/// When overridden in a derived class, gets the type of the
/// component this property is bound to.
Expand Down Expand Up @@ -132,13 +138,11 @@ public virtual void AddValueChanged(object component, EventHandler handler)
throw new ArgumentNullException(nameof(handler));
}

if (_valueChangedHandlers == null)
lock (SyncObject)
{
_valueChangedHandlers = new Hashtable();
_valueChangedHandlers ??= new ConcurrentDictionary<object, EventHandler?>(concurrencyLevel: 1, capacity: 0);
_valueChangedHandlers.AddOrUpdate(component, handler, (k, v) => (EventHandler?)Delegate.Combine(v, handler));
}

EventHandler? h = (EventHandler?)_valueChangedHandlers[component];
_valueChangedHandlers[component] = Delegate.Combine(h, handler);
}

/// <summary>
Expand Down Expand Up @@ -392,7 +396,7 @@ protected virtual void OnValueChanged(object? component, EventArgs e)
{
if (component != null)
{
((EventHandler?)_valueChangedHandlers?[component])?.Invoke(component, e);
_valueChangedHandlers?.GetValueOrDefault(component, defaultValue: null)?.Invoke(component, e);
}
}

Expand All @@ -412,15 +416,18 @@ public virtual void RemoveValueChanged(object component, EventHandler handler)

if (_valueChangedHandlers != null)
{
EventHandler? h = (EventHandler?)_valueChangedHandlers[component];
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.TryRemove(component, out EventHandler? _);
}
}
}
}
Expand All @@ -434,7 +441,7 @@ public virtual void RemoveValueChanged(object component, EventHandler handler)
{
if (component != null && _valueChangedHandlers != null)
{
return (EventHandler?)_valueChangedHandlers[component];
return _valueChangedHandlers.GetValueOrDefault(component, defaultValue: null);
}
else
{
Expand Down
65 changes: 21 additions & 44 deletions ....ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -23,10 +24,8 @@ namespace System.ComponentModel
/// </summary>
internal sealed partial class ReflectTypeDescriptionProvider : TypeDescriptionProvider
{
// Hastable of Type -> ReflectedTypeData. ReflectedTypeData contains all
// of the type information we have gathered for a given type.
//
private Hashtable? _typeData;
// ReflectedTypeData contains all of the type information we have gathered for a given type.
private readonly ConcurrentDictionary<Type, ReflectedTypeData> _typeData = new ConcurrentDictionary<Type, ReflectedTypeData>();

// 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;
Expand Down Expand Up @@ -81,8 +80,6 @@ internal sealed partial class ReflectTypeDescriptionProvider : TypeDescriptionPr

internal static Guid ExtenderProviderKey { get; } = Guid.NewGuid();


private static readonly object s_internalSyncObject = new object();
/// <summary>
/// Creates a new ReflectTypeDescriptionProvider. The type is the
/// type we will obtain type information for.
Expand Down Expand Up @@ -234,7 +231,7 @@ internal static void AddEditorTable(Type editorBaseType, Hashtable table)
// don't throw; RTM didn't so we can't do it either.
}

lock (s_internalSyncObject)
lock (TypeDescriptor.s_commonSyncObject)
{
Hashtable editorTables = EditorTables;
if (!editorTables.ContainsKey(editorBaseType))
Expand Down Expand Up @@ -289,7 +286,6 @@ internal static void AddEditorTable(Type editorBaseType, Hashtable table)
return obj ?? Activator.CreateInstance(objectType, args);
}


/// <summary>
/// Helper method to create editors and type converters. This checks to see if the
/// type implements a Type constructor, and if it does it invokes that ctor.
Expand Down Expand Up @@ -421,7 +417,7 @@ internal TypeConverter GetConverter([DynamicallyAccessedMembers(DynamicallyAcces
//
if (table == null)
{
lock (s_internalSyncObject)
lock (TypeDescriptor.s_commonSyncObject)
{
table = editorTables[editorBaseType];
if (table == null)
Expand Down Expand Up @@ -838,22 +834,11 @@ internal Type[] GetPopulatedTypes(Module module)
{
List<Type> typeList = new List<Type>();

lock (s_internalSyncObject)
foreach (KeyValuePair<Type, ReflectedTypeData> kvp in _typeData)
{
Hashtable? typeData = _typeData;
if (typeData != null)
if (kvp.Key.Module == module && kvp.Value!.IsPopulated)
{
// Manual use of IDictionaryEnumerator instead of foreach to avoid DictionaryEntry box allocations.
IDictionaryEnumerator e = typeData.GetEnumerator();
while (e.MoveNext())
{
DictionaryEntry de = e.Entry;
Type type = (Type)de.Key;
if (type.Module == module && ((ReflectedTypeData)de.Value!).IsPopulated)
{
typeList.Add(type);
}
}
typeList.Add(kvp.Key);
}
}

Expand Down Expand Up @@ -898,31 +883,23 @@ public override Type GetReflectionType(
/// </summary>
private ReflectedTypeData? GetTypeData([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type type, bool createIfNeeded)
{
ReflectedTypeData? td = null;

if (_typeData != null)
if (_typeData.TryGetValue(type, out ReflectedTypeData? td))
{
td = (ReflectedTypeData?)_typeData[type];
if (td != null)
{
return td;
}
Debug.Assert(td != null);
return td;
}

lock (s_internalSyncObject)
lock (TypeDescriptor.s_commonSyncObject)
{
if (_typeData != null)
if (_typeData.TryGetValue(type, out td))
{
td = (ReflectedTypeData?)_typeData[type];
Debug.Assert(td != null);
return td;
}

if (td == null && createIfNeeded)
if (createIfNeeded)
{
td = new ReflectedTypeData(type);
if (_typeData == null)
{
_typeData = new Hashtable();
}
_typeData[type] = td;
}
}
Expand Down Expand Up @@ -1010,7 +987,7 @@ internal static Attribute[] ReflectGetAttributes(Type type)
return attrs;
}

lock (s_internalSyncObject)
lock (TypeDescriptor.s_commonSyncObject)
{
attrs = (Attribute[]?)attributeCache[type];
if (attrs == null)
Expand Down Expand Up @@ -1038,7 +1015,7 @@ internal static Attribute[] ReflectGetAttributes(MemberInfo member)
return attrs;
}

lock (s_internalSyncObject)
lock (TypeDescriptor.s_commonSyncObject)
{
attrs = (Attribute[]?)attributeCache[member];
if (attrs == null)
Expand Down Expand Up @@ -1067,7 +1044,7 @@ private static EventDescriptor[] ReflectGetEvents(
return events;
}

lock (s_internalSyncObject)
lock (TypeDescriptor.s_commonSyncObject)
{
events = (EventDescriptor[]?)eventCache[type];
if (events == null)
Expand Down Expand Up @@ -1164,7 +1141,7 @@ private static PropertyDescriptor[] ReflectGetExtendedProperties(IExtenderProvid
ReflectPropertyDescriptor[]? extendedProperties = (ReflectPropertyDescriptor[]?)extendedPropertyCache[providerType];
if (extendedProperties == null)
{
lock (s_internalSyncObject)
lock (TypeDescriptor.s_commonSyncObject)
{
extendedProperties = (ReflectPropertyDescriptor[]?)extendedPropertyCache[providerType];

Expand Down Expand Up @@ -1244,7 +1221,7 @@ private static PropertyDescriptor[] ReflectGetProperties(
return properties;
}

lock (s_internalSyncObject)
lock (TypeDescriptor.s_commonSyncObject)
{
properties = (PropertyDescriptor[]?)propertyCache[type];

Expand Down
Loading

0 comments on commit e32da1e

Please sign in to comment.