[ios] fix memory leak with Page + Layout

[jonathanpeppers]

Further fixes #13520

7d0af63 did not appear to be a complete fix for #13520, as when I retested the customer sample, I found I needed to remove a VerticalStackLayout for the problem to go away.

I could reproduce the issue in a test by doing:

var page = new ContentPage { Title = "Page 2", Content = new VerticalStackLayout { new Label() } };

After lots of debugging, I found the underlying issue was that MauiView on iOS held a reference to the cross-platform IView.

After changing the backing field for this to be a WeakReference<IView> the above problem appears to be solved.

I also removed LayoutView.CrossplatformArrange/Measure as we can just use the View property directly instead.

[jonathanpeppers]

I removed these, because you can just call View.CrossPlatformMeasure() directly.

Otherwise we might need to unset them in places, like in 7d0af63.

[hartez]

I removed these, because you can just call View.CrossPlatformMeasure() directly.

The design intent here was for the handlers to be able to directly modify the xplat measure/arrange methods directly when necessary. It's not currently used on iOS (mostly for accidental historical reasons), but the other platforms do make use of it. The intent was to keep the designs symmetric on all the platforms.

(The main place you can see this at work in the Android and Windows ScrollViewHandlers.)

So I'd rather not remove these. Or, if there's a compelling memory-leak reason to remove them, we'll need to work out a way to inject custom xplat measure/arrange methods from the handlers.

[jonathanpeppers]

The design intent here was for the handlers to be able to directly modify the xplat measure/arrange methods directly when necessary.

Is this design documented? So other people will know about it?

Or, if there's a compelling memory-leak reason to remove them, we'll need to work out a way to inject custom xplat measure/arrange methods from the handlers.

The leak appears to return if I put them back. It makes the shell & navigation DoNotLeak tests fail. Will share a *.gcdump file tomorrow.

[hartez]

Is this design documented? So other people will know about it?

Clearly not well enough, if at all. 🙂

[hartez]

After lots of debugging, I found the underlying issue was that MauiView on iOS held a reference to the cross-platform IView.

Why is the MauiView still around? What's holding a reference to it? I'm not clear on why the MauiView itself can't be collected along with the View (the VerticalStackLayout?) that it's referencing.

[jonathanpeppers]

Why is the MauiView still around?

The *.gcdump file I'm able to get from Mono just says MauiView > Other Roots. I can make a new one tomorrow and share it here. Other Roots is probably a GCHandle that is holding it until its Objective-C refcount is 0. In 7d0af63, calling RemoveFromSuperview() fixed a similar issue.

It may be that we are still leaking MauiView's all the time -- and this is only slightly better in that Page's on iOS can go away now. When removing CrossplatformArrange/Measure and making View weak, the MauiView no longer holds onto the cross-platform IView.

I believe there could be some core problem here that isn't understood. But I've not been able to reproduce a real issue here: https://github.com/jonathanpeppers/MemoryLeaksOniOS

[jonathanpeppers]

Here is the path to the leak with the changes reverted:

I changed the ContentPage to FooPage : ContentPage, to make it easier to spot. (ContentPage's are in the test runner UI)

You can unzip & open this in VS: my-dev-port_20230323_094815.zip

[jonathanpeppers]

I've been doing more testing and I think this is what was going on:

1. VerticalStackLayout -> LayoutHandler : MAUI views hold a strong reference to handlers
   a. NOTE: VerticalStackLayout.Parent is a ContentPage which is the main leak
2. LayoutHandler -> LayoutView: handlers hold a strong reference to native view
3. LayoutView -> VerticalStackLayout: LayoutView held a strong reference to the MAUI view due to CrossplatformLayout/Arrange and the View property.

I made no. 3 a weak reference, and this appears to break the cycle and all of the above can go away... Note that cycles are only a problem for NSObject subclasses -- there is not an issue like this with plain C# objects or Android Java.Lang.Object subclasses.

I just retested with the fix in place, and I don't see VerticalStackLayout, LayoutHandler, or LayoutView staying around from the page I'm testing.

[jonathanpeppers]

Ok, I have a minimal repro of a leak on iOS.

The core issue was doing something like this:

class MyViewSubclass : UIView
{
    public UIView? Parent { get; set; }

    public void Add(MyViewSubclass subview)
    {
        subview.Parent = this;
        AddSubview(subview);
    }
}

//...

var parent = new MyViewSubclass();
var view = new MyViewSubclass();
parent.Add(view);

Appears to leak unless you do:

view.Parent = null;

[jonathanpeppers]

@hartez do you have any other concerns here?