Memory Leak by default in VisualElementRenderer on iOS

[markmccaigue]

Description

Using a view backed by a renderer inheriting from VisualElementRenderer, such as via AddCompatibilityRenderer, will result in a memory leak on iOS, unless DisconnectHandler is explicitly called on the element.

In particular, it is the Dispose function on the VisualElementRenderer which must be invoked to avoid the leak, and this is invoked by the DisconnectHandler function on the RendererToHandlerShim instance which is created by the framework at runtime.

I'm hoping to establish whether this is indeed, as I currently understand it, a bug, or if this is the behaviour by design.

It's my understanding currently that by design it should not be necessary for DisconnectHandler to be called on a view in order to allow that view to be eligible for garbage collection. This would seem to be supported by my understanding of the "Handler Does Not Leak" device tests, which don't call DisconnectHandler, but do show collection of the view taking place. It also matches my interpretation of the intended MAUI design philosophy in this area, which is that unless we developers hold a reference to a view, the GC will take care of it. Thirdly, the Android view is not leaked in the reproduction scenario even when DisconnectHandler is not called.

What gives me some pause is the documentation would suggest that DisconnectHandler is the intended place for unsubscribing from event handlers (which could presumably be long-lived) and performing native cleanup, and indeed calling Dispose on native resources, which to my interpretation would indicate that it would in fact be necessary to call this to mark views as eligible for garbage collection. This does seem to somewhat be at odds from what would be implied by the above, so it would be much appreciated if anyone from the team is able to help me improve my understanding of this area.

However, whether by design or as a bug, this issue does represent somewhat of a hazard when migrating from XF apps. Previously in XF, renderers could be used without causing leaks, due to the more aggressive XF philosophy of clearing up view hierarchies on behalf of the developer. However, when using the compatibility renderers in the same manner as previously, my understanding is that any usage will cause a leak on iOS. This will likely also cause large leaks, as the leaked views will maintain strong references to their parents all the way up to the Page level.

Whilst it would be possible to avoid these leaks by calling DisconnectHandler, it's possibly a bit of a hidden workaround that may not be obvious to developers. It's also a bit tricky to call DisconnectHandler at the right time. The unloaded event is often not suitable as there are some scenarios where views can be pushed over other views for navigation, and will become unloaded at that time, even though they should remain alive for when they are returned from the back stack.

It would be great to hear some thoughts from the team about the best way to fix or mitigate this issue, many thanks.

Steps to Reproduce

1. Run the sample at the linked repository
2. Use the navigate button to navigate to a new page, and press the back arrow to go back to the original page. Repeat this a number of times.
3. Back on the main page, use the GC button to trigger garbage collections by clicking a few times.
4. Observe the memory usage does not return to baseline (rendered in onscreen label for convenience).
5. Uncomment the call to DisconnectHandler in the HighMemoryPage and repeat the steps above
6. Observe the memory returns to baseline (rendered in onscreen label for convenience)

Link to public reproduction project repository

https://github.com/markmccaigue/VisualElementRendererLeakTest

Version with bug

8.0.21 SR4.1

Is this a regression from previous behavior?

Yes, this used to work in Xamarin.Forms

Last version that worked well

Unknown/Other

Affected platforms

iOS

Affected platform versions

No response

Did you find any workaround?

No response

Relevant log output

No response

[kevinxufei]

Can repro this issue at iOS platform at the latest 17.10 Preview 5 (8.0.3/8.0.21) with sample project.

[markmccaigue]

Hi @PureWeen thank you for reviewing this issue. Whilst I was hoping that such a major leak would be assigned to an upcoming cycle rather than the backlog, I do fully understand that there are competing priorities for upcoming work.

I wonder however if you might be able, in the interim, to give me a steer on the question on the design intent of DisconnectHandler in relation to GC that I raised in the ticket? It would be great to have your opinion on this matter, as it would help us best design some patterns to workaround the issue in our codebase.