1742 some bug in mqttclientconnectionvalidatorhandle (#1746)

Author: chkr1011

.github/workflows/ReleaseNotes.md

@@ -8,4 +8,5 @@
 * [Client] Exposed more TLS options (#1729).
 * [Client] Fixed wrong return code conversion (#1729).
 * [Server] Improved performance by changing internal locking strategy for subscriptions (#1716, thanks to @zeheng).
+* [Server] Fixed exceptions when clients are connecting and disconnecting very fast while accessing the client status for connection validation (#1742).
 * [Server] Exposed more properties in _ClientConnectedEventArgs_ (#1738).

Source/MQTTnet/Server/Internal/MqttClientSessionsManager.cs

@@ -58,7 +58,7 @@ public MqttClientSessionsManager(
  _eventContainer = eventContainer ?? throw new ArgumentNullException(nameof(eventContainer));
  }
 
- public async Task CloseAllConnectionsAsync()
+ public async Task CloseAllConnections()
  {
  List<MqttClient> connections;
  lock (_clients)
@@ -306,17 +306,17 @@ public List<MqttClient> GetClients()
  }
  }
 
- public Task<IList<MqttClientStatus>> GetClientStatusesAsync()
+ public Task<IList<MqttClientStatus>> GetClientsStatus()
  {
  var result = new List<MqttClientStatus>();
 
  lock (_clients)
  {
- foreach (var connection in _clients.Values)
+ foreach (var client in _clients.Values)
  {
- var clientStatus = new MqttClientStatus(connection)
+ var clientStatus = new MqttClientStatus(client)
  {
- Session = new MqttSessionStatus(connection.Session)
+ Session = new MqttSessionStatus(client.Session)
  };
 
  result.Add(clientStatus);
@@ -326,7 +326,7 @@ public Task<IList<MqttClientStatus>> GetClientStatusesAsync()
  return Task.FromResult((IList<MqttClientStatus>)result);
  }
 
- public Task<IList<MqttSessionStatus>> GetSessionStatusAsync()
+ public Task<IList<MqttSessionStatus>> GetSessionsStatus()
  {
  var result = new List<MqttSessionStatus>();
 
@@ -614,20 +614,15 @@ async Task<MqttClient> CreateClientConnection(
  // Create a new client (always required).
  lock (_clients)
  {
- _clients.TryGetValue(connectPacket.ClientId, out oldClient); 
- }
- 
- if (oldClient != null)
- {
- // This will stop the current client from sending and receiving but remains the connection active
- // for a later DISCONNECT packet.
- oldClient.IsTakenOver = true;
- }
-
- client = CreateClient(connectPacket, channelAdapter, session);
+ _clients.TryGetValue(connectPacket.ClientId, out oldClient);
+ if (oldClient != null)
+ {
+ // This will stop the current client from sending and receiving but remains the connection active
+ // for a later DISCONNECT packet.
+ oldClient.IsTakenOver = true;
+ }
 
- lock (_clients)
- {
+ client = CreateClient(connectPacket, channelAdapter, session);
  _clients[connectPacket.ClientId] = client;
  }
  }

Source/MQTTnet/Server/MqttServer.cs

@@ -200,7 +200,7 @@ public Task<IList<MqttClientStatus>> GetClientsAsync()
  {
  ThrowIfNotStarted();
 
- return _clientSessionsManager.GetClientStatusesAsync();
+ return _clientSessionsManager.GetClientsStatus();
  }
 
  public Task<IList<MqttApplicationMessage>> GetRetainedMessagesAsync()
@@ -226,7 +226,7 @@ public Task<IList<MqttSessionStatus>> GetSessionsAsync()
  {
  ThrowIfNotStarted();
 
- return _clientSessionsManager.GetSessionStatusAsync();
+ return _clientSessionsManager.GetSessionsStatus();
  }
 
  public Task InjectApplicationMessage(InjectedMqttApplicationMessage injectedApplicationMessage, CancellationToken cancellationToken = default)
@@ -292,7 +292,7 @@ public async Task StopAsync()
 
  _cancellationTokenSource.Cancel(false);
 
- await _clientSessionsManager.CloseAllConnectionsAsync().ConfigureAwait(false);
+ await _clientSessionsManager.CloseAllConnections().ConfigureAwait(false);
 
  foreach (var adapter in _adapters)
  {