Skip to content

Commit 2d9fe31

Browse files
committed
CSHARP-3302: Lock eliminated at ServerMonitor.RequestHeartbeat. Eliminated concurrent Cluster.RapidHeartbeatTimerCallback invocations.
1 parent 91b5a43 commit 2d9fe31

File tree

5 files changed

+372
-16
lines changed

5 files changed

+372
-16
lines changed

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

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ internal abstract class Cluster : ICluster
3737
{
3838
#region static
3939
// static fields
40-
private static readonly TimeSpan __minHeartbeatInterval = TimeSpan.FromMilliseconds(500);
40+
private static readonly TimeSpan __minHeartbeatIntervalDefault = TimeSpan.FromMilliseconds(500);
4141
private static readonly SemanticVersion __minSupportedServerVersion = new SemanticVersion(2, 6, 0);
4242
private static readonly IServerSelector __randomServerSelector = new RandomServerSelector();
4343
private static readonly Range<int> __supportedWireVersionRange = new Range<int>(2, 9);
@@ -61,6 +61,7 @@ internal abstract class Cluster : ICluster
6161
#endregion
6262

6363
// fields
64+
private readonly TimeSpan _minHeartbeatInterval = __minHeartbeatIntervalDefault;
6465
private readonly IClusterClock _clusterClock = new ClusterClock();
6566
private readonly ClusterId _clusterId;
6667
private CryptClient _cryptClient = null;
@@ -75,6 +76,7 @@ internal abstract class Cluster : ICluster
7576
private readonly ICoreServerSessionPool _serverSessionPool;
7677
private readonly ClusterSettings _settings;
7778
private readonly InterlockedInt32 _state;
79+
private readonly InterlockedInt32 _rapidHeartbeatTimerCallbackState;
7880

7981
private readonly Action<ClusterDescriptionChangedEvent> _descriptionChangedEventHandler;
8082
private readonly Action<ClusterSelectingServerEvent> _selectingServerEventHandler;
@@ -88,6 +90,7 @@ protected Cluster(ClusterSettings settings, IClusterableServerFactory serverFact
8890
_serverFactory = Ensure.IsNotNull(serverFactory, nameof(serverFactory));
8991
Ensure.IsNotNull(eventSubscriber, nameof(eventSubscriber));
9092
_state = new InterlockedInt32(State.Initial);
93+
_rapidHeartbeatTimerCallbackState = new InterlockedInt32(RapidHeartbeatTimerCallbackState.NotRunning);
9194

9295
_clusterId = new ClusterId();
9396
_description = ClusterDescription.CreateInitial(_clusterId, _settings.ConnectionMode);
@@ -178,7 +181,7 @@ private void EnterServerSelectionWaitQueue()
178181

179182
if (++_serverSelectionWaitQueueSize == 1)
180183
{
181-
_rapidHeartbeatTimer.Change(TimeSpan.Zero, __minHeartbeatInterval);
184+
_rapidHeartbeatTimer.Change(TimeSpan.Zero, _minHeartbeatInterval);
182185
}
183186
}
184187
}
@@ -208,15 +211,23 @@ public virtual void Initialize()
208211

209212
private void RapidHeartbeatTimerCallback(object args)
210213
{
211-
try
214+
// avoid requesting heartbeat reentrantly
215+
if (_rapidHeartbeatTimerCallbackState.TryChange(RapidHeartbeatTimerCallbackState.NotRunning, RapidHeartbeatTimerCallbackState.Running))
212216
{
213-
RequestHeartbeat();
214-
}
215-
catch
216-
{
217-
// TODO: Trace this
218-
// If we don't protect this call, we could
219-
// take down the app domain.
217+
try
218+
{
219+
RequestHeartbeat();
220+
}
221+
catch
222+
{
223+
// TODO: Trace this
224+
// If we don't protect this call, we could
225+
// take down the app domain.
226+
}
227+
finally
228+
{
229+
_rapidHeartbeatTimerCallbackState.TryChange(RapidHeartbeatTimerCallbackState.NotRunning);
230+
}
220231
}
221232
}
222233

@@ -596,5 +607,11 @@ private static class State
596607
public const int Open = 1;
597608
public const int Disposed = 2;
598609
}
610+
611+
private static class RapidHeartbeatTimerCallbackState
612+
{
613+
public const int NotRunning = 0;
614+
public const int Running = 1;
615+
}
599616
}
600617
}

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
*/
1515

1616
using System;
17-
using System.Net;
1817
using System.Threading;
1918
using System.Threading.Tasks;
2019

@@ -63,7 +62,14 @@ public void RequestHeartbeat()
6362
}
6463
else
6564
{
66-
_timer.Change(earlyHeartbeatDelay, Timeout.InfiniteTimeSpan);
65+
try
66+
{
67+
_timer.Change(earlyHeartbeatDelay, Timeout.InfiniteTimeSpan);
68+
}
69+
catch (ObjectDisposedException)
70+
{
71+
// Allow timer to be disposed during RequestHeartbeat
72+
}
6773
}
6874
}
6975
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,10 @@ public void Initialize()
143143
public void RequestHeartbeat()
144144
{
145145
ThrowIfNotOpen();
146-
lock (_lock)
147-
{
148-
_heartbeatDelay?.RequestHeartbeat();
149-
}
146+
147+
// CSHARP-3302: Accessing _heartbeatDelay inside _lock can lead to deadlock when processing concurrent heartbeats from old and new primaries.
148+
// Accessing _heartbeatDelay outside of _lock avoids the deadlock and will at worst reference the previous delay
149+
_heartbeatDelay?.RequestHeartbeat();
150150
}
151151

152152
// private methods

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,5 +615,8 @@ private void AddOrRemoveServers(ClusterDescription clusterDescription)
615615
internal static class ClusterReflector
616616
{
617617
public static InterlockedInt32 _state(this Cluster cluster) => (InterlockedInt32)Reflector.GetFieldValue(cluster, nameof(_state));
618+
619+
public static TimeSpan _minHeartbeatInterval(this Cluster cluster) => (TimeSpan)Reflector.GetFieldValue(cluster, nameof(_minHeartbeatInterval));
620+
public static void _minHeartbeatInterval(this Cluster cluster, TimeSpan timeSpan) => Reflector.SetFieldValue(cluster, nameof(_minHeartbeatInterval), timeSpan);
618621
}
619622
}

0 commit comments

Comments
 (0)