From b6b681f13534a53714e4217a829af6cf2bbcad33 Mon Sep 17 00:00:00 2001
From: Ben Adams <thundercat@illyriad.co.uk>
Date: Fri, 24 May 2024 22:54:35 +0100
Subject: [PATCH] Use tighter locking for Node list enumerator (#7070)

---
 .../RoutingTable/NodeBucket.cs                | 24 ++++++-------
 .../RoutingTable/NodeTable.cs                 | 35 ++++++++-----------
 2 files changed, 26 insertions(+), 33 deletions(-)

diff --git a/src/Nethermind/Nethermind.Network.Discovery/RoutingTable/NodeBucket.cs b/src/Nethermind/Nethermind.Network.Discovery/RoutingTable/NodeBucket.cs
index e0a5445a4fa..6ce661d0546 100644
--- a/src/Nethermind/Nethermind.Network.Discovery/RoutingTable/NodeBucket.cs
+++ b/src/Nethermind/Nethermind.Network.Discovery/RoutingTable/NodeBucket.cs
@@ -51,9 +51,11 @@ public struct BondedItemsEnumerator : IEnumerator<NodeBucketItem>, IEnumerable<N
         public BondedItemsEnumerator(NodeBucket nodeBucket)
         {
             _nodeBucket = nodeBucket;
+            lock (_nodeBucket._nodeBucketLock)
+            {
+                _currentNode = nodeBucket._items.Last;
+            }
             _referenceTime = DateTime.UtcNow;
-            Monitor.Enter(_nodeBucket._nodeBucketLock);
-            _currentNode = nodeBucket._items.Last;
             Current = null!;
         }
 
@@ -63,13 +65,16 @@ public BondedItemsEnumerator(NodeBucket nodeBucket)
 
         public bool MoveNext()
         {
-            while (_currentNode is not null)
+            lock (_nodeBucket._nodeBucketLock)
             {
-                Current = _currentNode.Value;
-                _currentNode = _currentNode.Previous;
-                if (Current.IsBonded(_referenceTime))
+                while (_currentNode is not null)
                 {
-                    return true;
+                    Current = _currentNode.Value;
+                    _currentNode = _currentNode.Previous;
+                    if (Current.IsBonded(_referenceTime))
+                    {
+                        return true;
+                    }
                 }
             }
 
@@ -81,11 +86,6 @@ public bool MoveNext()
 
         public void Dispose()
         {
-            if (_nodeBucket is not null)
-            {
-                Monitor.Exit(_nodeBucket._nodeBucketLock);
-            }
-            _nodeBucket = null!;
         }
         public BondedItemsEnumerator GetEnumerator() => this;
 
diff --git a/src/Nethermind/Nethermind.Network.Discovery/RoutingTable/NodeTable.cs b/src/Nethermind/Nethermind.Network.Discovery/RoutingTable/NodeTable.cs
index c0a5813e395..4dc4eae50d7 100644
--- a/src/Nethermind/Nethermind.Network.Discovery/RoutingTable/NodeTable.cs
+++ b/src/Nethermind/Nethermind.Network.Discovery/RoutingTable/NodeTable.cs
@@ -105,32 +105,25 @@ public ClosestNodesEnumerator(NodeBucket[] buckets, int bucketSize)
 
         public bool MoveNext()
         {
-            try
+            while (_count < _bucketSize)
             {
-                while (_count < _bucketSize)
+                if (!_enumeratorSet || !_itemEnumerator.MoveNext())
                 {
-                    if (!_enumeratorSet || !_itemEnumerator.MoveNext())
+                    _itemEnumerator.Dispose();
+                    _bucketIndex++;
+                    if (_bucketIndex >= _buckets.Length)
                     {
-                        _itemEnumerator.Dispose();
-                        _bucketIndex++;
-                        if (_bucketIndex >= _buckets.Length)
-                        {
-                            return false;
-                        }
-
-                        _itemEnumerator = _buckets[_bucketIndex].BondedItems.GetEnumerator();
-                        _enumeratorSet = true;
-                        continue;
+                        return false;
                     }
 
-                    Current = _itemEnumerator.Current.Node!;
-                    _count++;
-                    return true;
+                    _itemEnumerator = _buckets[_bucketIndex].BondedItems.GetEnumerator();
+                    _enumeratorSet = true;
+                    continue;
                 }
-            }
-            finally
-            {
-                _itemEnumerator.Dispose();
+
+                Current = _itemEnumerator.Current.Node!;
+                _count++;
+                return true;
             }
 
             return false;
@@ -138,7 +131,7 @@ public bool MoveNext()
 
         void IEnumerator.Reset() => throw new NotSupportedException();
 
-        public void Dispose() { }
+        public void Dispose() => _itemEnumerator.Dispose();
 
         public ClosestNodesEnumerator GetEnumerator() => this;