search

mono / SkiaSharp.Extended

SkiaSharp is a cross-platform, comprehensive 2D graphics API for all .NET platforms. And, here is where you will find all sorts of extras that you can use with it.
https://mono.github.io/SkiaSharp.Extended
MIT License
218 stars 69 forks source link

[BUG] MAUI SKLottieView causes cascading memory leak #250

Closed AdamEssenmacher closed 2 months ago

AdamEssenmacher commented 6 months ago

Description

Adding an SKLottieView to any MAUI page introduces a memory leak that will cause the SKLottieView and the containing page to never be garbage collected. This is especially problematic as it has a cascading effect that prevents anything else on the page from being collected, as well as anything else the page might be referencing.

Code

I'm omitting the rest of the standard bug report because I've isolated the culprit code, and the issue should be self-evident:

https://github.com/mono/SkiaSharp.Extended/blob/93efd2d38783d6d8f638623327e7f9749cea70e2/source/SkiaSharp.Extended.UI/Controls/SKAnimatedSurfaceView.shared.cs#L76-L83

The callback passed into StartTimer captures a reference to the current class instance. It is only terminated when IsAnimationEnabled is false. With the dispatcher referencing the class instance, the class instance, in turn, references its parent page (either directly or indirectly) through the .Parent property, so the leak propagates.

I haven't put much thought into an appropriate fix, but my first thought is that this code should use a weak reference to prevent the dispatcher from capturing the instance.

Yannikk1996 commented 5 months ago

I am also facing this problem. Used lots of SKLottieViews. Does anyone knows a workaround for this?

AdamEssenmacher commented 5 months ago

I am also facing this problem. Used lots of SKLottieViews. Does anyone knows a workaround for this?

@Yannikk1996 set IsAnimationEnabled to false when you're done with the animation. It will terminate the loop, which will break the reference from the Dispatcher and allow the view to be GC'ed.

sjorsmiltenburg commented 5 months ago

Thank you Adam for finding these leaks! I hope with this evidence the team will pick them up quickly as they are blocking for production apps that need to be able to run continuously.

For reference: I removed my lottie animations because of this leak and replaced my looping animation with a static image. Also I replaced my custom loading animations for activityindicator controls.

logikonline commented 4 months ago

I am also facing this problem. Used lots of SKLottieViews. Does anyone knows a workaround for this?

@Yannikk1996 set IsAnimationEnabled to false when you're done with the animation. It will terminate the loop, which will break the reference from the Dispatcher and allow the view to be GC'ed.

The problem is we need the frame it stops on to remain. Doing what you say removes the current frame and leaves it blank.

AdamEssenmacher commented 4 months ago

The problem is we need the frame it stops on to remain. Doing what you say removes the current frame and leaves it blank.

Sorry I should have said 'done with the SKLottieView' and not 'done with the animation'.

So, whenever the view is permanently unloaded.