From ef31935b243776fd6234d1ab2846553acff46d66 Mon Sep 17 00:00:00 2001 From: IceStormNG Date: Thu, 27 Jul 2023 10:13:24 +0200 Subject: [PATCH 1/6] Improved handling of critical sections to solve deadlocks. --- app/Peripherals/PeripheralsProvider.cs | 136 +++++++++++++------------ 1 file changed, 73 insertions(+), 63 deletions(-) diff --git a/app/Peripherals/PeripheralsProvider.cs b/app/Peripherals/PeripheralsProvider.cs index 618bd1c8..f1926b00 100644 --- a/app/Peripherals/PeripheralsProvider.cs +++ b/app/Peripherals/PeripheralsProvider.cs @@ -1,12 +1,11 @@ using GHelper.Peripherals.Mouse; using GHelper.Peripherals.Mouse.Models; -using System.Runtime.CompilerServices; namespace GHelper.Peripherals { public class PeripheralsProvider { - public static object _LOCK = new object(); + private static readonly object _LOCK = new object(); public static List ConnectedMice = new List(); @@ -14,7 +13,15 @@ namespace GHelper.Peripherals public static bool IsMouseConnected() { - return ConnectedMice.Count > 0; + lock (_LOCK) + { + return ConnectedMice.Count > 0; + } + } + + public static bool IsDeviceConnected(IPeripheral peripheral) + { + return AllPeripherals().Contains(peripheral); } //Expand if keyboards or other device get supported later. @@ -26,31 +33,29 @@ namespace GHelper.Peripherals public static List AllPeripherals() { List l = new List(); - l.AddRange(ConnectedMice); - + lock (_LOCK) + { + l.AddRange(ConnectedMice); + } return l; } public static void RefreshBatteryForAllDevices() { - if (!IsAnyPeripheralConnect()) return; + List l = AllPeripherals(); - lock (_LOCK) + foreach (IPeripheral m in l) { - foreach (IPeripheral m in AllPeripherals()) + if (!m.IsDeviceReady) { - if (!m.IsDeviceReady) - { - //Try to sync the device if that hasn't been done yet - m.SynchronizeDevice(); - } - else - { - m.ReadBattery(); - } + //Try to sync the device if that hasn't been done yet + m.SynchronizeDevice(); + } + else + { + m.ReadBattery(); } } - } public static void Disconnect(AsusMouse am) @@ -58,54 +63,57 @@ namespace GHelper.Peripherals lock (_LOCK) { ConnectedMice.Remove(am); - if (DeviceChanged is not null) - { - DeviceChanged(am, EventArgs.Empty); - } + } + if (DeviceChanged is not null) + { + DeviceChanged(am, EventArgs.Empty); } } public static void Connect(AsusMouse am) { + + if (IsDeviceConnected(am)) + { + //Mouse already connected; + return; + } + + try + { + am.Connect(); + } + catch (IOException e) + { + Logger.WriteLine(am.GetDisplayName() + " failed to connect to device: " + e); + return; + } + + //The Mouse might needs a few ms to register all its subdevices or the sync will fail. + //Retry 3 times. Do not call this on main thread! It would block the UI + + int tries = 0; + while (!am.IsDeviceReady && tries < 3) + { + Thread.Sleep(250); + Logger.WriteLine(am.GetDisplayName() + " synchronising. Try " + (tries + 1)); + am.SynchronizeDevice(); + ++tries; + } + lock (_LOCK) { - if (ConnectedMice.Contains(am)) - { - //Mouse already connected; - return; - } - try - { - am.Connect(); - } - catch (IOException e) - { - Logger.WriteLine(am.GetDisplayName() + " failed to connect to device: " + e); - return; - } - - am.Disconnect += Mouse_Disconnect; - - //The Mouse might needs a few ms to register all its subdevices or the sync will fail. - //Retry 3 times. Do not call this on main thread! It would block the UI - - int tries = 0; - while (!am.IsDeviceReady && tries < 3) - { - Thread.Sleep(250); - Logger.WriteLine(am.GetDisplayName() + " synchronising. Try " + (tries + 1)); - am.SynchronizeDevice(); - ++tries; - } - ConnectedMice.Add(am); - Logger.WriteLine(am.GetDisplayName() + " added to the list: " + ConnectedMice.Count + " device are conneted."); - if (DeviceChanged is not null) - { - DeviceChanged(am, EventArgs.Empty); - } - UpdateSettingsView(); } + Logger.WriteLine(am.GetDisplayName() + " added to the list: " + ConnectedMice.Count + " device are conneted."); + + + am.Disconnect += Mouse_Disconnect; + if (DeviceChanged is not null) + { + DeviceChanged(am, EventArgs.Empty); + } + UpdateSettingsView(); } private static void Mouse_Disconnect(object? sender, EventArgs e) @@ -114,14 +122,17 @@ namespace GHelper.Peripherals { return; } + + AsusMouse am = (AsusMouse)sender; lock (_LOCK) { - AsusMouse am = (AsusMouse)sender; ConnectedMice.Remove(am); - Logger.WriteLine(am.GetDisplayName() + " reported disconnect. " + ConnectedMice.Count + " device are conneted."); - am.Dispose(); - UpdateSettingsView(); } + + Logger.WriteLine(am.GetDisplayName() + " reported disconnect. " + ConnectedMice.Count + " device are conneted."); + am.Dispose(); + + UpdateSettingsView(); } @@ -141,12 +152,11 @@ namespace GHelper.Peripherals DetectMouse(new ChakramXWired()); DetectMouse(new GladiusIII()); DetectMouse(new GladiusIIIWired()); - UpdateSettingsView(); } public static void DetectMouse(AsusMouse am) { - if (am.IsDeviceConnected() && !ConnectedMice.Contains(am)) + if (am.IsDeviceConnected() && !IsDeviceConnected(am)) { Logger.WriteLine("Detected a new" + am.GetDisplayName() + " . Connecting..."); Connect(am); From acaa8bc523eed0681390c7a0609db422170e19df Mon Sep 17 00:00:00 2001 From: IceStormNG Date: Thu, 27 Jul 2023 10:13:46 +0200 Subject: [PATCH 2/6] Added missing import for synchronized annotation --- app/Peripherals/PeripheralsProvider.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/app/Peripherals/PeripheralsProvider.cs b/app/Peripherals/PeripheralsProvider.cs index f1926b00..2673e57f 100644 --- a/app/Peripherals/PeripheralsProvider.cs +++ b/app/Peripherals/PeripheralsProvider.cs @@ -1,5 +1,6 @@ using GHelper.Peripherals.Mouse; using GHelper.Peripherals.Mouse.Models; +using System.Runtime.CompilerServices; namespace GHelper.Peripherals { From 6e4b5226f5133ca6e88b6156b8043871951a28fe Mon Sep 17 00:00:00 2001 From: IceStormNG Date: Thu, 27 Jul 2023 10:14:06 +0200 Subject: [PATCH 3/6] Do not refresh battery on main thread. --- app/Settings.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Settings.cs b/app/Settings.cs index 31ec67c7..2ffd86be 100644 --- a/app/Settings.cs +++ b/app/Settings.cs @@ -236,7 +236,7 @@ namespace GHelper { screenControl.InitScreen(); gpuControl.InitXGM(); - PeripheralsProvider.RefreshBatteryForAllDevices(); + Task.Run((Action)PeripheralsProvider.RefreshBatteryForAllDevices); updateControl.CheckForUpdates(); } } From a323bd85ab877437930b5e3e9feef8465d2f35a5 Mon Sep 17 00:00:00 2001 From: IceStormNG Date: Thu, 27 Jul 2023 10:19:17 +0200 Subject: [PATCH 4/6] Mouse refreshes battery again during "ReadSensors". But it does that nonblocking and event driven. --- app/Peripherals/PeripheralsProvider.cs | 15 +++++++++++++++ app/Settings.cs | 1 + 2 files changed, 16 insertions(+) diff --git a/app/Peripherals/PeripheralsProvider.cs b/app/Peripherals/PeripheralsProvider.cs index 2673e57f..23b91e16 100644 --- a/app/Peripherals/PeripheralsProvider.cs +++ b/app/Peripherals/PeripheralsProvider.cs @@ -63,6 +63,9 @@ namespace GHelper.Peripherals { lock (_LOCK) { + am.Disconnect -= Mouse_Disconnect; + am.MouseReadyChanged -= MouseReadyChanged; + am.BatteryUpdated -= BatteryUpdated; ConnectedMice.Remove(am); } if (DeviceChanged is not null) @@ -110,6 +113,8 @@ namespace GHelper.Peripherals am.Disconnect += Mouse_Disconnect; + am.MouseReadyChanged += MouseReadyChanged; + am.BatteryUpdated += BatteryUpdated; if (DeviceChanged is not null) { DeviceChanged(am, EventArgs.Empty); @@ -117,6 +122,16 @@ namespace GHelper.Peripherals UpdateSettingsView(); } + private static void BatteryUpdated(object? sender, EventArgs e) + { + UpdateSettingsView(); + } + + private static void MouseReadyChanged(object? sender, EventArgs e) + { + UpdateSettingsView(); + } + private static void Mouse_Disconnect(object? sender, EventArgs e) { if (sender is null) diff --git a/app/Settings.cs b/app/Settings.cs index 2ffd86be..bf05341c 100644 --- a/app/Settings.cs +++ b/app/Settings.cs @@ -814,6 +814,7 @@ namespace GHelper string battery = ""; HardwareControl.ReadSensors(); + Task.Run((Action)PeripheralsProvider.RefreshBatteryForAllDevices); if (HardwareControl.cpuTemp > 0) cpuTemp = ": " + Math.Round((decimal)HardwareControl.cpuTemp).ToString() + "°C"; From 7c42f8775128a70baf5b36f1a72b36569ff7d1bb Mon Sep 17 00:00:00 2001 From: IceStormNG Date: Thu, 27 Jul 2023 10:40:41 +0200 Subject: [PATCH 5/6] Check for dispose before trying to Invoke. --- app/AsusMouseSettings.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/AsusMouseSettings.cs b/app/AsusMouseSettings.cs index d3c3b069..8454008e 100644 --- a/app/AsusMouseSettings.cs +++ b/app/AsusMouseSettings.cs @@ -94,14 +94,14 @@ namespace GHelper private void Mouse_MouseReadyChanged(object? sender, EventArgs e) { + if (Disposing || IsDisposed) + { + return; + } if (!mouse.IsDeviceReady) { this.Invoke(delegate { - if (Disposing || IsDisposed) - { - return; - } Close(); }); } @@ -109,12 +109,12 @@ namespace GHelper private void Mouse_BatteryUpdated(object? sender, EventArgs e) { + if (Disposing || IsDisposed) + { + return; + } this.Invoke(delegate { - if (Disposing || IsDisposed) - { - return; - } VisualizeBatteryState(); }); From 42a346b19eaf3ab06c0c5a27db2e94f6389f4609 Mon Sep 17 00:00:00 2001 From: IceStormNG Date: Thu, 27 Jul 2023 10:41:23 +0200 Subject: [PATCH 6/6] Wait for mouse data before opening the window so the user does not see the form populating data. --- app/AsusMouseSettings.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/AsusMouseSettings.cs b/app/AsusMouseSettings.cs index 8454008e..8bf30f6a 100644 --- a/app/AsusMouseSettings.cs +++ b/app/AsusMouseSettings.cs @@ -82,7 +82,7 @@ namespace GHelper InitMouseCapabilities(); Logger.WriteLine(mouse.GetDisplayName() + " (GUI): Initialized capabilities. Synchronizing mouse data"); - Task task = Task.Run((Action)RefreshMouseData); + RefreshMouseData(); } private void AsusMouseSettings_FormClosing(object? sender, FormClosingEventArgs e)