AdamEssenmacher / MemoryToolkit.Maui

A developer toolkit for detecting, diagnosing, and mitigating memory leaks in .NET MAUI applications.
MIT License
251 stars 13 forks source link

Border StrokeShape being forgotten (?) #18

Open Mysame opened 3 months ago

Mysame commented 3 months ago

Behold a sample of our logging, I'm sure you get the gist.

image

We do use RoundRectangle to specify the border shape in the xaml where we've implemented the TearDown

<CollectionView.ItemTemplate>
  <DataTemplate>
     <Border Margin="10,5,10,5"
             Padding="0"
             BackgroundColor="White">

         <Border.StrokeShape>
             <RoundRectangle CornerRadius="15" />
         </Border.StrokeShape>

So I suppose there's a very real possibility of the parent being disposed whilst the RoundRectangle is still alive.

sjorsmiltenburg commented 3 months ago

Good find, did you also report this on the dotnet/maui github?

Mysame commented 3 months ago

I was considering it because the urgency of the output message, but I figured there's no real point poking them if it's most likely caused by this toolkit. But if requested I definitely will, however their response will most likely be "well stop garbage collecting RealParent, then".

sjorsmiltenburg commented 3 months ago

Fair point. But I would say let them analyse the cause and determine if the relation with the toolkit is at fault.

To be clear I don't have in depth knowledge of the inner workings of this memorytoolkit. I am just a developer that applauds all efforts in finding / fixing memoryleaks in Xamarin / Maui. I want all the Maui.Windows (and other platform) memory leaks fixed ASAP as it currently does not seem possible to develop a stable product on MAUI (or xamarin for that matter). As I have a product that needs to be able to run 24/7 this is a big problem for me.

AdamEssenmacher commented 3 months ago

The error The RealParent on... was introduced in https://github.com/dotnet/maui/pull/22561, which shipped with SR6. This PR should fix the propagating leak behavior this toolkit was primarily designed to address!

So yes, using this toolkit is probably causing this (new) warn log to occur--but the good news is that TearDownBehavior probably isn't needed anymore. Try removing it and see if any leaks still happen with LeakMonitorBehavior.

Mysame commented 3 months ago

That mentions iOS specifically, I should have mentioned this is on Android.

Disabling the TearDownBehavior allows memory to rise steadily in our app before it crashes, something that doesn't happen (as fast) with it enabled.

AdamEssenmacher commented 3 months ago

That mentions iOS specifically, I should have mentioned this is on Android.

The PR comments do not fully explain the scope of the impact here. I'm not sure if PureWeen realized it or not. The reason leaks were spreading through MAUI pages so rampantly was because the strong references up through the visual tree via the Parent property. By making this property a weak reference, leaks should be compartmentalizing at the lowest level they occur (...I think; I haven't had a chance to validate this in practice yet).

Disabling the TearDownBehavior allows memory to rise steadily in our app before it crashes, something that doesn't happen (as fast) with it enabled.

That's good to know. TearDownBehavior does a handful of things, and it sounds like some of them are still useful. In addition to breaking apart views (which I think is the part that's no longer necessary), it also clears binding contexts and calls DisconnectHandler(), which isn't supposed to be necessary, but sometimes is.

Anyway, until I have a chance to dive back into this (I had to retreat back into Xamarin for a bit) you might try using the Suppress option to exclude the Border from the effects of TearDownBehavior to maybe avoid those noisy logs.