dotnet / maui

.NET MAUI is the .NET Multi-platform App UI, a framework for building native device applications spanning mobile, tablet, and desktop.
https://dot.net/maui
MIT License
21.83k stars 1.67k forks source link

`RefreshView` can have a memory leak with a custom `ICommand` #16124

Open jonathanpeppers opened 11 months ago

jonathanpeppers commented 11 months ago

Description

This problem can be illustrated with a test such as:

https://github.com/dotnet/maui/compare/main...jonathanpeppers:RefreshViewLeak

If you create a custom ICommand on something like a singleton ViewModel, and use it on a RefreshView, the RefreshView will live forever.

I think this is lower priority, as the customer samples I've seen use Command or Command<T>:

https://github.com/dotnet/maui/blob/49d22ac57ce1aeb23546f5b2f8155fa97cf002e3/src/Controls/src/Core/Command.cs#L113-L117

And WeakEventManager sidesteps this problem.

If we fix this, it might be worth auditing all properties of type ICommand.

Steps to Reproduce

  1. Create a custom ICommand
  2. Use it on RefreshView
  3. RefreshView lives forever

Link to public reproduction project repository

https://github.com/dotnet/maui/compare/main...jonathanpeppers:RefreshViewLeak

Version with bug

8.0.0-preview.5.8529

Last version that worked well

Unknown/Other

Affected platforms

iOS, Android, Windows, macOS, Other (Tizen, Linux, etc. not supported by Microsoft directly)

Affected platform versions

All

Did you find any workaround?

Use Command.

Relevant log output

No response

ghost commented 11 months ago

We've added this issue to our backlog, and we will work to address it as time and resources allow. If you have any additional information or questions about this issue, please leave a comment. For additional info about issue management, please read our Triage Process.

dk-mushiyoke commented 3 weeks ago

Another workaround that I have found, in xaml give refresh view an x:Name and in code behind subscribe to the page Unloaded event and set RefreshView.Command to null

public MainPage()
{
    InitializeComponent();

    Unloaded += MainPage_Unloaded;
}

private static void MainPage_Unloaded(object? sender, EventArgs e)
{
    // Clear refresh view command to avoid memory leaks
    if (sender is MainPage mainPage && mainPage.MyRefreshView?.Command != null)
    {
        mainPage.MyRefreshView.Command = null;
    }
}