Remove _syncRoot field from collections (#21628)

* SyncRoot

* Temp disable tests

* Disable outdated CoreFX test

* Tidier

diff --git a/src/System.Private.CoreLib/shared/System/Collections/Generic/Dictionary.cs b/src/System.Private.CoreLib/shared/System/Collections/Generic/Dictionary.cs
index 4710565..dbb0bdf 100644 (file)
--- a/src/System.Private.CoreLib/shared/System/Collections/Generic/Dictionary.cs
+++ b/src/System.Private.CoreLib/shared/System/Collections/Generic/Dictionary.cs
@@ -53,7 +53,6 @@ namespace System.Collections.Generic
         private IEqualityComparer<TKey> _comparer;
         private KeyCollection _keys;
         private ValueCollection _values;
-        private object _syncRoot;
 
         // constants for serialization
         private const string VersionName = "Version"; // Do not rename (binary serialization)
@@ -1038,17 +1037,7 @@ namespace System.Collections.Generic
 
         bool ICollection.IsSynchronized => false;
 
-        object ICollection.SyncRoot
-        {
-            get
-            {
-                if (_syncRoot == null)
-                {
-                    Interlocked.CompareExchange<object>(ref _syncRoot, new object(), null);
-                }
-                return _syncRoot;
-            }
-        }
+        object ICollection.SyncRoot => this;
 
         bool IDictionary.IsFixedSize => false;
 

diff --git a/src/System.Private.CoreLib/shared/System/Collections/Generic/List.cs b/src/System.Private.CoreLib/shared/System/Collections/Generic/List.cs
index f1bb651..59dcd93 100644 (file)
--- a/src/System.Private.CoreLib/shared/System/Collections/Generic/List.cs
+++ b/src/System.Private.CoreLib/shared/System/Collections/Generic/List.cs
@@ -26,8 +26,6 @@ namespace System.Collections.Generic
         private T[] _items; // Do not rename (binary serialization)
         private int _size; // Do not rename (binary serialization)
         private int _version; // Do not rename (binary serialization)
-        [NonSerialized]
-        private object _syncRoot;
 
         private static readonly T[] s_emptyArray = new T[0];
 
@@ -142,17 +140,7 @@ namespace System.Collections.Generic
         bool ICollection.IsSynchronized => false;
 
         // Synchronization root for this object.
-        object ICollection.SyncRoot
-        {
-            get
-            {
-                if (_syncRoot == null)
-                {
-                    Interlocked.CompareExchange<object>(ref _syncRoot, new object(), null);
-                }
-                return _syncRoot;
-            }
-        }
+        object ICollection.SyncRoot => this;
 
         // Sets or Gets the element at the given index.
         public T this[int index]

diff --git a/src/System.Private.CoreLib/shared/System/Collections/Hashtable.cs b/src/System.Private.CoreLib/shared/System/Collections/Hashtable.cs
index 1c3139d..5c56212 100644 (file)
--- a/src/System.Private.CoreLib/shared/System/Collections/Hashtable.cs
+++ b/src/System.Private.CoreLib/shared/System/Collections/Hashtable.cs
@@ -150,7 +150,6 @@ namespace System.Collections
         private ICollection _values;
 
         private IEqualityComparer _keycomparer;
-        private object _syncRoot;
 
         [Obsolete("Please use EqualityComparer property.")]
         protected IHashCodeProvider hcp
@@ -1073,17 +1072,7 @@ namespace System.Collections
         }
 
         // Returns the object to synchronize on for this hash table.
-        public virtual object SyncRoot
-        {
-            get
-            {
-                if (_syncRoot == null)
-                {
-                    System.Threading.Interlocked.CompareExchange<object>(ref _syncRoot, new object(), null);
-                }
-                return _syncRoot;
-            }
-        }
+        public virtual object SyncRoot => this;
 
         // Returns the number of associations in this hashtable.
         // 

diff --git a/src/System.Private.CoreLib/shared/System/Collections/ListDictionaryInternal.cs b/src/System.Private.CoreLib/shared/System/Collections/ListDictionaryInternal.cs
index 6045778..1bc61db 100644 (file)
--- a/src/System.Private.CoreLib/shared/System/Collections/ListDictionaryInternal.cs
+++ b/src/System.Private.CoreLib/shared/System/Collections/ListDictionaryInternal.cs
@@ -26,8 +26,6 @@ namespace System.Collections
         private DictionaryNode head; // Do not rename (binary serialization)
         private int version; // Do not rename (binary serialization)
         private int count; // Do not rename (binary serialization)
-        [NonSerialized]
-        private object _syncRoot;
 
         public ListDictionaryInternal()
         {
@@ -134,17 +132,7 @@ namespace System.Collections
             }
         }
 
-        public object SyncRoot
-        {
-            get
-            {
-                if (_syncRoot == null)
-                {
-                    System.Threading.Interlocked.CompareExchange<object>(ref _syncRoot, new object(), null);
-                }
-                return _syncRoot;
-            }
-        }
+        public object SyncRoot => this;
 
         public ICollection Values
         {

diff --git a/src/System.Private.CoreLib/shared/System/Collections/ObjectModel/Collection.cs b/src/System.Private.CoreLib/shared/System/Collections/ObjectModel/Collection.cs
index 53272b3..9efd671 100644 (file)
--- a/src/System.Private.CoreLib/shared/System/Collections/ObjectModel/Collection.cs
+++ b/src/System.Private.CoreLib/shared/System/Collections/ObjectModel/Collection.cs
@@ -14,8 +14,6 @@ namespace System.Collections.ObjectModel
     public class Collection<T> : IList<T>, IList, IReadOnlyList<T>
     {
         private IList<T> items; // Do not rename (binary serialization)
-        [NonSerialized]
-        private object _syncRoot;
 
         public Collection()
         {
@@ -186,19 +184,7 @@ namespace System.Collections.ObjectModel
         {
             get
             {
-                if (_syncRoot == null)
-                {
-                    ICollection c = items as ICollection;
-                    if (c != null)
-                    {
-                        _syncRoot = c.SyncRoot;
-                    }
-                    else
-                    {
-                        System.Threading.Interlocked.CompareExchange<object>(ref _syncRoot, new object(), null);
-                    }
-                }
-                return _syncRoot;
+                return (items is ICollection coll) ? coll.SyncRoot : this;
             }
         }
 

diff --git a/src/System.Private.CoreLib/shared/System/Collections/ObjectModel/ReadOnlyCollection.cs b/src/System.Private.CoreLib/shared/System/Collections/ObjectModel/ReadOnlyCollection.cs
index 3c4eda2..584b4cc 100644 (file)
--- a/src/System.Private.CoreLib/shared/System/Collections/ObjectModel/ReadOnlyCollection.cs
+++ b/src/System.Private.CoreLib/shared/System/Collections/ObjectModel/ReadOnlyCollection.cs
@@ -14,8 +14,6 @@ namespace System.Collections.ObjectModel
     public class ReadOnlyCollection<T> : IList<T>, IList, IReadOnlyList<T>
     {
         private IList<T> list; // Do not rename (binary serialization)
-        [NonSerialized]
-        private object _syncRoot;
 
         public ReadOnlyCollection(IList<T> list)
         {
@@ -118,19 +116,7 @@ namespace System.Collections.ObjectModel
         {
             get
             {
-                if (_syncRoot == null)
-                {
-                    ICollection c = list as ICollection;
-                    if (c != null)
-                    {
-                        _syncRoot = c.SyncRoot;
-                    }
-                    else
-                    {
-                        System.Threading.Interlocked.CompareExchange<object>(ref _syncRoot, new object(), null);
-                    }
-                }
-                return _syncRoot;
+                return (list is ICollection coll) ? coll.SyncRoot : this;
             }
         }
 

diff --git a/tests/CoreFX/CoreFX.issues.json b/tests/CoreFX/CoreFX.issues.json
index 8a8c556..8b27b71 100644 (file)
--- a/tests/CoreFX/CoreFX.issues.json
+++ b/tests/CoreFX/CoreFX.issues.json
@@ -336,7 +336,73 @@
                 {
                     "name": "System.Collections.Tests.Dictionary_Generic_Tests_int_int.IEnumerable_Generic_Enumerator_MoveNext_ModifiedDuringEnumeration_ThrowsInvalidOperationException",
                     "reason": "outdated"
-                }
+                },
+                {
+                    "name": "System.Collections.Tests.HashtableBasicTests.ICollection_NonGeneric_SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
+                {
+                    "name": "System.Collections.Tests.HashtableValuesTests.ICollection_NonGeneric_SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
+                {
+                    "name": "System.Collections.Tests.Hashtable_SyncRootTests.SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
+                {
+                    "name": "System.Collections.Tests.HashtableBasicTests.ICollection_NonGeneric_SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
+                {
+                    "name": "System.Collections.Tests.HashtableValuesTests.ICollection_NonGeneric_SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
+                {
+                    "name": "System.Collections.Tests.HashtableSynchronizedTests.ICollection_NonGeneric_SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
+                {
+                    "name": "System.Collections.Tests.DictionaryBaseTests.SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
+                {
+                    "name": "System.Collections.Tests.HashtableKeysTests.ICollection_NonGeneric_SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
+                {
+                    "name": "System.Collections.Tests.Dictionary_IDictionary_NonGeneric_Tests.ICollection_NonGeneric_SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
+                {
+                    "name": "System.Collections.Tests.Dictionary_Generic_Tests_Keys_AsICollection.ICollection_NonGeneric_SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
+                {
+                    "name": "System.Collections.Tests.Dictionary_Generic_Tests_Values_AsICollection.ICollection_NonGeneric_SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
+            ]
+        }
+    },
+    {
+        "name": "System.Collections.Specialized.Tests",
+        "enabled": true,
+        "exclusions": {
+            "namespaces": null,
+            "classes": null,
+            "methods": [
+                {
+                    "name": "System.Collections.Specialized.Tests.StringDictionarySyncRootTests.SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
+                {
+                    "name": "System.Collections.Specialized.Tests.HybridDictionaryValuesTests.ICollection_NonGeneric_SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
+                {
+                    "name": "System.Collections.Specialized.Tests.HybridDictionaryKeysTests.ICollection_NonGeneric_SyncRoot",
+                    "reason": "waiting on corefx update"
+                },
             ]
         }
     },
@@ -868,7 +934,7 @@
                 },
             ]
         }
-    },    
+    },
     {
         "name": "System.Linq.Expressions.Tests",
         "enabled": true,