CommunityToolkit / Maui

The .NET MAUI Community Toolkit is a community-created library that contains .NET MAUI Extensions, Advanced UI/UX Controls, and Behaviors to help make your life as a .NET MAUI developer easier
https://learn.microsoft.com/dotnet/communitytoolkit/maui
MIT License
2.2k stars 375 forks source link

[BUG] iOS: Popup Remains in Page's LogicalChildren after Closing #2149

Closed NeilMalcolm closed 5 days ago

NeilMalcolm commented 3 weeks ago

Is there an existing issue for this?

Did you read the "Reporting a bug" section on Contributing file?

Current Behavior

When we create a popup on iOS we add two Elements to the Page's LogicalChildren: the Popup, and the ContentPage housing the Content.

When the popup is closed, the former is removed from the Page's Logical Children but the latter is not. This causes the popup (and its ViewModel) to actually remain in the heap until the page is popped and the reference (the Page's logical children) is cleared out.

Expected Behavior

The Popup and ContentPage should be removed from the Logical Children.

We can't really rely on popping the page to do this for us because the page in question could be the home page of an application which may not ever be popped.

Steps To Reproduce

  1. Create a page with two buttons
  2. One button, Show Popup, should create a new popup and call App.Current.MainPage.ShowPopup
  3. The other, Collect Garbage, should force Garbage Collection
  4. Run this on iOS and tap ShowPopup a few times
  5. Tap Collect Garbage (a few times for good measure...)
  6. Get a GC dump

When looking at the GC dump you'll see the popups exist in memory. In my reproduction sample I'm checking WeakReference.IsAlive, but this was after checking GC dumps manually.

Link to public reproduction project repository

https://github.com/NeilMalcolm/PopupInMemoryIssue

In my reproduction project, I've added a static boolean which can be toggled to enable the 'fix' for subsequent popups which allows these to be garbage collected. The fix was made a subclass of the PopupHandler, see here https://github.com/NeilMalcolm/PopupInMemoryIssue/blob/main/src/MauiPopupInMemory/Platforms/iOS/MyPopupHandler.cs

My fix is related to issue https://github.com/CommunityToolkit/Maui/issues/1676. When the page is popped, the popup is able to be garbage collected, but as long as the page remains in the navigation stack it won't because the ContentPage remains a Logical Child of the Page.

Environment

- .NET MAUI CommunityToolkit:9.0.3
- OS:iOS
- .NET MAUI: 8.0.80

Anything else?

Thank you all for maintaining this project, it's so important for MAUI.

cat0363 commented 2 weeks ago

@NeilMalcolm , Is the behavior you expect as below?

https://github.com/user-attachments/assets/f80614eb-a6ae-4cff-8154-f48a4e870f47

Currently, I am testing the case where Popup is displayed hierarchically.

NeilMalcolm commented 1 week ago

@cat0363 Thanks for taking a look at this and sorry for the late reply -- been a way for a little while.

That looks more like what I'd expect! From the recording it seems like with this change when the GC happens all popups that are no longer needed are collected.

cat0363 commented 1 week ago

@NeilMalcolm , Thank you for your reply. Thanks to your code, I was able to find out that the Popup and ContentPage continue to remain in LogicalChildren. I just created a PR.

GUELIANEBelkacem commented 1 week ago

I was going to create an issue on this, my case is exactly the worst case scenario where many popups are shown in the main page so it's never popped.

Hope this gets resolved soon 🫡