From 7041139ee0556c474dad7dc599c0b69bcdf5d1b7 Mon Sep 17 00:00:00 2001 From: Giuseppe Lippolis Date: Mon, 2 Sep 2024 18:09:40 +0200 Subject: [PATCH 1/3] fix: PopupImpl memory leak --- src/Avalonia.Controls/TopLevel.cs | 20 +++++++++------- src/Avalonia.Controls/Window.cs | 24 ++++++++++++------- src/Avalonia.Controls/WindowBase.cs | 2 +- .../Avalonia.Win32/Input/WindowsInputPane.cs | 3 ++- src/Windows/Avalonia.Win32/WindowImpl.cs | 7 +++--- 5 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/Avalonia.Controls/TopLevel.cs b/src/Avalonia.Controls/TopLevel.cs index b4b76085d9e..724ff9e6f6c 100644 --- a/src/Avalonia.Controls/TopLevel.cs +++ b/src/Avalonia.Controls/TopLevel.cs @@ -23,7 +23,6 @@ using Avalonia.Input.Platform; using System.Linq; using System.Threading.Tasks; -using Avalonia.Metadata; using Avalonia.Rendering.Composition; using Avalonia.Threading; @@ -220,7 +219,7 @@ public TopLevel(ITopLevelImpl impl, IAvaloniaDependencyResolver? dependencyResol _globalStyles = TryGetService(dependencyResolver); _applicationThemeHost = TryGetService(dependencyResolver); - Renderer = new CompositingRenderer(this, impl.Compositor, () => impl.Surfaces); + Renderer = new CompositingRenderer(this, impl.Compositor, GetSurfaces); Renderer.SceneInvalidated += SceneInvalidated; impl.SetInputRoot(this); @@ -232,11 +231,11 @@ public TopLevel(ITopLevelImpl impl, IAvaloniaDependencyResolver? dependencyResol impl.ScalingChanged = HandleScalingChanged; impl.TransparencyLevelChanged = HandleTransparencyLevelChanged; - CreatePlatformImplBinding(TransparencyLevelHintProperty, hint => PlatformImpl.SetTransparencyLevelHint(hint ?? Array.Empty())); - CreatePlatformImplBinding(ActualThemeVariantProperty, variant => + CreatePlatformImplBinding(TransparencyLevelHintProperty, (plat, hint) => plat.SetTransparencyLevelHint(hint ?? [])); + CreatePlatformImplBinding(ActualThemeVariantProperty, (plat, variant) => { variant ??= ThemeVariant.Default; - PlatformImpl?.SetFrameThemeVariant((PlatformThemeVariant?)variant ?? PlatformThemeVariant.Light); + plat.SetFrameThemeVariant((PlatformThemeVariant?)variant ?? PlatformThemeVariant.Light); }); _keyboardNavigationHandler?.SetOwner(this); @@ -431,7 +430,7 @@ internal ILayoutManager LayoutManager /// public IPlatformHandle? TryGetPlatformHandle() => PlatformImpl?.Handle; - private protected void CreatePlatformImplBinding(StyledProperty property, Action onValue) + private protected void CreatePlatformImplBinding(StyledProperty property, Action onValue) { _platformImplBindings.TryGetValue(property, out var actions); _platformImplBindings[property] = actions + UpdatePlatformImpl; @@ -440,9 +439,9 @@ private protected void CreatePlatformImplBinding(StyledProperty void UpdatePlatformImpl() { - if (PlatformImpl is not null) + if (PlatformImpl is { } platformImpl) { - onValue(GetValue(property)); + onValue(platformImpl, GetValue(property)); } } } @@ -974,5 +973,10 @@ public void Dispose() _layoutManager.LayoutPassTimed = null; } } + + private IEnumerable GetSurfaces() + { + return PlatformImpl?.Surfaces ?? []; + } } } diff --git a/src/Avalonia.Controls/Window.cs b/src/Avalonia.Controls/Window.cs index bbaed03dd47..12921e934ab 100644 --- a/src/Avalonia.Controls/Window.cs +++ b/src/Avalonia.Controls/Window.cs @@ -230,22 +230,28 @@ public Window(IWindowImpl impl) impl.ExtendClientAreaToDecorationsChanged = ExtendClientAreaToDecorationsChanged; this.GetObservable(ClientSizeProperty).Skip(1).Subscribe(x => PlatformImpl?.Resize(x, WindowResizeReason.Application)); - CreatePlatformImplBinding(TitleProperty, title => PlatformImpl!.SetTitle(title)); - CreatePlatformImplBinding(IconProperty, icon => PlatformImpl!.SetIcon((icon ?? s_defaultIcon.Value)?.PlatformImpl)); - CreatePlatformImplBinding(CanResizeProperty, canResize => PlatformImpl!.CanResize(canResize)); - CreatePlatformImplBinding(ShowInTaskbarProperty, show => PlatformImpl!.ShowTaskbarIcon(show)); + CreatePlatformImplBinding(TitleProperty,(plat, title) => (plat as IWindowImpl)?.SetTitle(title)); + CreatePlatformImplBinding(IconProperty, (plat, icon) => (plat as IWindowImpl)?.SetIcon((icon ?? s_defaultIcon.Value)?.PlatformImpl)); + CreatePlatformImplBinding(CanResizeProperty,(plat, canResize) => (plat as IWindowImpl)?.CanResize(canResize)); + CreatePlatformImplBinding(ShowInTaskbarProperty, (plat, show) => (plat as IWindowImpl)?.ShowTaskbarIcon(show)); - CreatePlatformImplBinding(WindowStateProperty, state => PlatformImpl!.WindowState = state); - CreatePlatformImplBinding(ExtendClientAreaToDecorationsHintProperty, hint => PlatformImpl!.SetExtendClientAreaToDecorationsHint(hint)); - CreatePlatformImplBinding(ExtendClientAreaChromeHintsProperty, hint => PlatformImpl!.SetExtendClientAreaChromeHints(hint)); - CreatePlatformImplBinding(ExtendClientAreaTitleBarHeightHintProperty, height => PlatformImpl!.SetExtendClientAreaTitleBarHeightHint(height)); + CreatePlatformImplBinding(WindowStateProperty, (plat, state) => + { + if (plat is IWindowImpl impl1) + { + impl1.WindowState = state; + } + }); + CreatePlatformImplBinding(ExtendClientAreaToDecorationsHintProperty, (plat, hint) => (plat as IWindowImpl)?.SetExtendClientAreaToDecorationsHint(hint)); + CreatePlatformImplBinding(ExtendClientAreaChromeHintsProperty, (plat, hint) => (plat as IWindowImpl)?.SetExtendClientAreaChromeHints(hint)); + CreatePlatformImplBinding(ExtendClientAreaTitleBarHeightHintProperty, (plat, height) => (plat as IWindowImpl)?.SetExtendClientAreaTitleBarHeightHint(height)); CreatePlatformImplBinding(MinWidthProperty, UpdateMinMaxSize); CreatePlatformImplBinding(MaxWidthProperty, UpdateMinMaxSize); CreatePlatformImplBinding(MinHeightProperty, UpdateMinMaxSize); CreatePlatformImplBinding(MaxHeightProperty, UpdateMinMaxSize); - void UpdateMinMaxSize(double _) => PlatformImpl!.SetMinMaxSize(new Size(MinWidth, MinHeight), new Size(MaxWidth, MaxHeight)); + void UpdateMinMaxSize(ITopLevelImpl plat, double _) => (plat as IWindowImpl)?.SetMinMaxSize(new Size(MinWidth, MinHeight), new Size(MaxWidth, MaxHeight)); } /// diff --git a/src/Avalonia.Controls/WindowBase.cs b/src/Avalonia.Controls/WindowBase.cs index 055979efb4c..aa668393f51 100644 --- a/src/Avalonia.Controls/WindowBase.cs +++ b/src/Avalonia.Controls/WindowBase.cs @@ -47,7 +47,7 @@ static WindowBase() public WindowBase(IWindowBaseImpl impl) : this(impl, AvaloniaLocator.Current) { - CreatePlatformImplBinding(TopmostProperty, topmost => PlatformImpl!.SetTopmost(topmost)); + CreatePlatformImplBinding(TopmostProperty, (plat, topmost) => (plat as IWindowImpl)?.SetTopmost(topmost)); FrameSize = impl.FrameSize; } diff --git a/src/Windows/Avalonia.Win32/Input/WindowsInputPane.cs b/src/Windows/Avalonia.Win32/Input/WindowsInputPane.cs index 73195ad500d..2e65831ace9 100644 --- a/src/Windows/Avalonia.Win32/Input/WindowsInputPane.cs +++ b/src/Windows/Avalonia.Win32/Input/WindowsInputPane.cs @@ -18,7 +18,7 @@ internal unsafe class WindowsInputPane : InputPaneBase, IDisposable // GUID: 5752238B-24F0-495A-82F1-2FD593056796 private static readonly Guid SID_IFrameworkInputPane = new(0x5752238B, 0x24F0, 0x495A, 0x82, 0xF1, 0x2F, 0xD5, 0x93, 0x05, 0x67, 0x96); - private readonly WindowImpl _windowImpl; + private WindowImpl _windowImpl; private IFrameworkInputPane? _inputPane; private readonly uint _cookie; @@ -72,6 +72,7 @@ private Rect ScreenRectToClient(UnmanagedMethods.RECT screenRect) public void Dispose() { + _windowImpl = null!; if (_inputPane is not null) { if (_cookie != 0) diff --git a/src/Windows/Avalonia.Win32/WindowImpl.cs b/src/Windows/Avalonia.Win32/WindowImpl.cs index 2acbd2a1dbe..f18c358525a 100644 --- a/src/Windows/Avalonia.Win32/WindowImpl.cs +++ b/src/Windows/Avalonia.Win32/WindowImpl.cs @@ -20,13 +20,10 @@ using Avalonia.Win32.Interop; using Avalonia.Win32.OpenGl; using Avalonia.Win32.OpenGl.Angle; -using Avalonia.Win32.WinRT; using Avalonia.Win32.WinRT.Composition; using static Avalonia.Win32.Interop.UnmanagedMethods; -using System.Diagnostics; using Avalonia.Platform.Storage.FileIO; using Avalonia.Threading; -using static Avalonia.Controls.Platform.IWin32OptionsTopLevelImpl; using static Avalonia.Controls.Win32Properties; using Avalonia.Logging; @@ -83,7 +80,7 @@ internal partial class WindowImpl : IWindowImpl, EglGlPlatformSurface.IEglWindow private readonly Win32NativeControlHost _nativeControlHost; private readonly IStorageProvider _storageProvider; - private readonly WindowsInputPane? _inputPane; + private WindowsInputPane? _inputPane; private WndProc _wndProcDelegate; private string? _className; private IntPtr _hwnd; @@ -613,6 +610,8 @@ public void Activate() public void Dispose() { + _inputPane?.Dispose(); + _inputPane = null; if (_hwnd != IntPtr.Zero) { // Detect if we are being closed programmatically - this would mean that WM_CLOSE was not called From 41e86fb2a2a4137695dfe23625895c08e6274dc2 Mon Sep 17 00:00:00 2001 From: Giuseppe Lippolis Date: Tue, 3 Sep 2024 18:30:58 +0200 Subject: [PATCH 2/3] fix: Address review --- src/Avalonia.Controls/TopLevel.cs | 19 +++++++------------ src/Avalonia.Controls/Window.cs | 24 +++++++++--------------- src/Avalonia.Controls/WindowBase.cs | 2 +- 3 files changed, 17 insertions(+), 28 deletions(-) diff --git a/src/Avalonia.Controls/TopLevel.cs b/src/Avalonia.Controls/TopLevel.cs index 724ff9e6f6c..3d1c0b5b5f9 100644 --- a/src/Avalonia.Controls/TopLevel.cs +++ b/src/Avalonia.Controls/TopLevel.cs @@ -219,7 +219,7 @@ public TopLevel(ITopLevelImpl impl, IAvaloniaDependencyResolver? dependencyResol _globalStyles = TryGetService(dependencyResolver); _applicationThemeHost = TryGetService(dependencyResolver); - Renderer = new CompositingRenderer(this, impl.Compositor, GetSurfaces); + Renderer = new CompositingRenderer(this, impl.Compositor, () => PlatformImpl.Surfaces ?? []); Renderer.SceneInvalidated += SceneInvalidated; impl.SetInputRoot(this); @@ -231,11 +231,11 @@ public TopLevel(ITopLevelImpl impl, IAvaloniaDependencyResolver? dependencyResol impl.ScalingChanged = HandleScalingChanged; impl.TransparencyLevelChanged = HandleTransparencyLevelChanged; - CreatePlatformImplBinding(TransparencyLevelHintProperty, (plat, hint) => plat.SetTransparencyLevelHint(hint ?? [])); - CreatePlatformImplBinding(ActualThemeVariantProperty, (plat, variant) => + CreatePlatformImplBinding(TransparencyLevelHintProperty, hint => PlatformImpl.SetTransparencyLevelHint(hint ?? Array.Empty())); + CreatePlatformImplBinding(ActualThemeVariantProperty, variant => { variant ??= ThemeVariant.Default; - plat.SetFrameThemeVariant((PlatformThemeVariant?)variant ?? PlatformThemeVariant.Light); + PlatformImpl?.SetFrameThemeVariant((PlatformThemeVariant?)variant ?? PlatformThemeVariant.Light); }); _keyboardNavigationHandler?.SetOwner(this); @@ -430,7 +430,7 @@ internal ILayoutManager LayoutManager /// public IPlatformHandle? TryGetPlatformHandle() => PlatformImpl?.Handle; - private protected void CreatePlatformImplBinding(StyledProperty property, Action onValue) + private protected void CreatePlatformImplBinding(StyledProperty property, Action onValue) { _platformImplBindings.TryGetValue(property, out var actions); _platformImplBindings[property] = actions + UpdatePlatformImpl; @@ -439,9 +439,9 @@ private protected void CreatePlatformImplBinding(StyledProperty void UpdatePlatformImpl() { - if (PlatformImpl is { } platformImpl) + if (PlatformImpl is not null) { - onValue(platformImpl, GetValue(property)); + onValue(GetValue(property)); } } } @@ -973,10 +973,5 @@ public void Dispose() _layoutManager.LayoutPassTimed = null; } } - - private IEnumerable GetSurfaces() - { - return PlatformImpl?.Surfaces ?? []; - } } } diff --git a/src/Avalonia.Controls/Window.cs b/src/Avalonia.Controls/Window.cs index 12921e934ab..bbaed03dd47 100644 --- a/src/Avalonia.Controls/Window.cs +++ b/src/Avalonia.Controls/Window.cs @@ -230,28 +230,22 @@ public Window(IWindowImpl impl) impl.ExtendClientAreaToDecorationsChanged = ExtendClientAreaToDecorationsChanged; this.GetObservable(ClientSizeProperty).Skip(1).Subscribe(x => PlatformImpl?.Resize(x, WindowResizeReason.Application)); - CreatePlatformImplBinding(TitleProperty,(plat, title) => (plat as IWindowImpl)?.SetTitle(title)); - CreatePlatformImplBinding(IconProperty, (plat, icon) => (plat as IWindowImpl)?.SetIcon((icon ?? s_defaultIcon.Value)?.PlatformImpl)); - CreatePlatformImplBinding(CanResizeProperty,(plat, canResize) => (plat as IWindowImpl)?.CanResize(canResize)); - CreatePlatformImplBinding(ShowInTaskbarProperty, (plat, show) => (plat as IWindowImpl)?.ShowTaskbarIcon(show)); + CreatePlatformImplBinding(TitleProperty, title => PlatformImpl!.SetTitle(title)); + CreatePlatformImplBinding(IconProperty, icon => PlatformImpl!.SetIcon((icon ?? s_defaultIcon.Value)?.PlatformImpl)); + CreatePlatformImplBinding(CanResizeProperty, canResize => PlatformImpl!.CanResize(canResize)); + CreatePlatformImplBinding(ShowInTaskbarProperty, show => PlatformImpl!.ShowTaskbarIcon(show)); - CreatePlatformImplBinding(WindowStateProperty, (plat, state) => - { - if (plat is IWindowImpl impl1) - { - impl1.WindowState = state; - } - }); - CreatePlatformImplBinding(ExtendClientAreaToDecorationsHintProperty, (plat, hint) => (plat as IWindowImpl)?.SetExtendClientAreaToDecorationsHint(hint)); - CreatePlatformImplBinding(ExtendClientAreaChromeHintsProperty, (plat, hint) => (plat as IWindowImpl)?.SetExtendClientAreaChromeHints(hint)); - CreatePlatformImplBinding(ExtendClientAreaTitleBarHeightHintProperty, (plat, height) => (plat as IWindowImpl)?.SetExtendClientAreaTitleBarHeightHint(height)); + CreatePlatformImplBinding(WindowStateProperty, state => PlatformImpl!.WindowState = state); + CreatePlatformImplBinding(ExtendClientAreaToDecorationsHintProperty, hint => PlatformImpl!.SetExtendClientAreaToDecorationsHint(hint)); + CreatePlatformImplBinding(ExtendClientAreaChromeHintsProperty, hint => PlatformImpl!.SetExtendClientAreaChromeHints(hint)); + CreatePlatformImplBinding(ExtendClientAreaTitleBarHeightHintProperty, height => PlatformImpl!.SetExtendClientAreaTitleBarHeightHint(height)); CreatePlatformImplBinding(MinWidthProperty, UpdateMinMaxSize); CreatePlatformImplBinding(MaxWidthProperty, UpdateMinMaxSize); CreatePlatformImplBinding(MinHeightProperty, UpdateMinMaxSize); CreatePlatformImplBinding(MaxHeightProperty, UpdateMinMaxSize); - void UpdateMinMaxSize(ITopLevelImpl plat, double _) => (plat as IWindowImpl)?.SetMinMaxSize(new Size(MinWidth, MinHeight), new Size(MaxWidth, MaxHeight)); + void UpdateMinMaxSize(double _) => PlatformImpl!.SetMinMaxSize(new Size(MinWidth, MinHeight), new Size(MaxWidth, MaxHeight)); } /// diff --git a/src/Avalonia.Controls/WindowBase.cs b/src/Avalonia.Controls/WindowBase.cs index aa668393f51..055979efb4c 100644 --- a/src/Avalonia.Controls/WindowBase.cs +++ b/src/Avalonia.Controls/WindowBase.cs @@ -47,7 +47,7 @@ static WindowBase() public WindowBase(IWindowBaseImpl impl) : this(impl, AvaloniaLocator.Current) { - CreatePlatformImplBinding(TopmostProperty, (plat, topmost) => (plat as IWindowImpl)?.SetTopmost(topmost)); + CreatePlatformImplBinding(TopmostProperty, topmost => PlatformImpl!.SetTopmost(topmost)); FrameSize = impl.FrameSize; } From 77193d107d9d5ea73a2b393ffcfd00728f26c2cd Mon Sep 17 00:00:00 2001 From: Giuseppe Lippolis Date: Mon, 30 Sep 2024 11:39:55 +0200 Subject: [PATCH 3/3] fix: Address review --- src/Windows/Avalonia.Win32/Input/WindowsInputPane.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Windows/Avalonia.Win32/Input/WindowsInputPane.cs b/src/Windows/Avalonia.Win32/Input/WindowsInputPane.cs index 2e65831ace9..79e67ecc820 100644 --- a/src/Windows/Avalonia.Win32/Input/WindowsInputPane.cs +++ b/src/Windows/Avalonia.Win32/Input/WindowsInputPane.cs @@ -21,6 +21,7 @@ internal unsafe class WindowsInputPane : InputPaneBase, IDisposable private WindowImpl _windowImpl; private IFrameworkInputPane? _inputPane; private readonly uint _cookie; + private bool _disposed; private WindowsInputPane(WindowImpl windowImpl) { @@ -72,6 +73,9 @@ private Rect ScreenRectToClient(UnmanagedMethods.RECT screenRect) public void Dispose() { + if (_disposed) + return; + _disposed = true; _windowImpl = null!; if (_inputPane is not null) { @@ -83,6 +87,8 @@ public void Dispose() _inputPane.Dispose(); _inputPane = null; } + // Suppress finalization. + GC.SuppressFinalize(this); } private class Handler : CallbackBase, IFrameworkInputPaneHandler