CSHARP-3302: Lock eliminated at ServerMonitor.RequestHeartbeat. Eliminated concurrent Cluster.RapidHeartbeatTimerCallback invocations.

Author: BorisDog

src/MongoDB.Driver.Core/Core/Clusters/Cluster.cs

@@ -37,7 +37,7 @@ internal abstract class Cluster : ICluster
     {
         #region static
         // static fields
-        private static readonly TimeSpan __minHeartbeatInterval = TimeSpan.FromMilliseconds(500);
+        private static readonly TimeSpan __minHeartbeatIntervalDefault = TimeSpan.FromMilliseconds(500);
         private static readonly SemanticVersion __minSupportedServerVersion = new SemanticVersion(2, 6, 0);
         private static readonly IServerSelector __randomServerSelector = new RandomServerSelector();
         private static readonly Range<int> __supportedWireVersionRange = new Range<int>(2, 9);
@@ -61,6 +61,7 @@ internal abstract class Cluster : ICluster
         #endregion
 
         // fields
+        private readonly TimeSpan _minHeartbeatInterval = __minHeartbeatIntervalDefault;
         private readonly IClusterClock _clusterClock = new ClusterClock();
         private readonly ClusterId _clusterId;
         private CryptClient _cryptClient = null;
@@ -75,6 +76,7 @@ internal abstract class Cluster : ICluster
         private readonly ICoreServerSessionPool _serverSessionPool;
         private readonly ClusterSettings _settings;
         private readonly InterlockedInt32 _state;
+        private readonly InterlockedInt32 _rapidHeartbeatTimerCallbackState;
 
         private readonly Action<ClusterDescriptionChangedEvent> _descriptionChangedEventHandler;
         private readonly Action<ClusterSelectingServerEvent> _selectingServerEventHandler;
@@ -88,6 +90,7 @@ protected Cluster(ClusterSettings settings, IClusterableServerFactory serverFact
             _serverFactory = Ensure.IsNotNull(serverFactory, nameof(serverFactory));
             Ensure.IsNotNull(eventSubscriber, nameof(eventSubscriber));
             _state = new InterlockedInt32(State.Initial);
+            _rapidHeartbeatTimerCallbackState = new InterlockedInt32(RapidHeartbeatTimerCallbackState.NotRunning);
 
             _clusterId = new ClusterId();
             _description = ClusterDescription.CreateInitial(_clusterId, _settings.ConnectionMode);
@@ -178,7 +181,7 @@ private void EnterServerSelectionWaitQueue()
 
                 if (++_serverSelectionWaitQueueSize == 1)
                 {
-                    _rapidHeartbeatTimer.Change(TimeSpan.Zero, __minHeartbeatInterval);
+                    _rapidHeartbeatTimer.Change(TimeSpan.Zero, _minHeartbeatInterval);
                 }
             }
         }
@@ -208,15 +211,23 @@ public virtual void Initialize()
 
         private void RapidHeartbeatTimerCallback(object args)
         {
-            try
+            // avoid requesting heartbeat reentrantly
+            if (_rapidHeartbeatTimerCallbackState.TryChange(RapidHeartbeatTimerCallbackState.NotRunning, RapidHeartbeatTimerCallbackState.Running))
             {
-                RequestHeartbeat();
-            }
-            catch
-            {
-                // TODO: Trace this
-                // If we don't protect this call, we could
-                // take down the app domain.
+                try
+                {
+                    RequestHeartbeat();
+                }
+                catch
+                {
+                    // TODO: Trace this
+                    // If we don't protect this call, we could
+                    // take down the app domain.
+                }
+                finally
+                {
+                    _rapidHeartbeatTimerCallbackState.TryChange(RapidHeartbeatTimerCallbackState.NotRunning);
+                }
             }
         }
 
@@ -596,5 +607,11 @@ private static class State
             public const int Open = 1;
             public const int Disposed = 2;
         }
+
+        private static class RapidHeartbeatTimerCallbackState
+        {
+            public const int NotRunning = 0;
+            public const int Running = 1;
+        }
     }
 }

src/MongoDB.Driver.Core/Core/Servers/HeartbeatDelay.cs

@@ -14,7 +14,6 @@
 */
 
 using System;
-using System.Net;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -63,7 +62,14 @@ public void RequestHeartbeat()
                 }
                 else
                 {
-                    _timer.Change(earlyHeartbeatDelay, Timeout.InfiniteTimeSpan);
+                    try
+                    {
+                        _timer.Change(earlyHeartbeatDelay, Timeout.InfiniteTimeSpan);
+                    }
+                    catch (ObjectDisposedException)
+                    {
+                        // Allow timer to be disposed during RequestHeartbeat
+                    }
                 }
             }
         }

src/MongoDB.Driver.Core/Core/Servers/ServerMonitor.cs

@@ -143,10 +143,10 @@ public void Initialize()
         public void RequestHeartbeat()
         {
             ThrowIfNotOpen();
-            lock (_lock)
-            {
-                _heartbeatDelay?.RequestHeartbeat();
-            }
+
+            // CSHARP-3302: Accessing _heartbeatDelay inside _lock can lead to deadlock when processing concurrent heartbeats from old and new primaries.
+            // Accessing _heartbeatDelay outside of _lock avoids the deadlock and will at worst reference the previous delay
+            _heartbeatDelay?.RequestHeartbeat();
         }
 
         // private methods

tests/MongoDB.Driver.Core.Tests/Core/Clusters/ClusterTests.cs

@@ -615,5 +615,8 @@ private void AddOrRemoveServers(ClusterDescription clusterDescription)
     internal static class ClusterReflector
     {
         public static InterlockedInt32 _state(this Cluster cluster) => (InterlockedInt32)Reflector.GetFieldValue(cluster, nameof(_state));
+
+        public static TimeSpan _minHeartbeatInterval(this Cluster cluster) => (TimeSpan)Reflector.GetFieldValue(cluster, nameof(_minHeartbeatInterval));
+        public static void _minHeartbeatInterval(this Cluster cluster, TimeSpan timeSpan) => Reflector.SetFieldValue(cluster, nameof(_minHeartbeatInterval), timeSpan);
     }
 }