diff --git a/.github/workflows/ReleaseNotes.md b/.github/workflows/ReleaseNotes.md index 13664446c..b5b84be15 100644 --- a/.github/workflows/ReleaseNotes.md +++ b/.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). diff --git a/Source/MQTTnet/Server/Internal/MqttClientSessionsManager.cs b/Source/MQTTnet/Server/Internal/MqttClientSessionsManager.cs index 0c16fd4b4..c0d385d63 100644 --- a/Source/MQTTnet/Server/Internal/MqttClientSessionsManager.cs +++ b/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 connections; lock (_clients) @@ -306,17 +306,17 @@ public List GetClients() } } - public Task> GetClientStatusesAsync() + public Task> GetClientsStatus() { var result = new List(); 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> GetClientStatusesAsync() return Task.FromResult((IList)result); } - public Task> GetSessionStatusAsync() + public Task> GetSessionsStatus() { var result = new List(); @@ -614,20 +614,15 @@ async Task 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; } } diff --git a/Source/MQTTnet/Server/MqttServer.cs b/Source/MQTTnet/Server/MqttServer.cs index daed20556..ff6c806e0 100644 --- a/Source/MQTTnet/Server/MqttServer.cs +++ b/Source/MQTTnet/Server/MqttServer.cs @@ -200,7 +200,7 @@ public Task> GetClientsAsync() { ThrowIfNotStarted(); - return _clientSessionsManager.GetClientStatusesAsync(); + return _clientSessionsManager.GetClientsStatus(); } public Task> GetRetainedMessagesAsync() @@ -226,7 +226,7 @@ public Task> 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) {