diff --git a/src/PepperDash.Essentials.MobileControl.Messengers/Messengers/MessengerBase.cs b/src/PepperDash.Essentials.MobileControl.Messengers/Messengers/MessengerBase.cs index a7210a13..6fc80adc 100644 --- a/src/PepperDash.Essentials.MobileControl.Messengers/Messengers/MessengerBase.cs +++ b/src/PepperDash.Essentials.MobileControl.Messengers/Messengers/MessengerBase.cs @@ -31,7 +31,12 @@ namespace PepperDash.Essentials.AppServer.Messengers /// /// Unsoliciited feedback from a device in a messenger will ONLY be sent to devices in this subscription list. When a client disconnects, it's ID will be removed from the collection. /// - protected HashSet SubscriberIds = new HashSet(); + private readonly HashSet subscriberIds = new HashSet(); + + /// + /// Lock object for thread-safe access to SubscriberIds + /// + private readonly object _subscriberLock = new object(); private readonly List _deviceInterfaces; @@ -193,14 +198,15 @@ namespace PepperDash.Essentials.AppServer.Messengers return; } - if (SubscriberIds.Any(id => id == clientId)) + lock (_subscriberLock) { - this.LogVerbose("Client {clientId} already subscribed", clientId); - return; + if (!subscriberIds.Add(clientId)) + { + this.LogVerbose("Client {clientId} already subscribed", clientId); + return; + } } - SubscriberIds.Add(clientId); - this.LogDebug("Client {clientId} subscribed", clientId); } @@ -216,14 +222,22 @@ namespace PepperDash.Essentials.AppServer.Messengers return; } - if (!SubscriberIds.Any(i => i == clientId)) + bool wasSubscribed; + lock (_subscriberLock) + { + wasSubscribed = subscriberIds.Contains(clientId); + if (wasSubscribed) + { + subscriberIds.Remove(clientId); + } + } + + if (!wasSubscribed) { this.LogVerbose("Client with ID {clientId} is not subscribed", clientId); return; } - SubscriberIds.RemoveWhere((i) => i == clientId); - this.LogInformation("Client with ID {clientId} unsubscribed", clientId); } @@ -312,7 +326,14 @@ namespace PepperDash.Essentials.AppServer.Messengers // If client is null or empty, this message is unsolicited feedback. Iterate through the subscriber list and send to all interested parties if (string.IsNullOrEmpty(clientId)) { - foreach (var client in SubscriberIds) + // Create a snapshot of subscribers to avoid collection modification during iteration + List subscriberSnapshot; + lock (_subscriberLock) + { + subscriberSnapshot = new List(subscriberIds); + } + + foreach (var client in subscriberSnapshot) { AppServerController?.SendMessageObject(new MobileControlMessage { Type = !string.IsNullOrEmpty(type) ? type : MessagePath, ClientId = client, Content = content }); }