From c48eb532f2baf666f0931607b61d14480791748a Mon Sep 17 00:00:00 2001 From: Stone_Red <56473591+Stone-Red-Code@users.noreply.github.com> Date: Sun, 2 Aug 2026 01:07:00 +0200 Subject: [PATCH] Fix memory leak caused by plugins not properly unloading --- src/DesktopMagic/Plugins/PluginWindow.xaml.cs | 233 +++++++++++++----- src/DesktopMagic/Plugins/SettingElement.cs | 2 +- .../Plugins/WebPluginWindow.xaml.cs | 74 +++++- 3 files changed, 230 insertions(+), 79 deletions(-) diff --git a/src/DesktopMagic/Plugins/PluginWindow.xaml.cs b/src/DesktopMagic/Plugins/PluginWindow.xaml.cs index 4376fae..531cfea 100644 --- a/src/DesktopMagic/Plugins/PluginWindow.xaml.cs +++ b/src/DesktopMagic/Plugins/PluginWindow.xaml.cs @@ -10,6 +10,8 @@ using SkiaSharp; using System; using System.Collections.Generic; +using System.Collections.Specialized; +using System.ComponentModel; using System.Drawing; using System.Drawing.Imaging; using System.IO; @@ -51,6 +53,14 @@ public partial class PluginWindow : Window, IPluginWindow private WriteableBitmap? writeableBitmap; private BitmapScalingMode lastBitmapScalingMode = BitmapScalingMode.Unspecified; + // Event handlers on long-lived settings objects, tracked so they can be unsubscribed on close. + private readonly PropertyChangedEventHandler settingsPropertyChangedHandler; + private readonly PropertyChangedEventHandler themePropertyChangedHandler; + private Theme? subscribedTheme; + private NotifyCollectionChangedEventHandler? themesCollectionChangedHandler; + private readonly List subscribedSettings = []; + private readonly List<(Setting Setting, Action Handler)> defaultSettingsSubscriptions = []; + public bool IsRunning { get; private set; } = true; public PluginMetadata PluginMetadata { get; private set; } public string PluginFolderPath { get; private set; } @@ -76,14 +86,11 @@ public partial class PluginWindow : Window, IPluginWindow Owner = w; - settings.PropertyChanged += (e, s) => + settingsPropertyChangedHandler = (_, s) => { if (s.PropertyName == nameof(PluginSettings.CurrentThemeName)) { - settings.Theme.PropertyChanged += (se, ev) => - { - ThemeChanged(); - }; + SubscribeToTheme(settings.Theme); ThemeChanged(); } else if (s.PropertyName == nameof(PluginSettings.Position)) @@ -95,11 +102,10 @@ public partial class PluginWindow : Window, IPluginWindow UpdateSize(); } }; + settings.PropertyChanged += settingsPropertyChangedHandler; - settings.Theme.PropertyChanged += (se, ev) => - { - ThemeChanged(); - }; + themePropertyChangedHandler = (_, _) => ThemeChanged(); + SubscribeToTheme(settings.Theme); PluginMetadata = pluginMetadata; this.settings = settings; @@ -129,6 +135,22 @@ public partial class PluginWindow : Window, IPluginWindow this.pluginClassInstance = pluginClassInstance; } + private void SubscribeToTheme(Theme theme) + { + if (ReferenceEquals(subscribedTheme, theme)) + { + return; + } + + if (subscribedTheme is not null) + { + subscribedTheme.PropertyChanged -= themePropertyChangedHandler; + } + + subscribedTheme = theme; + subscribedTheme.PropertyChanged += themePropertyChangedHandler; + } + private AssemblyLoadContext CreateAssemblyLoadContext() { AssemblyLoadContext context = new(PluginMetadata.Name, isCollectible: true); @@ -580,14 +602,18 @@ public partial class PluginWindow : Window, IPluginWindow SetThemeOverride(); SetThemeOverrideItems(); - pluginClassInstance.horizontalAlignment.OnValueChanged += SetHorizontalAlignment; - pluginClassInstance.verticalAlignment.OnValueChanged += SetVerticalAlignment; - pluginClassInstance.windowLayer.OnValueChanged += SetWindowLayer; - pluginClassInstance.rotation.OnValueChanged += SetRotation; - pluginClassInstance.themeOverride.OnValueChanged += SetThemeOverride; + SubscribeToDefaultSetting(pluginClassInstance.horizontalAlignment, SetHorizontalAlignment); + SubscribeToDefaultSetting(pluginClassInstance.verticalAlignment, SetVerticalAlignment); + SubscribeToDefaultSetting(pluginClassInstance.windowLayer, SetWindowLayer); + SubscribeToDefaultSetting(pluginClassInstance.rotation, SetRotation); + SubscribeToDefaultSetting(pluginClassInstance.themeOverride, SetThemeOverride); - DesktopMagicSettings desktopMagicSettings = MainWindowDataContext.GetSettings(); - desktopMagicSettings.Themes.CollectionChanged += (s, e) => SetThemeOverrideItems(); + if (themesCollectionChangedHandler is null) + { + themesCollectionChangedHandler = (_, _) => SetThemeOverrideItems(); + DesktopMagicSettings desktopMagicSettings = MainWindowDataContext.GetSettings(); + desktopMagicSettings.Themes.CollectionChanged += themesCollectionChangedHandler; + } }); void SetVerticalAlignment() @@ -654,6 +680,22 @@ public partial class PluginWindow : Window, IPluginWindow } } + private void SubscribeToDefaultSetting(Setting setting, Action handler) + { + setting.OnValueChanged += handler; + defaultSettingsSubscriptions.Add((setting, handler)); + } + + private void OnPluginSettingValueChanged() + { + pluginClassInstance?.OnSettingsChanged(); + + if (pluginClassInstance?.UpdateInterval is 0 or > 500) + { + pluginClassInstance.Application.UpdateWindow(); + } + } + private async Task LoadOptions(object instance) { App.Logger.LogInfo($"\"{PluginMetadata.Name}\" - Loading plugin options", source: "Plugin"); @@ -679,15 +721,8 @@ public partial class PluginWindow : Window, IPluginWindow settingElement.JsonValue = settingsSettingElement.JsonValue; } - element.OnValueChanged += () => - { - pluginClassInstance?.OnSettingsChanged(); - - if (pluginClassInstance?.UpdateInterval is 0 or > 500) - { - pluginClassInstance.Application.UpdateWindow(); - } - }; + element.OnValueChanged += OnPluginSettingValueChanged; + subscribedSettings.Add(element); settingElements.Add(settingElement); break; @@ -848,54 +883,73 @@ public partial class PluginWindow : Window, IPluginWindow private async void UpdateTimer_Elapsed(object? sender, ElapsedEventArgs? e) { + // Capture the fields into locals so an in-flight tick keeps the plugin assembly + // alive and stays safe when the window is being closed or the plugin reloaded. + Plugin? plugin = pluginClassInstance; + System.Timers.Timer? timer = updateTimer; + + if (!IsRunning || plugin is null) + { + return; + } + try { - if (IsRunning && pluginClassInstance is not null) + if (plugin is SkiaPlugin or SkiaAsyncPlugin) { - if (pluginClassInstance is SkiaPlugin or SkiaAsyncPlugin) + await RenderSkiaFrame(); + } + else + { + Bitmap? result; + + if (plugin is AsyncPlugin asyncPlugin) { - await RenderSkiaFrame(); + CancellationToken token = pluginCancellationTokenSource?.Token ?? CancellationToken.None; + result = await asyncPlugin.MainAsync(token); } else { - Bitmap? result; - - if (pluginClassInstance is AsyncPlugin asyncPlugin) - { - CancellationToken token = pluginCancellationTokenSource?.Token ?? CancellationToken.None; - result = await asyncPlugin.MainAsync(token); - } - else - { - result = pluginClassInstance.Main(); - } - - if (result is not null) - { - BitmapScalingMode renderOptions = pluginClassInstance.RenderQuality switch - { - RenderQuality.High => BitmapScalingMode.HighQuality, - RenderQuality.Low => BitmapScalingMode.LowQuality, - RenderQuality.Performance => BitmapScalingMode.NearestNeighbor, - _ => BitmapScalingMode.Unspecified - }; - - UpdateImageFromBitmap(result, renderOptions); - } + result = plugin.Main(); } - if (pluginClassInstance.UpdateInterval > 0) + if (result is not null) { - updateTimer!.Interval = pluginClassInstance.UpdateInterval; - } - else - { - updateTimer!.Stop(); + BitmapScalingMode renderOptions = plugin.RenderQuality switch + { + RenderQuality.High => BitmapScalingMode.HighQuality, + RenderQuality.Low => BitmapScalingMode.LowQuality, + RenderQuality.Performance => BitmapScalingMode.NearestNeighbor, + _ => BitmapScalingMode.Unspecified + }; + + UpdateImageFromBitmap(result, renderOptions); } } + + if (timer is null) + { + return; + } + + if (plugin.UpdateInterval > 0) + { + timer.Interval = plugin.UpdateInterval; + } + else + { + timer.Stop(); + } } catch (Exception ex) { + // If the window is closing, the plugin state is being torn down and the + // exception is a teardown race - ignore it instead of showing an error. + if (!IsRunning) + { + return; + } + IsRunning = false; App.Logger.LogError($"\"{PluginMetadata.Name}\" - {ex}", source: "Plugin"); _ = await Dispatcher.InvokeAsync(async () => @@ -909,17 +963,18 @@ public partial class PluginWindow : Window, IPluginWindow _ = await messageBox.ShowDialogAsync(); }); Exit(); - return; - } - - if (!IsRunning) - { - updateTimer!.Stop(); } } private async Task RenderSkiaFrame() { + Plugin? plugin = pluginClassInstance; + + if (plugin is null) + { + return; + } + int width = 0; int height = 0; @@ -932,12 +987,12 @@ public partial class PluginWindow : Window, IPluginWindow using SKSurface surface = SKSurface.Create(new SKImageInfo(width, height, SKColorType.Bgra8888, SKAlphaType.Premul)); surface.Canvas.Clear(SKColors.Transparent); - if (pluginClassInstance is SkiaAsyncPlugin skiaAsyncPlugin) + if (plugin is SkiaAsyncPlugin skiaAsyncPlugin) { CancellationToken token = pluginCancellationTokenSource?.Token ?? CancellationToken.None; await skiaAsyncPlugin.MainAsync(surface.Canvas, token); } - else if (pluginClassInstance is SkiaPlugin skiaPlugin) + else if (plugin is SkiaPlugin skiaPlugin) { skiaPlugin.Main(surface.Canvas); } @@ -1006,8 +1061,54 @@ public partial class PluginWindow : Window, IPluginWindow reloadDebounceTimer?.Dispose(); reloadDebounceTimer = null; + updateTimer?.Stop(); + updateTimer?.Dispose(); + updateTimer = null; + // Stop plugin using the shared method (no need to await in synchronous event handler) _ = StopPlugin(unloadAssembly: true); + + UnsubscribeEvents(); + DetachSettings(); + } + + private void DetachSettings() + { + foreach (SettingElement settingElement in settings.Settings) + { + string value = settingElement.JsonValue; + settingElement.Input = null; + settingElement.JsonValue = value; + } + } + + private void UnsubscribeEvents() + { + settings.PropertyChanged -= settingsPropertyChangedHandler; + + if (subscribedTheme is not null) + { + subscribedTheme.PropertyChanged -= themePropertyChangedHandler; + subscribedTheme = null; + } + + if (themesCollectionChangedHandler is not null) + { + MainWindowDataContext.GetSettings().Themes.CollectionChanged -= themesCollectionChangedHandler; + themesCollectionChangedHandler = null; + } + + foreach (Setting setting in subscribedSettings) + { + setting.OnValueChanged -= OnPluginSettingValueChanged; + } + subscribedSettings.Clear(); + + foreach ((Setting Setting, Action Handler) subscription in defaultSettingsSubscriptions) + { + subscription.Setting.OnValueChanged -= subscription.Handler; + } + defaultSettingsSubscriptions.Clear(); } private void UpdatePosition() diff --git a/src/DesktopMagic/Plugins/SettingElement.cs b/src/DesktopMagic/Plugins/SettingElement.cs index cbc45a6..5b2beba 100644 --- a/src/DesktopMagic/Plugins/SettingElement.cs +++ b/src/DesktopMagic/Plugins/SettingElement.cs @@ -9,7 +9,7 @@ public class SettingElement private string jsonValue = string.Empty; [JsonIgnore] - public Setting Input { get; set; } + public Setting? Input { get; set; } public string Name { get; set; } public int OrderIndex { get; set; } diff --git a/src/DesktopMagic/Plugins/WebPluginWindow.xaml.cs b/src/DesktopMagic/Plugins/WebPluginWindow.xaml.cs index fe2e56a..2d61343 100644 --- a/src/DesktopMagic/Plugins/WebPluginWindow.xaml.cs +++ b/src/DesktopMagic/Plugins/WebPluginWindow.xaml.cs @@ -7,6 +7,7 @@ using Microsoft.Web.WebView2.Core; using System; using System.Collections.Generic; +using System.ComponentModel; using System.IO; using System.Linq; using System.Text.Json; @@ -33,6 +34,13 @@ public partial class WebPluginWindow : Window, IPluginWindow private readonly string screenDeviceName; private bool isUpdatingPosition = false; + // Event handlers on long-lived settings objects, tracked so they can be unsubscribed on close. + private readonly PropertyChangedEventHandler settingsPropertyChangedHandler; + private readonly PropertyChangedEventHandler themePropertyChangedHandler; + private Theme? subscribedTheme; + private readonly List<(Setting Setting, Action Handler)> subscribedSettings = []; + private readonly List<(Button Button, Action Handler)> subscribedButtonClicks = []; + public bool IsRunning { get; private set; } = true; public PluginMetadata PluginMetadata { get; private set; } public string PluginFolderPath { get; private set; } @@ -58,14 +66,11 @@ public partial class WebPluginWindow : Window, IPluginWindow Owner = w; - settings.PropertyChanged += (e, s) => + settingsPropertyChangedHandler = (_, s) => { if (s.PropertyName == nameof(PluginSettings.CurrentThemeName)) { - settings.Theme.PropertyChanged += (se, ev) => - { - ThemeChanged(); - }; + SubscribeToTheme(settings.Theme); ThemeChanged(); } else if (s.PropertyName == nameof(PluginSettings.Position)) @@ -77,11 +82,10 @@ public partial class WebPluginWindow : Window, IPluginWindow UpdateSize(); } }; + settings.PropertyChanged += settingsPropertyChangedHandler; - settings.Theme.PropertyChanged += (se, ev) => - { - ThemeChanged(); - }; + themePropertyChangedHandler = (_, _) => ThemeChanged(); + SubscribeToTheme(settings.Theme); PluginMetadata = pluginMetadata; this.settings = settings; @@ -103,6 +107,22 @@ public partial class WebPluginWindow : Window, IPluginWindow } } + private void SubscribeToTheme(Theme theme) + { + if (ReferenceEquals(subscribedTheme, theme)) + { + return; + } + + if (subscribedTheme is not null) + { + subscribedTheme.PropertyChanged -= themePropertyChangedHandler; + } + + subscribedTheme = theme; + subscribedTheme.PropertyChanged += themePropertyChangedHandler; + } + public void Exit() { IsRunning = false; @@ -266,6 +286,9 @@ public partial class WebPluginWindow : Window, IPluginWindow reloadDebounceTimer?.Stop(); reloadDebounceTimer?.Dispose(); + reloadDebounceTimer = null; + + UnsubscribeEvents(); try { @@ -280,6 +303,29 @@ public partial class WebPluginWindow : Window, IPluginWindow } } + private void UnsubscribeEvents() + { + settings.PropertyChanged -= settingsPropertyChangedHandler; + + if (subscribedTheme is not null) + { + subscribedTheme.PropertyChanged -= themePropertyChangedHandler; + subscribedTheme = null; + } + + foreach ((Setting setting, Action handler) in subscribedSettings) + { + setting.OnValueChanged -= handler; + } + subscribedSettings.Clear(); + + foreach ((Button button, Action handler) in subscribedButtonClicks) + { + button.OnClick -= handler; + } + subscribedButtonClicks.Clear(); + } + private void UpdatePosition() { if (isUpdatingPosition) @@ -415,7 +461,7 @@ public partial class WebPluginWindow : Window, IPluginWindow string capturedId = id; if (setting is Button button) { - button.OnClick += () => + Action clickHandler = () => { _ = webView.Dispatcher.InvokeAsync(async () => { @@ -429,9 +475,11 @@ public partial class WebPluginWindow : Window, IPluginWindow } }); }; + button.OnClick += clickHandler; + subscribedButtonClicks.Add((button, clickHandler)); } - setting.OnValueChanged += () => + Action valueChangedHandler = () => { _ = webView.Dispatcher.InvokeAsync(async () => { @@ -445,6 +493,8 @@ public partial class WebPluginWindow : Window, IPluginWindow } }); }; + setting.OnValueChanged += valueChangedHandler; + subscribedSettings.Add((setting, valueChangedHandler)); settingElements.Add(settingElement); orderIndex++; @@ -614,7 +664,7 @@ public partial class WebPluginWindow : Window, IPluginWindow Dictionary dict = []; foreach (SettingElement element in settings.Settings) { - dict[element.Id] = GetSettingValue(element.Input); + dict[element.Id] = GetSettingValue(element.Input!); } return JsonSerializer.Serialize(dict); }