diff --git a/eng/pipelines/coreclr/templates/helix-queues-setup.yml b/eng/pipelines/coreclr/templates/helix-queues-setup.yml index b7311bb317983..8130fabfda83d 100644 --- a/eng/pipelines/coreclr/templates/helix-queues-setup.yml +++ b/eng/pipelines/coreclr/templates/helix-queues-setup.yml @@ -32,7 +32,7 @@ jobs: # Android arm64 - ${{ if in(parameters.platform, 'Android_arm64') }}: - - Windows.10.Amd64.Android.Open + - Windows.11.Amd64.Android.Open # Android x64 - ${{ if in(parameters.platform, 'Android_x64') }}: diff --git a/eng/pipelines/libraries/helix-queues-setup.yml b/eng/pipelines/libraries/helix-queues-setup.yml index 6d35d023b1188..a2d8b893c7f46 100644 --- a/eng/pipelines/libraries/helix-queues-setup.yml +++ b/eng/pipelines/libraries/helix-queues-setup.yml @@ -95,7 +95,7 @@ jobs: - ${{ if in(parameters.platform, 'Android_x86', 'Android_x64') }}: - Ubuntu.2204.Amd64.Android.29.Open - ${{ if in(parameters.platform, 'Android_arm', 'Android_arm64') }}: - - Windows.10.Amd64.Android.Open + - Windows.11.Amd64.Android.Open # iOS Simulator/Mac Catalyst arm64 - ${{ if in(parameters.platform, 'MacCatalyst_arm64', 'iOSSimulator_arm64') }}: diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509_macos.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509_macos.c index 41ba9648259c7..fc261117a1588 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509_macos.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509_macos.c @@ -391,7 +391,7 @@ int32_t AppleCryptoNative_X509CopyWithPrivateKey(SecCertificateRef cert, SecKeychainItemRef itemCopy = NULL; // This only happens with an ephemeral key, so the keychain we're adding it to is temporary. - if (status == errSecNoSuchKeychain) + if (status == errSecNoSuchKeychain || status == errSecInvalidItemRef) { status = AddKeyToKeychain(privateKey, targetKeychain, NULL); } 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 db7c6d6c275cd..997eabda752e5 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System.ComponentModel.TypeConverter.csproj @@ -240,6 +240,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 9ffaaa702da16..51feae8627fb9 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/PropertyDescriptor.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/PropertyDescriptor.cs @@ -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 { @@ -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? _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 @@ -48,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. @@ -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(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); } /// @@ -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); } } @@ -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? _); + } } } } @@ -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 { 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 ef3cc90c16dbb..969c6712dfd3b 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,10 +24,8 @@ namespace System.ComponentModel /// 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 _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; @@ -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(); /// /// Creates a new ReflectTypeDescriptionProvider. The type is the /// type we will obtain type information for. @@ -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)) @@ -289,7 +286,6 @@ internal static void AddEditorTable(Type editorBaseType, Hashtable table) return obj ?? Activator.CreateInstance(objectType, args); } - /// /// 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. @@ -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) @@ -838,22 +834,11 @@ internal Type[] GetPopulatedTypes(Module module) { List typeList = new List(); - lock (s_internalSyncObject) + foreach (KeyValuePair 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); } } @@ -898,31 +883,23 @@ public override Type GetReflectionType( /// 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; } } @@ -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) @@ -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) @@ -1067,7 +1044,7 @@ private static EventDescriptor[] ReflectGetEvents( return events; } - lock (s_internalSyncObject) + lock (TypeDescriptor.s_commonSyncObject) { events = (EventDescriptor[]?)eventCache[type]; if (events == null) @@ -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]; @@ -1244,7 +1221,7 @@ private static PropertyDescriptor[] ReflectGetProperties( return properties; } - lock (s_internalSyncObject) + lock (TypeDescriptor.s_commonSyncObject) { properties = (PropertyDescriptor[]?)propertyCache[type]; 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 4967187d00644..cde70538ab57d 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,8 @@ // 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; using System.Diagnostics; @@ -25,12 +27,23 @@ public sealed class TypeDescriptor // lock on it for thread safety. It is used from nearly // every call to this class, so it will be created soon after // class load anyway. - private static readonly WeakHashtable s_providerTable = new WeakHashtable(); // mapping of type or object hash to a provider list - private static readonly Hashtable s_providerTypeTable = new Hashtable(); // A direct mapping from type to provider. + private static readonly WeakHashtable s_providerTable = new WeakHashtable(); + + // This lock object protects access to several thread-unsafe areas below, and is a single lock object to prevent deadlocks. + // - During s_providerTypeTable access. + // - To act as a mutex for CheckDefaultProvider() when it needs to create the default provider, which may re-enter the above case. + // - For cache access in the ReflectTypeDescriptionProvider class which may re-enter the above case. + // - For logic added by consumers, such as custom provider, constructor and property logic, which may re-enter the above cases in unexpected ways. + internal static readonly object s_commonSyncObject = new object(); + + // A direct mapping from type to provider. + 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 ConcurrentDictionary s_defaultProviderInitialized = new ConcurrentDictionary(); - private static readonly Hashtable s_defaultProviderInitialized = new Hashtable(); // A table of type -> object to track DefaultTypeDescriptionProviderAttributes. - // A value of `null` indicates initialization is in progress. - // A value of s_initializedDefaultProvider indicates the provider is initialized. private static readonly object s_initializedDefaultProvider = new object(); private static WeakHashtable? s_associationTable; @@ -199,7 +212,7 @@ public static void AddProvider(TypeDescriptionProvider provider, Type type) throw new ArgumentNullException(nameof(type)); } - lock (s_providerTable) + lock (s_commonSyncObject) { // Get the root node, hook it up, and stuff it back into // the provider cache. @@ -235,7 +248,7 @@ public static void AddProvider(TypeDescriptionProvider provider, object instance // Get the root node, hook it up, and stuff it back into // the provider cache. - lock (s_providerTable) + lock (s_commonSyncObject) { refreshNeeded = s_providerTable.ContainsKey(instance); TypeDescriptionNode node = NodeFor(instance, true); @@ -305,15 +318,12 @@ public static void AddProviderTransparent(TypeDescriptionProvider provider, obje /// private static void CheckDefaultProvider(Type type) { - if (s_defaultProviderInitialized[type] == s_initializedDefaultProvider) + if (s_defaultProviderInitialized.TryGetValue(type, out object? provider) && provider == s_initializedDefaultProvider) { return; } - // Lock on s_providerTable even though s_providerTable is not modified here. - // Using a single lock prevents deadlocks since other methods that call into or are called - // by this method also lock on s_providerTable and the ordering of the locks may be different. - lock (s_providerTable) + lock (s_commonSyncObject) { AddDefaultProvider(type); } @@ -321,7 +331,7 @@ private static void CheckDefaultProvider(Type type) /// /// Add the default provider, if it exists. - /// For threading, this is always called under a 'lock (s_providerTable)'. + /// For threading, this is always called under a 'lock (s_commonSyncObject)'. /// private static void AddDefaultProvider(Type type) { @@ -335,7 +345,7 @@ private static void AddDefaultProvider(Type type) // Immediately set this to null to indicate we are in progress setting the default provider for a type. // This prevents re-entrance to this method. - s_defaultProviderInitialized[type] = null; + s_defaultProviderInitialized.TryAdd(type, null); // Always use core reflection when checking for the default provider attribute. // If there is a provider, we probably don't want to build up our own cache state against the type. @@ -1555,8 +1565,10 @@ private static TypeDescriptionNode NodeFor(Type type, bool createDelegator) while (node == null) { - node = (TypeDescriptionNode?)s_providerTypeTable[searchType] ?? - (TypeDescriptionNode?)s_providerTable[searchType]; + if (!s_providerTypeTable.TryGetValue(searchType, out node)) + { + node = (TypeDescriptionNode?)s_providerTable[searchType]; + } if (node == null) { @@ -1564,7 +1576,7 @@ private static TypeDescriptionNode NodeFor(Type type, bool createDelegator) if (searchType == typeof(object) || baseType == null) { - lock (s_providerTable) + lock (s_commonSyncObject) { node = (TypeDescriptionNode?)s_providerTable[searchType]; @@ -1580,9 +1592,9 @@ private static TypeDescriptionNode NodeFor(Type type, bool createDelegator) else if (createDelegator) { node = new TypeDescriptionNode(new DelegatingTypeDescriptionProvider(baseType)); - lock (s_providerTable) + lock (s_commonSyncObject) { - s_providerTypeTable[searchType] = node; + s_providerTypeTable.TryAdd(searchType, node); } } else @@ -1672,7 +1684,7 @@ private static TypeDescriptionNode NodeFor(object instance, bool createDelegator /// private static void NodeRemove(object key, TypeDescriptionProvider provider) { - lock (s_providerTable) + lock (s_commonSyncObject) { TypeDescriptionNode? head = (TypeDescriptionNode?)s_providerTable[key]; TypeDescriptionNode? target = head; @@ -2195,7 +2207,7 @@ private static void Refresh(object component, bool refreshReflectionProvider) { Type type = component.GetType(); - lock (s_providerTable) + lock (s_commonSyncObject) { // ReflectTypeDescritionProvider is only bound to object, but we // need go to through the entire table to try to find custom @@ -2279,7 +2291,7 @@ public static void Refresh(Type type) bool found = false; - lock (s_providerTable) + lock (s_commonSyncObject) { // ReflectTypeDescritionProvider is only bound to object, but we // need go to through the entire table to try to find custom @@ -2344,7 +2356,7 @@ public static void Refresh(Module module) // each of these levels. Hashtable? refreshedTypes = null; - lock (s_providerTable) + lock (s_commonSyncObject) { // Manual use of IDictionaryEnumerator instead of foreach to avoid DictionaryEntry box allocations. IDictionaryEnumerator e = s_providerTable.GetEnumerator(); 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]