Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[release/6.0] ComponentModel threading fixes #107354

Merged
merged 2 commits into from
Sep 6, 2024
Merged

[release/6.0] ComponentModel threading fixes #107354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e8a5f58
Port 103835 + Dictionary changes
steveharter Sep 4, 2024
8928bab
Port 104407
steveharter Sep 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...raries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading