mono / SkiaSharp

SkiaSharp is a cross-platform 2D graphics API for .NET platforms based on Google's Skia Graphics Library. It provides a comprehensive 2D API that can be used across mobile, server and desktop models to render images.
MIT License
4.38k stars 535 forks source link

[BUG] .Net MAUI SKCanvasView memory leak on IOS #2923

Closed nevse closed 1 month ago

nevse commented 2 months ago

Description

If you place SKCanvasView on Page it cause a memory leak.

Code

I create a simple app wich reproduce this leak https://github.com/nevse/maui-tests-SKCanvasViewLeak

Step to reproduce:

  1. Click button "Open Page"
  2. Navigate back from page with SKCanvasView
  3. Click button "Check Memory Leak"

I use such code to detect memory leak:

WeakReference pageRef;
void OnOpenPage(object sender, EventArgs e)
{
    var page = new SKCanvasViewPage();
    Navigation.PushAsync(page);
    pageRef = new WeakReference(page);
}
void OnCheckMemoryLeak(object sender, EventArgs e)
{
    GC.Collect();
    GC.WaitForPendingFinalizers();
    GC.Collect();
    if (pageRef != null && pageRef.IsAlive)
    {
        DisplayAlert("Memory Leak", "Memory Leak Detected", "OK");
    }
    else
    {
        DisplayAlert("Memory Leak", "No Memory Leak Detected", "OK");
    }
}

Expected Behavior

No response

Actual Behavior

No response

Version of SkiaSharp

2.88.3 (Current)

Last Known Good Version of SkiaSharp

Other (Please indicate in the description)

IDE / Editor

Visual Studio Code (macOS)

Platform / Operating System

iOS

Platform / Operating System Version

No response

Devices

No response

Relevant Screenshots

No response

Relevant Log Output

No response

Code of Conduct

Alexgoon commented 1 month ago

Some DevExpress components have a memory leak due to this issue, which prevents the entire page from being released. This problem is affecting many of our customers. @mattleibow, could you please take a look at this when you have a chance?

janne-hmp commented 1 month ago

@mattleibow @davidortinau I think this should be critically prioritized, since you can't use SkiaSharp with .NET MAUI on iOS seriously. I think my memory corruption problem mentioned in #2840 maybe because of the same or related issue.

janne-hmp commented 1 month ago

I just made a test app (see #2951) that confirms this by crashing my iPad by trying to open and close the same page 500 times. It crashes after opening the page c. 145th time.

mattleibow commented 1 month ago

I think I know what is happening. After all our work to fix leaks in Maui itself, it appears that an event on the platform view to the handler keeps everything.

The platform view event keeps the handler alive, the handler keeps the Xaml view alive and the xaml view keeps the parent alive. The virtual view also keeps the handler alive and the handler keeps the platform view. So basically a big web of survival.

However, there has also been a fix in Maui to break the virtual view and it's parent: https://github.com/dotnet/maui/pull/22561

We just have to do something like this: https://github.com/dotnet/maui/pull/18682

Thanks to the hard work of @jonathanpeppers ❤️

jonathanpeppers commented 1 month ago

If some SKSomethingView has a circular reference, I don't think https://github.com/dotnet/maui/pull/22561 will fix it.

Do you have any on-device tests, can you write one like:

And then, the solution would be to fix some of the warnings this analyzer catches: