[NativeAOT] Better fix for nativeaot lock/typesystem reentrancy (#70605)

* Better fix for nativeaot lock/typesystem reentrancy

* Turn list into a ring

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
* Update a comment

* CR feedback

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadInt64PersistentCounter.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadInt64PersistentCounter.cs
index 2c39a5d..c2d950e 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadInt64PersistentCounter.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadInt64PersistentCounter.cs
@@ -9,16 +9,20 @@ namespace System.Threading
 {
     internal sealed class ThreadInt64PersistentCounter
     {
-        private static readonly LowLevelLock s_lock = new LowLevelLock();
+        private readonly LowLevelLock _lock = new LowLevelLock();
 
         [ThreadStatic]
         private static List<ThreadLocalNodeFinalizationHelper>? t_nodeFinalizationHelpers;
 
         private long _overflowCount;
 
-        // we pass comparer explicitly so that in NativeAOT case we would not end up
-        // calling into the type system when lock contention increments its counter
-        private HashSet<ThreadLocalNode> _nodes = new HashSet<ThreadLocalNode>(ObjectEqualityComparer<object>.Default);
+        // dummy node serving as a start and end of the ring list
+        private ThreadLocalNode _nodes;
+
+        public ThreadInt64PersistentCounter()
+        {
+            _nodes = new ThreadLocalNode(this);
+        }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public static void Increment(object threadLocalCountObject)
@@ -41,14 +45,17 @@ namespace System.Threading
             List<ThreadLocalNodeFinalizationHelper>? nodeFinalizationHelpers = t_nodeFinalizationHelpers ??= new List<ThreadLocalNodeFinalizationHelper>(1);
             nodeFinalizationHelpers.Add(new ThreadLocalNodeFinalizationHelper(node));
 
-            s_lock.Acquire();
+            _lock.Acquire();
             try
             {
-                _nodes.Add(node);
+                node._next = _nodes._next;
+                node._prev = _nodes;
+                _nodes._next._prev = node;
+                _nodes._next = node;
             }
             finally
             {
-                s_lock.Release();
+                _lock.Release();
             }
 
             return node;
@@ -58,22 +65,21 @@ namespace System.Threading
         {
             get
             {
-                s_lock.Acquire();
+                _lock.Acquire();
                 long count = _overflowCount;
                 try
                 {
-                    foreach (ThreadLocalNode node in _nodes)
+                    ThreadLocalNode first = _nodes;
+                    ThreadLocalNode node = first._next;
+                    while (node != first)
                     {
                         count += node.Count;
+                        node = node._next;
                     }
                 }
-                catch (OutOfMemoryException)
-                {
-                    // Some allocation occurs above and it may be a bit awkward to get an OOM from this property getter
-                }
                 finally
                 {
-                    s_lock.Release();
+                    _lock.Release();
                 }
 
                 return count;
@@ -85,24 +91,31 @@ namespace System.Threading
             private uint _count;
             private readonly ThreadInt64PersistentCounter _counter;
 
+            internal ThreadLocalNode _prev;
+            internal ThreadLocalNode _next;
+
             public ThreadLocalNode(ThreadInt64PersistentCounter counter)
             {
                 Debug.Assert(counter != null);
                 _counter = counter;
+                _prev = this;
+                _next = this;
             }
 
             public void Dispose()
             {
                 ThreadInt64PersistentCounter counter = _counter;
-                s_lock.Acquire();
+                counter._lock.Acquire();
                 try
                 {
                     counter._overflowCount += _count;
-                    counter._nodes.Remove(this);
+
+                    _prev._next = _next;
+                    _next._prev = _prev;
                 }
                 finally
                 {
-                    s_lock.Release();
+                    counter._lock.Release();
                 }
             }
 
@@ -145,7 +158,7 @@ namespace System.Threading
                 // The lock, in coordination with other places that read these values, ensures that both changes below become
                 // visible together
                 ThreadInt64PersistentCounter counter = _counter;
-                s_lock.Acquire();
+                counter._lock.Acquire();
                 try
                 {
                     counter._overflowCount += (long)_count + count;
@@ -153,7 +166,7 @@ namespace System.Threading
                 }
                 finally
                 {
-                    s_lock.Release();
+                    counter._lock.Release();
                 }
             }
         }