Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Page-level memory leak on modal navigation on iOS #20094

Closed
AdamEssenmacher opened this issue Jan 23, 2024 · 4 comments · Fixed by #22089
Closed

Page-level memory leak on modal navigation on iOS #20094

AdamEssenmacher opened this issue Jan 23, 2024 · 4 comments · Fixed by #22089
Assignees
@jonathanpeppers
Labels
area-navigation NavigationPage fixed-in-8.0.40 memory-leak 💦 Memory usage grows / objects live forever platform/iOS 🍎 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.)
Milestone
.NET 8 SR6

Comments

@AdamEssenmacher
Copy link

AdamEssenmacher commented Jan 23, 2024 •
edited

Description

Pages pushed modally on iOS are not garbage collected after being popped, resulting in the modal page leaking.

Page-level leaks are serious, as they prevent child elements from being collected as well.

Steps to Reproduce

  1. Push page modally
  2. Pop modal page
  3. Force GC runs
  4. Observe modal page never collected

Link to public reproduction project repository

https://github.com/AdamEssenmacher/iOSModalLeak.Maui

Version with bug

8.0.6

Is this a regression from previous behavior?

Not sure, did not test other versions

Last version that worked well

Unknown/Other

Affected platforms

iOS / MacOS

Affected platform versions

iOS 17.2

Did you find any workaround?

No response

Relevant log output

No response
@AdamEssenmacher AdamEssenmacher added the t/bug Something isn't working label Jan 23, 2024
@jfversluis jfversluis added platform/iOS 🍎 legacy-area-perf Startup / Runtime performance labels Jan 23, 2024
@jsuarezruiz jsuarezruiz added the area-navigation NavigationPage label Jan 23, 2024
@fischberg
Copy link

Having same issue using Navigation.PushAsync(). When page is popped from the stack, objects on the popped ContentPage are not collected. On iOS, the app is quickly jettisoned due to excessive RAM usage. Seeing the issue on Windows and Mac Catalyst in addition to iOS.

@AdamEssenmacher
Copy link
Author

@fischberg I opened this issue because I have good reason to believe that there is leak in modal navigation itself, specifically on iOS.

It's a more general problem in MAUI apps where would-be small leaks spread through entire pages and make them uncollectible by the GC--but that's not necessarily an issue specific to navigation. It sounds like your problem may fall in to this category.

The MAUI repo has an in-depth discussion on memory leaks on the wiki: https://github.com/dotnet/maui/wiki/Memory-Leaks

I'm personally working on a toolkit that can help detect and mitigate these general-case leaks, while also helping us isolate leaks to the offending component: https://github.com/AdamEssenmacher/MemoryToolkit.Maui

@AdamEssenmacher AdamEssenmacher mentioned this issue Apr 12, 2024
Open
@RoiChen001
Copy link
Collaborator

Can repro this issue at iOS platform on the latest 17.10 Preview 4(8.0.6&8.0.20).

@RoiChen001 RoiChen001 added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Apr 23, 2024
@PureWeen PureWeen added the memory-leak 💦 Memory usage grows / objects live forever label Apr 25, 2024
@PureWeen PureWeen added this to the .NET 8 SR6 milestone Apr 25, 2024
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Apr 26, 2024
@jonathanpeppers
[ios/catalyst] fix memory leak in modal pages
503e2d6 
Fixes: dotnet#20094
Context: https://github.com/AdamEssenmacher/iOSModalLeak.Maui

In the above sample, you can see that modal `Page`'s on iOS or Catalyst
live forever after they are dismissed. I was able to reproduce this
issue in a device test.

After some investigation, the `ContainerViewController` appears to have
a cycle:

* `ContainerViewController` -> `IElement? _view;` -> `PageHandler` -> `ContainerViewController

After 7d0af63 was merged, this works fine when using `NavigationPage`,
but not when using modals.

It appears after solving the cycle, the `ContainerViewController` goes
away as well as the `PageHandler` and the `Page`.
@jonathanpeppers jonathanpeppers mentioned this issue Apr 26, 2024
Merged
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Apr 29, 2024
@jonathanpeppers
[ios/catalyst] fix memory leak in modal pages
bd07027 
Fixes: dotnet#20094
Context: https://github.com/AdamEssenmacher/iOSModalLeak.Maui

In the above sample, you can see that modal `Page`'s on iOS or Catalyst
live forever after they are dismissed. I was able to reproduce this
issue in a device test.

After some investigation, the `ContainerViewController` appears to have
a cycle:

* `ContainerViewController` -> `IElement? _view;` -> `PageHandler` -> `ContainerViewController

After 7d0af63 was merged, this works fine when using `NavigationPage`,
but not when using modals.

It appears after solving the cycle, the `ContainerViewController` goes
away as well as the `PageHandler` and the `Page`.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue May 6, 2024
@jonathanpeppers
[ios/catalyst] fix memory leak in modal pages
ec3d854 
Fixes: dotnet#20094
Context: https://github.com/AdamEssenmacher/iOSModalLeak.Maui

In the above sample, you can see that modal `Page`'s on iOS or Catalyst
live forever after they are dismissed. I was able to reproduce this
issue in a device test.

After some investigation, the `ContainerViewController` appears to have
a cycle:

* `ContainerViewController` -> `IElement? _view;` -> `PageHandler` -> `ContainerViewController

After 7d0af63 was merged, this works fine when using `NavigationPage`,
but not when using modals.

It appears after solving the cycle, the `ContainerViewController` goes
away as well as the `PageHandler` and the `Page`.
@AdamEssenmacher
Copy link
Author

Just found something else that could very well be related.

If a XAML element has an assigned name (e.g. x:Name="Something") inside a page that's being displayed modally, that element will leak.

I have a hunch this behavior will go away once the modal page leak does. I can check once the fix is in the nightly builds.

jonathanpeppers added a commit that referenced this issue May 10, 2024
@jonathanpeppers
[ios/catalyst] fix memory leak in modal pages (#22089)
7076514 
Fixes: #20094
Context: https://github.com/AdamEssenmacher/iOSModalLeak.Maui

In the above sample, you can see that modal `Page`'s on iOS or Catalyst
live forever after they are dismissed. I was able to reproduce this
issue in a device test.

After some investigation, the `ContainerViewController` appears to have
a cycle:

* `ContainerViewController` -> `IElement? _view;` -> `PageHandler` -> `ContainerViewController

After 7d0af63 was merged, this works fine when using `NavigationPage`,
but not when using modals.

It appears after solving the cycle, the `ContainerViewController` goes
away as well as the `PageHandler` and the `Page`.
@Eilon Eilon added t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) and removed legacy-area-perf Startup / Runtime performance labels May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jonathanpeppers jonathanpeppers

Labels
area-navigation NavigationPage fixed-in-8.0.40 memory-leak 💦 Memory usage grows / objects live forever platform/iOS 🍎 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.)
Projects
MAUI SDK Ongoing
Status: Done
Milestone
.NET 8 SR6
Development

Successfully merging a pull request may close this issue.

[ios/catalyst] fix memory leak in modal pages jonathanpeppers/maui
9 participants
@Eilon @jonathanpeppers @jfversluis @PureWeen @jsuarezruiz @samhouts @AdamEssenmacher @fischberg @RoiChen001