From ca453f205245ed6c65792efb975b22081e47cc62 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 23 Aug 2021 20:41:07 +0200 Subject: [PATCH] =?UTF-8?q?Core=20-=20Kill=20zombies=20after=202=20seconds?= =?UTF-8?q?=20=F0=9F=94=AB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Core - Fix an access violation on surface changes (when loading device providers/moving devices) --- src/Artemis.Core/Services/CoreService.cs | 4 +- .../Services/Interfaces/IRgbService.cs | 11 +- src/Artemis.Core/Services/RgbService.cs | 194 ++++++++++-------- src/Artemis.UI/ApplicationStateManager.cs | 5 +- .../Steps/LayoutStepViewModel.cs | 4 +- .../SurfaceEditor/SurfaceEditorViewModel.cs | 4 +- 6 files changed, 127 insertions(+), 95 deletions(-) diff --git a/src/Artemis.Core/Services/CoreService.cs b/src/Artemis.Core/Services/CoreService.cs index 9dee9fedb..8f9bfdbd5 100644 --- a/src/Artemis.Core/Services/CoreService.cs +++ b/src/Artemis.Core/Services/CoreService.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; using System.Reflection; +using System.Threading; using System.Threading.Tasks; using Artemis.Core.Ninject; using Artemis.Core.ScriptingProviders; @@ -58,7 +59,6 @@ namespace Artemis.Core.Services _frameStopWatch = new Stopwatch(); StartupArguments = new List(); - _rgbService.IsRenderPaused = true; _rgbService.Surface.Updating += SurfaceOnUpdating; _loggingLevel.SettingChanged += (sender, args) => ApplyLoggingLevel(); } @@ -222,7 +222,7 @@ namespace Artemis.Core.Services _pluginManagementService.CopyBuiltInPlugins(); _pluginManagementService.LoadPlugins(StartupArguments, IsElevated); - _rgbService.IsRenderPaused = false; + _rgbService.SetRenderPaused(false); OnInitialized(); } diff --git a/src/Artemis.Core/Services/Interfaces/IRgbService.cs b/src/Artemis.Core/Services/Interfaces/IRgbService.cs index 5420edb8f..c9bed101a 100644 --- a/src/Artemis.Core/Services/Interfaces/IRgbService.cs +++ b/src/Artemis.Core/Services/Interfaces/IRgbService.cs @@ -34,13 +34,13 @@ namespace Artemis.Core.Services /// /// Gets or sets whether rendering should be paused /// - bool IsRenderPaused { get; set; } + bool IsRenderPaused { get; } /// /// Gets a boolean indicating whether the render pipeline is open /// bool RenderOpen { get; } - + /// /// Gets or sets a boolean indicating whether to flush the RGB.NET LEDs during next update /// @@ -138,6 +138,13 @@ namespace Artemis.Core.Services /// The device to disable void DisableDevice(ArtemisDevice device); + /// + /// Pauses or resumes rendering, method won't return until the current frame finished rendering + /// + /// + /// if the pause state was changed; otherwise . + bool SetRenderPaused(bool paused); + /// /// Occurs when a single device was added /// diff --git a/src/Artemis.Core/Services/RgbService.cs b/src/Artemis.Core/Services/RgbService.cs index da343b558..1922b4c64 100644 --- a/src/Artemis.Core/Services/RgbService.cs +++ b/src/Artemis.Core/Services/RgbService.cs @@ -27,7 +27,6 @@ namespace Artemis.Core.Services private readonly PluginSetting _targetFrameRateSetting; private readonly TextureBrush _textureBrush = new(ITexture.Empty) {CalculationMode = RenderMode.Absolute}; private Dictionary _ledMap; - private bool _modifyingProviders; private ListLedGroup? _surfaceLedGroup; private SKTexture? _texture; @@ -51,6 +50,7 @@ namespace Artemis.Core.Services _ledMap = new Dictionary(); UpdateTrigger = new TimerUpdateTrigger {UpdateFrequency = 1.0 / _targetFrameRateSetting.Value}; + SetRenderPaused(true); Surface.RegisterUpdateTrigger(UpdateTrigger); Utilities.ShutdownRequested += UtilitiesOnShutdownRequested; @@ -81,16 +81,14 @@ namespace Artemis.Core.Services private void UpdateLedGroup() { - lock (_devices) + bool changedRenderPaused = SetRenderPaused(true); + try { - if (_modifyingProviders) - return; - _ledMap = new Dictionary(_devices.SelectMany(d => d.Leds).ToDictionary(l => l.RgbLed)); if (_surfaceLedGroup == null) { - _surfaceLedGroup = new ListLedGroup(Surface, LedMap.Select(l => l.Key)) {Brush = _textureBrush}; + _surfaceLedGroup = new ListLedGroup(Surface, LedMap.Select(l => l.Key)) { Brush = _textureBrush }; OnLedsChanged(); return; } @@ -101,10 +99,16 @@ namespace Artemis.Core.Services _surfaceLedGroup.Detach(); // Apply the application wide brush and decorator - _surfaceLedGroup = new ListLedGroup(Surface, LedMap.Select(l => l.Key)) {Brush = _textureBrush}; + _surfaceLedGroup = new ListLedGroup(Surface, LedMap.Select(l => l.Key)) { Brush = _textureBrush }; OnLedsChanged(); } } + finally + { + if (changedRenderPaused) + SetRenderPaused(false); + } + } private void TargetFrameRateSettingOnSettingChanged(object? sender, EventArgs e) @@ -141,95 +145,85 @@ namespace Artemis.Core.Services public void AddDeviceProvider(IRGBDeviceProvider deviceProvider) { - if (RenderOpen) - throw new ArtemisCoreException("Cannot add a device provider while rendering"); + bool changedRenderPaused = SetRenderPaused(true); - lock (_devices) + try { - try + List toRemove = _devices.Where(a => deviceProvider.Devices.Any(d => a.RgbDevice == d)).ToList(); + Surface.Detach(toRemove.Select(d => d.RgbDevice)); + foreach (ArtemisDevice device in toRemove) + RemoveDevice(device); + + List providerExceptions = new(); + + void DeviceProviderOnException(object? sender, ExceptionEventArgs e) { - _modifyingProviders = true; - - List toRemove = _devices.Where(a => deviceProvider.Devices.Any(d => a.RgbDevice == d)).ToList(); - Surface.Detach(toRemove.Select(d => d.RgbDevice)); - foreach (ArtemisDevice device in toRemove) - RemoveDevice(device); - - List providerExceptions = new(); - - void DeviceProviderOnException(object? sender, ExceptionEventArgs e) - { - if (e.IsCritical) - providerExceptions.Add(e.Exception); - else - _logger.Warning(e.Exception, "Device provider {deviceProvider} threw non-critical exception", deviceProvider.GetType().Name); - } - - deviceProvider.Exception += DeviceProviderOnException; - deviceProvider.Initialize(); - Surface.Attach(deviceProvider.Devices); - deviceProvider.Exception -= DeviceProviderOnException; - if (providerExceptions.Count == 1) - throw new ArtemisPluginException("RGB.NET threw exception: " + providerExceptions.First().Message, providerExceptions.First()); - if (providerExceptions.Count > 1) - throw new ArtemisPluginException("RGB.NET threw multiple exceptions", new AggregateException(providerExceptions)); - - if (!deviceProvider.Devices.Any()) - { - _logger.Warning("Device provider {deviceProvider} has no devices", deviceProvider.GetType().Name); - return; - } - - foreach (IRGBDevice rgbDevice in deviceProvider.Devices) - { - ArtemisDevice artemisDevice = GetArtemisDevice(rgbDevice); - AddDevice(artemisDevice); - _logger.Debug("Device provider {deviceProvider} added {deviceName}", deviceProvider.GetType().Name, rgbDevice.DeviceInfo.DeviceName); - } - - _devices.Sort((a, b) => a.ZIndex - b.ZIndex); + if (e.IsCritical) + providerExceptions.Add(e.Exception); + else + _logger.Warning(e.Exception, "Device provider {deviceProvider} threw non-critical exception", deviceProvider.GetType().Name); } - catch (Exception e) + + deviceProvider.Exception += DeviceProviderOnException; + deviceProvider.Initialize(); + Surface.Attach(deviceProvider.Devices); + deviceProvider.Exception -= DeviceProviderOnException; + if (providerExceptions.Count == 1) + throw new ArtemisPluginException("RGB.NET threw exception: " + providerExceptions.First().Message, providerExceptions.First()); + if (providerExceptions.Count > 1) + throw new ArtemisPluginException("RGB.NET threw multiple exceptions", new AggregateException(providerExceptions)); + + if (!deviceProvider.Devices.Any()) { - _logger.Error(e, "Exception during device loading for device provider {deviceProvider}", deviceProvider.GetType().Name); - throw; + _logger.Warning("Device provider {deviceProvider} has no devices", deviceProvider.GetType().Name); + return; } - finally + + foreach (IRGBDevice rgbDevice in deviceProvider.Devices) { - _modifyingProviders = false; - UpdateLedGroup(); + ArtemisDevice artemisDevice = GetArtemisDevice(rgbDevice); + AddDevice(artemisDevice); + _logger.Debug("Device provider {deviceProvider} added {deviceName}", deviceProvider.GetType().Name, rgbDevice.DeviceInfo.DeviceName); } + + _devices.Sort((a, b) => a.ZIndex - b.ZIndex); + } + catch (Exception e) + { + _logger.Error(e, "Exception during device loading for device provider {deviceProvider}", deviceProvider.GetType().Name); + throw; + } + finally + { + UpdateLedGroup(); + if (changedRenderPaused) + SetRenderPaused(false); } } public void RemoveDeviceProvider(IRGBDeviceProvider deviceProvider) { - if (RenderOpen) - throw new ArtemisCoreException("Cannot update the remove device provider while rendering"); + bool changedRenderPaused = SetRenderPaused(true); - lock (_devices) + try { - try - { - _modifyingProviders = true; + List toRemove = _devices.Where(a => deviceProvider.Devices.Any(d => a.RgbDevice == d)).ToList(); + Surface.Detach(toRemove.Select(d => d.RgbDevice)); + foreach (ArtemisDevice device in toRemove) + RemoveDevice(device); - List toRemove = _devices.Where(a => deviceProvider.Devices.Any(d => a.RgbDevice == d)).ToList(); - Surface.Detach(toRemove.Select(d => d.RgbDevice)); - foreach (ArtemisDevice device in toRemove) - RemoveDevice(device); - - _devices.Sort((a, b) => a.ZIndex - b.ZIndex); - } - catch (Exception e) - { - _logger.Error(e, "Exception during device removal for device provider {deviceProvider}", deviceProvider.GetType().Name); - throw; - } - finally - { - _modifyingProviders = false; - UpdateLedGroup(); - } + _devices.Sort((a, b) => a.ZIndex - b.ZIndex); + } + catch (Exception e) + { + _logger.Error(e, "Exception during device removal for device provider {deviceProvider}", deviceProvider.GetType().Name); + throw; + } + finally + { + UpdateLedGroup(); + if (changedRenderPaused) + SetRenderPaused(false); } } @@ -241,6 +235,24 @@ namespace Artemis.Core.Services Surface.Dispose(); } + public bool SetRenderPaused(bool paused) + { + if (IsRenderPaused == paused) + return false; + + if (paused) + { + UpdateTrigger.Stop(); + } + else + { + UpdateTrigger.Start(); + } + + IsRenderPaused = paused; + return true; + } + public event EventHandler? DeviceAdded; public event EventHandler? DeviceRemoved; public event EventHandler? LedsChanged; @@ -326,12 +338,22 @@ namespace Artemis.Core.Services public void AutoArrangeDevices() { - SurfaceArrangement surfaceArrangement = SurfaceArrangement.GetDefaultArrangement(); - surfaceArrangement.Arrange(_devices); - foreach (ArtemisDevice artemisDevice in _devices) - artemisDevice.ApplyDefaultCategories(); + bool changedRenderPaused = SetRenderPaused(true); - SaveDevices(); + try + { + SurfaceArrangement surfaceArrangement = SurfaceArrangement.GetDefaultArrangement(); + surfaceArrangement.Arrange(_devices); + foreach (ArtemisDevice artemisDevice in _devices) + artemisDevice.ApplyDefaultCategories(); + + SaveDevices(); + } + finally + { + if (changedRenderPaused) + SetRenderPaused(false); + } } public ArtemisLayout? ApplyBestDeviceLayout(ArtemisDevice device) @@ -402,7 +424,7 @@ namespace Artemis.Core.Services _deviceRepository.Save(device.DeviceEntity); DeviceProvider deviceProvider = device.DeviceProvider; - + // Feels bad but need to in order to get the initial LEDs back _pluginManagementService.DisablePluginFeature(deviceProvider, false); Thread.Sleep(500); diff --git a/src/Artemis.UI/ApplicationStateManager.cs b/src/Artemis.UI/ApplicationStateManager.cs index 45dc2098c..d2ac6eb02 100644 --- a/src/Artemis.UI/ApplicationStateManager.cs +++ b/src/Artemis.UI/ApplicationStateManager.cs @@ -92,7 +92,10 @@ namespace Artemis.UI using HttpClient client = new(); try { - HttpResponseMessage httpResponseMessage = client.Send(new HttpRequestMessage(HttpMethod.Post, url + "remote/bring-to-foreground")); + CancellationTokenSource cts = new(); + cts.CancelAfter(2000); + + HttpResponseMessage httpResponseMessage = client.Send(new HttpRequestMessage(HttpMethod.Post, url + "remote/bring-to-foreground"), cts.Token); httpResponseMessage.EnsureSuccessStatusCode(); return true; } diff --git a/src/Artemis.UI/Screens/StartupWizard/Steps/LayoutStepViewModel.cs b/src/Artemis.UI/Screens/StartupWizard/Steps/LayoutStepViewModel.cs index 701451d48..9b60880ba 100644 --- a/src/Artemis.UI/Screens/StartupWizard/Steps/LayoutStepViewModel.cs +++ b/src/Artemis.UI/Screens/StartupWizard/Steps/LayoutStepViewModel.cs @@ -31,14 +31,14 @@ namespace Artemis.UI.Screens.StartupWizard.Steps /// protected override void OnActivate() { - _rgbService.IsRenderPaused = true; + _rgbService.SetRenderPaused(true); base.OnActivate(); } /// protected override void OnDeactivate() { - _rgbService.IsRenderPaused = false; + _rgbService.SetRenderPaused(false); base.OnDeactivate(); } diff --git a/src/Artemis.UI/Screens/SurfaceEditor/SurfaceEditorViewModel.cs b/src/Artemis.UI/Screens/SurfaceEditor/SurfaceEditorViewModel.cs index 8564c645b..8bbcdfcf8 100644 --- a/src/Artemis.UI/Screens/SurfaceEditor/SurfaceEditorViewModel.cs +++ b/src/Artemis.UI/Screens/SurfaceEditor/SurfaceEditorViewModel.cs @@ -333,7 +333,7 @@ namespace Artemis.UI.Screens.SurfaceEditor SurfaceDeviceViewModel device = HitTestUtilities.GetHitViewModels((Visual) sender, selectedRect).FirstOrDefault(); if (device != null) { - _rgbService.IsRenderPaused = true; + _rgbService.SetRenderPaused(true); _mouseDragStatus = MouseDragStatus.Dragging; // If the device is not selected, deselect others and select only this one (if shift not held) if (device.SelectionStatus != SelectionStatus.Selected) @@ -378,7 +378,7 @@ namespace Artemis.UI.Screens.SurfaceEditor } _mouseDragStatus = MouseDragStatus.None; - _rgbService.IsRenderPaused = false; + _rgbService.SetRenderPaused(false); ApplySurfaceSelection(); }