From 53e7a30224a2748a322e92e65e9c343cfb3cb685 Mon Sep 17 00:00:00 2001 From: Andrew Welker Date: Fri, 26 Dec 2025 12:34:31 -0600 Subject: [PATCH] fix: handle threading issues for concurrent clients joining --- .../MobileControlSystemController.cs | 2 +- .../Utilities.cs | 6 +- .../MobileControlWebsocketServer.cs | 63 +++++++++++++++---- .../WebSocketServer/UiClient.cs | 5 ++ src/PepperDash.Essentials/ControlSystem.cs | 1 + 5 files changed, 60 insertions(+), 17 deletions(-) diff --git a/src/PepperDash.Essentials.MobileControl/MobileControlSystemController.cs b/src/PepperDash.Essentials.MobileControl/MobileControlSystemController.cs index 45ff814a..3adcaf87 100644 --- a/src/PepperDash.Essentials.MobileControl/MobileControlSystemController.cs +++ b/src/PepperDash.Essentials.MobileControl/MobileControlSystemController.cs @@ -1748,7 +1748,7 @@ namespace PepperDash.Essentials var clientNo = 1; foreach (var clientContext in _directServer.UiClientContexts) { - var clients = _directServer.UiClients.Values.Where(c => c.Token == clientContext.Value.Token.Token); + var clients = _directServer.UiClients.Values.Where(c => c.TokenKey == clientContext.Key); CrestronConsole.ConsoleCommandResponse( $"\r\nClient {clientNo}:\r\n" + diff --git a/src/PepperDash.Essentials.MobileControl/Utilities.cs b/src/PepperDash.Essentials.MobileControl/Utilities.cs index 8c2abf3e..ec9acd83 100644 --- a/src/PepperDash.Essentials.MobileControl/Utilities.cs +++ b/src/PepperDash.Essentials.MobileControl/Utilities.cs @@ -1,3 +1,4 @@ +using System.Threading; using PepperDash.Core; using PepperDash.Core.Logging; using WebSocketSharp; @@ -12,13 +13,12 @@ namespace PepperDash.Essentials private static int nextClientId = 0; /// - /// Get the next unique client ID + /// Get the next unique client ID (thread-safe) /// /// Client ID public static int GetNextClientId() { - nextClientId++; - return nextClientId; + return Interlocked.Increment(ref nextClientId); } /// /// Converts a WebSocketServer LogData object to Essentials logging calls. diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs index 0f2fa388..25cd7ba7 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/MobileControlWebsocketServer.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.ComponentModel; using System.IO; @@ -59,12 +60,18 @@ namespace PepperDash.Essentials.WebSocketServer /// public Dictionary UiClientContexts { get; private set; } - private readonly Dictionary uiClients = new Dictionary(); + private readonly ConcurrentDictionary uiClients = new ConcurrentDictionary(); + + /// + /// Stores pending client registrations using composite key: token-clientId + /// This ensures the correct client ID is matched even when connections establish out of order + /// + private readonly ConcurrentDictionary pendingClientRegistrations = new ConcurrentDictionary(); /// /// Gets the collection of UI clients /// - public ReadOnlyDictionary UiClients => new ReadOnlyDictionary(uiClients); + public IReadOnlyDictionary UiClients => uiClients; private readonly MobileControlSystemController _parent; @@ -723,20 +730,46 @@ namespace PepperDash.Essentials.WebSocketServer private UiClient BuildUiClient(string roomKey, JoinToken token, string key) { - var c = new UiClient($"uiclient-{key}-{roomKey}-{token.Id}", token.Id, token.Token, token.TouchpanelKey); - this.LogInformation("Constructing UiClient with key {key} and ID {id}", key, token.Id); + // Try to retrieve a pending client ID that was registered during the join request + // Use the composite key: token-clientId + string clientId = null; + string registrationKey = null; + + // Find a registration for this token + var matchingRegistrations = pendingClientRegistrations.Keys + .Where(k => k.StartsWith($"{key}-")) + .OrderBy(k => k) // Process in order for FIFO behavior + .ToList(); + + if (matchingRegistrations.Any()) + { + registrationKey = matchingRegistrations.First(); + if (pendingClientRegistrations.TryRemove(registrationKey, out clientId)) + { + this.LogVerbose("Retrieved pending clientId {clientId} for token {key}", clientId, key); + } + } + + if (clientId == null) + { + // Fallback: generate a new client ID if none was pending + clientId = $"{Utilities.GetNextClientId()}"; + this.LogWarning("No pending clientId found for token {key}, generated new ID {clientId}", key, clientId); + } + + var c = new UiClient($"uiclient-{key}-{roomKey}-{clientId}", clientId, token.Token, token.TouchpanelKey); + this.LogInformation("Constructing UiClient with key {key} and ID {id}", key, clientId); c.Controller = _parent; c.RoomKey = roomKey; + c.TokenKey = key; // Store the URL token key for filtering - if (uiClients.ContainsKey(token.Id)) + uiClients.AddOrUpdate(clientId, c, (id, existingClient) => { - this.LogWarning("removing client with duplicate id {id}", token.Id); - uiClients.Remove(token.Id); - } - uiClients.Add(token.Id, c); + this.LogWarning("replacing client with duplicate id {id}", id); + return c; + }); // UiClients[key].SetClient(c); - c.ConnectionClosed += (o, a) => uiClients.Remove(a.ClientId); - token.Id = null; + c.ConnectionClosed += (o, a) => uiClients.TryRemove(a.ClientId, out _); return c; } @@ -1046,10 +1079,14 @@ namespace PepperDash.Essentials.WebSocketServer }); } + // Generate a client ID for this join request and store it with a composite key var clientId = $"{Utilities.GetNextClientId()}"; - clientContext.Token.Id = clientId; + + // Store registration with composite key: token-clientId so BuildUiClient can verify it + var registrationKey = $"{token}-{clientId}"; + pendingClientRegistrations.TryAdd(registrationKey, clientId); - this.LogVerbose("Assigning ClientId: {clientId}", clientId); + this.LogVerbose("Assigning ClientId: {clientId} for token: {token}", clientId, token); // Construct the response object JoinResponse jRes = new JoinResponse diff --git a/src/PepperDash.Essentials.MobileControl/WebSocketServer/UiClient.cs b/src/PepperDash.Essentials.MobileControl/WebSocketServer/UiClient.cs index cf8dd7f9..61e5d042 100644 --- a/src/PepperDash.Essentials.MobileControl/WebSocketServer/UiClient.cs +++ b/src/PepperDash.Essentials.MobileControl/WebSocketServer/UiClient.cs @@ -31,6 +31,11 @@ namespace PepperDash.Essentials.WebSocketServer /// public string Token { get; private set; } + /// + /// The URL token key used to connect (from UiClientContexts dictionary key) + /// + public string TokenKey { get; set; } + /// /// Touchpanel Key associated with this client /// diff --git a/src/PepperDash.Essentials/ControlSystem.cs b/src/PepperDash.Essentials/ControlSystem.cs index 72d7c27d..ef19c955 100644 --- a/src/PepperDash.Essentials/ControlSystem.cs +++ b/src/PepperDash.Essentials/ControlSystem.cs @@ -662,6 +662,7 @@ namespace PepperDash.Essentials if (jsonFiles.Length > 1) { + Debug.LogError("Multiple configuration files found in application directory: {@jsonFiles}", jsonFiles.Select(f => f.FullName).ToArray()); throw new Exception("Multiple configuration files found. Cannot continue."); }