Skip to content

Commit a49bd65

Browse files
authored
OnManagedConnectionFailed thread/memory leak fix (#1710)
Hi. I faced a problem with 'ConnectionMultiplexer': when it loses connection with Redis cluster, 'OnManagedConnectionFailed' event handler creates timer, which tries to 'SwitchMaster'. Timer ticks every second, but there is PLINQ query in 'GetConfiguredMasterForService' method, that leads to constantly threads starting (threads start faster than they exit). And if disconnect time is long enough, I get thread/memory leak in service. PR contains hotfix of a problem with timer (timer starts next iteration only after previous iteration complete). But maybe you should take a closer look at PLINQ queries (they seem superfluous). Also, I commented `TextWriter.Null` creation in 'SwitchMaster', because it seems like useless memory allocations (you handle nullable LogProxy).
1 parent 119f6fa commit a49bd65

File tree

1 file changed

+11
-8
lines changed

1 file changed

+11
-8
lines changed

src/StackExchange.Redis/ConnectionMultiplexer.cs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2477,11 +2477,9 @@ internal void OnManagedConnectionRestored(object sender, ConnectionFailedEventAr
24772477
{
24782478
ConnectionMultiplexer connection = (ConnectionMultiplexer)sender;
24792479

2480-
if (connection.sentinelMasterReconnectTimer != null)
2481-
{
2482-
connection.sentinelMasterReconnectTimer.Dispose();
2483-
connection.sentinelMasterReconnectTimer = null;
2484-
}
2480+
var oldTimer = Interlocked.Exchange(ref connection.sentinelMasterReconnectTimer, null);
2481+
oldTimer?.Dispose();
2482+
24852483
try
24862484
{
24872485

@@ -2524,7 +2522,7 @@ internal void OnManagedConnectionFailed(object sender, ConnectionFailedEventArgs
25242522
// or if we miss the published master change
25252523
if (connection.sentinelMasterReconnectTimer == null)
25262524
{
2527-
connection.sentinelMasterReconnectTimer = new Timer((_) =>
2525+
connection.sentinelMasterReconnectTimer = new Timer(_ =>
25282526
{
25292527
try
25302528
{
@@ -2533,9 +2531,12 @@ internal void OnManagedConnectionFailed(object sender, ConnectionFailedEventArgs
25332531
}
25342532
catch (Exception)
25352533
{
2536-
25372534
}
2538-
}, null, TimeSpan.FromSeconds(0), TimeSpan.FromSeconds(1));
2535+
finally
2536+
{
2537+
connection.sentinelMasterReconnectTimer?.Change(TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan);
2538+
}
2539+
}, null, TimeSpan.Zero, Timeout.InfiniteTimeSpan);
25392540
}
25402541
}
25412542

@@ -2730,6 +2731,8 @@ public void Dispose()
27302731
GC.SuppressFinalize(this);
27312732
Close(!_isDisposed);
27322733
sentinelConnection?.Dispose();
2734+
var oldTimer = Interlocked.Exchange(ref sentinelMasterReconnectTimer, null);
2735+
oldTimer?.Dispose();
27332736
}
27342737

27352738
internal Task<T> ExecuteAsyncImpl<T>(Message message, ResultProcessor<T> processor, object state, ServerEndPoint server)

0 commit comments

Comments
 (0)