Skip to content

Commit 6508d75

Browse files
committed
CSHARP-1690: Fix race condition with BinaryConnection Dispose.
1 parent 9f93fe1 commit 6508d75

File tree

2 files changed

+58
-10
lines changed

2 files changed

+58
-10
lines changed

src/MongoDB.Driver.Core/Core/Connections/BinaryConnection.cs

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ internal class BinaryConnection : IConnection
4848
private EndPoint _endPoint;
4949
private ConnectionDescription _description;
5050
private readonly Dropbox _dropbox = new Dropbox();
51+
private bool _failedEventHasBeenRaised;
5152
private DateTime _lastUsedAtUtc;
5253
private DateTime _openedAtUtc;
5354
private readonly object _openLock = new object();
@@ -151,8 +152,18 @@ public ConnectionSettings Settings
151152
// methods
152153
private void ConnectionFailed(Exception exception)
153154
{
154-
if (_state.TryChange(State.Open, State.Failed))
155+
if (!_state.TryChange(State.Open, State.Failed))
155156
{
157+
var currentState = _state.Value;
158+
if (currentState != State.Failed && currentState != State.Disposed)
159+
{
160+
throw new InvalidOperationException($"Invalid BinaryConnection state transition from {currentState} to Failed.");
161+
}
162+
}
163+
164+
if (!_failedEventHasBeenRaised)
165+
{
166+
_failedEventHasBeenRaised = true;
156167
if (_failedEventHandler != null)
157168
{
158169
_failedEventHandler(new ConnectionFailedEvent(_connectionId, exception));
@@ -661,7 +672,14 @@ public OpenConnectionHelper(BinaryConnection connection)
661672

662673
public void FailedOpeningConnection(Exception wrappedException)
663674
{
664-
_connection._state.TryChange(State.Failed);
675+
if (!_connection._state.TryChange(State.Connecting, State.Failed) && !_connection._state.TryChange(State.Initializing, State.Failed))
676+
{
677+
var currentState = _connection._state.Value;
678+
if (currentState != State.Disposed)
679+
{
680+
throw new InvalidOperationException($"Invalid BinaryConnection state transition from {currentState} to Failed.");
681+
}
682+
}
665683

666684
var handler = _connection._failedOpeningEventHandler;
667685
if (handler != null)
@@ -672,14 +690,37 @@ public void FailedOpeningConnection(Exception wrappedException)
672690

673691
public void InitializingConnection()
674692
{
675-
_connection._state.TryChange(State.Initializing);
693+
if (!_connection._state.TryChange(State.Connecting, State.Initializing))
694+
{
695+
var currentState = _connection._state.Value;
696+
if (currentState == State.Disposed)
697+
{
698+
throw new ObjectDisposedException(typeof(BinaryConnection).Name);
699+
}
700+
else
701+
{
702+
throw new InvalidOperationException($"Invalid BinaryConnection state transition from {currentState} to Initializing.");
703+
}
704+
}
676705
}
677706

678707
public void OpenedConnection()
679708
{
680709
_stopwatch.Stop();
681710
_connection._connectionId = _connection._description.ConnectionId;
682-
_connection._state.TryChange(State.Open);
711+
712+
if (!_connection._state.TryChange(State.Initializing, State.Open))
713+
{
714+
var currentState = _connection._state.Value;
715+
if (currentState == State.Disposed)
716+
{
717+
throw new ObjectDisposedException(typeof(BinaryConnection).Name);
718+
}
719+
else
720+
{
721+
throw new InvalidOperationException($"Invalid BinaryConnection state transition from {currentState} to Open.");
722+
}
723+
}
683724

684725
var handler = _connection._openedEventHandler;
685726
if (handler != null)
@@ -883,6 +924,7 @@ public void SentMessages(int bufferLength)
883924

884925
private static class State
885926
{
927+
// note: the numeric values matter because sometimes we compare their magnitudes
886928
public static int Initial = 0;
887929
public static int Connecting = 1;
888930
public static int Initializing = 2;

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ internal sealed class ServerMonitor : IServerMonitor
3434
private readonly ExponentiallyWeightedMovingAverage _averageRoundTripTimeCalculator = new ExponentiallyWeightedMovingAverage(0.2);
3535
private readonly ServerDescription _baseDescription;
3636
private readonly CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource();
37-
private IConnection _connection;
37+
private volatile IConnection _connection;
3838
private readonly IConnectionFactory _connectionFactory;
3939
private ServerDescription _currentDescription;
4040
private readonly EndPoint _endPoint;
@@ -136,25 +136,31 @@ private async Task<bool> HeartbeatAsync(CancellationToken cancellationToken)
136136
Exception heartbeatException = null;
137137
for (var attempt = 1; attempt <= maxRetryCount; attempt++)
138138
{
139+
var connection = _connection;
139140
try
140141
{
141-
if (_connection == null)
142+
if (connection == null)
142143
{
143-
_connection = _connectionFactory.CreateConnection(_serverId, _endPoint);
144+
connection = _connectionFactory.CreateConnection(_serverId, _endPoint);
144145
// if we are cancelling, it's because the server has
145146
// been shut down and we really don't need to wait.
146-
await _connection.OpenAsync(cancellationToken).ConfigureAwait(false);
147+
await connection.OpenAsync(cancellationToken).ConfigureAwait(false);
147148
}
148149

149-
heartbeatInfo = await GetHeartbeatInfoAsync(_connection, cancellationToken).ConfigureAwait(false);
150+
heartbeatInfo = await GetHeartbeatInfoAsync(connection, cancellationToken).ConfigureAwait(false);
150151
heartbeatException = null;
152+
153+
_connection = connection;
151154
break;
152155
}
153156
catch (Exception ex)
154157
{
155158
heartbeatException = ex;
156-
_connection.Dispose();
157159
_connection = null;
160+
if (connection != null)
161+
{
162+
connection.Dispose();
163+
}
158164
}
159165
}
160166

0 commit comments

Comments
 (0)