Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Win32): PopupImpl memory leak #16890

Merged
merged 4 commits into from
Sep 30, 2024
+12 −7
Merged

fix(Win32): PopupImpl memory leak #16890

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7041139
fix: PopupImpl memory leak
workgroupengineering Sep 2, 2024
41e86fb
fix: Address review
workgroupengineering Sep 3, 2024
77193d1
fix: Address review
workgroupengineering Sep 30, 2024
154cd0d
Merge branch 'master' into fix/Platform/Windows/PopupImpl
maxkatz6 Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/Avalonia.Controls/TopLevel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -220,7 +219,7 @@ public TopLevel(ITopLevelImpl impl, IAvaloniaDependencyResolver? dependencyResol
_globalStyles = TryGetService<IGlobalStyles>(dependencyResolver);
_applicationThemeHost = TryGetService<IThemeVariantHost>(dependencyResolver);

Renderer = new CompositingRenderer(this, impl.Compositor, () => impl.Surfaces);
Renderer = new CompositingRenderer(this, impl.Compositor, () => PlatformImpl.Surfaces ?? []);
Renderer.SceneInvalidated += SceneInvalidated;

impl.SetInputRoot(this);
Expand Down
3 changes: 2 additions & 1 deletion src/Windows/Avalonia.Win32/Input/WindowsInputPane.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -72,6 +72,7 @@ private Rect ScreenRectToClient(UnmanagedMethods.RECT screenRect)

public void Dispose()
{
_windowImpl = null!;
if (_inputPane is not null)
{
if (_cookie != 0)
Expand Down
7 changes: 3 additions & 4 deletions src/Windows/Avalonia.Win32/WindowImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -613,6 +610,8 @@ public void Activate()

public void Dispose()
{
_inputPane?.Dispose();
_inputPane = null;
Comment on lines +613 to +614
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already dispose pane on WM_DESTROY. And it should be guaranteed to be called unless process was terminated (in which case we don't care about disposing).

Avalonia/src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs

Line 112 in 41e86fb

_inputPane?.Dispose();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than that lgtm

if (_hwnd != IntPtr.Zero)
{
// Detect if we are being closed programmatically - this would mean that WM_CLOSE was not called
Expand Down
Loading