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.

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

[ios] fix memory leaks in various controls #18434

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

jonathanpeppers
Copy link
Member

Context: #18365

  • ActivityIndicator
  • BoxView
  • Polygon
  • Polyline
  • IndicatorView

Some of these were based on PlatformGraphicsView, which had a circular reference, for example:

  • BoxView -> BoxViewHandler/PlatformGraphicsViewHandler -> PlatformGraphicsView -> BoxView

  • It appears both IDrawable and IGraphicsRenderer values in PlatformGraphicsView were circular references.

Various shapes were based on PlatformGraphicsView. We don't have the iOS memory analyzer running on Microsoft.Maui.Graphics.dll yet.

Similarly:

  • ActivityIndicator -> ActivityIndicatorHandler -> MauiActivityIndicator -> ActivityIndicator

  • IndicatorView -> IndicatorViewHandler -> MauiPageControl -> IndicatorView

Open question: Why didn't the analyzer catch the issues with ActivityIndicator and IndicatorView? Investigating why.

Fixes:

* `ActivityIndicator`
* `BoxView`
* `Polygon`
* `Polyline`
* `IndicatorView`

Some of these were based on `PlatformGraphicsView`, which had a circular
reference, for example:

* `BoxView` -> `BoxViewHandler`/`PlatformGraphicsViewHandler` ->
  `PlatformGraphicsView` -> `BoxView`

* It appears both `IDrawable` and `IGraphicsRenderer` values in
  `PlatformGraphicsView` were circular references.

Various shapes were based on `PlatformGraphicsView`. We don't have
the iOS memory analyzer running on `Microsoft.Maui.Graphics.dll` yet.

Similarly:

* `ActivityIndicator` -> `ActivityIndicatorHandler` ->
  `MauiActivityIndicator` -> `ActivityIndicator`

* `IndicatorView` -> `IndicatorViewHandler` ->
  `MauiPageControl` -> `IndicatorView`

*Open question*: Why didn't the analyzer catch the issues with
`ActivityIndicator` and `IndicatorView`? Investigating why.
@jonathanpeppers jonathanpeppers added memory-leak 💦 Memory usage grows / objects live forever (sub: perf) platform/iOS 🍎 labels Oct 30, 2023
IActivityIndicator? _virtualView;
readonly WeakReference<IActivityIndicator>? _virtualView;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, it appears that interfaces slip through the analyzer:

image

Will fix in next version of analyzer.

jonathanpeppers added a commit to jonathanpeppers/memory-analyzers that referenced this pull request Oct 30, 2023
Context: dotnet/maui#18434 (comment)

This situation came up in .NET MAUI. You can create circular references
with interfaces, as any `NSObject` can implement an interface. Warn
about interfaces.
jonathanpeppers added a commit to jonathanpeppers/memory-analyzers that referenced this pull request Oct 30, 2023
Context: dotnet/maui#18434 (comment)

This situation came up in .NET MAUI. You can create circular references
with interfaces, as any `NSObject` can implement an interface. Warn
about interfaces.
@jonathanpeppers jonathanpeppers marked this pull request as ready for review October 30, 2023 17:58
@jonathanpeppers jonathanpeppers requested a review from a team as a code owner October 30, 2023 17:58
@PureWeen PureWeen added this to the .NET 8 SR1 milestone Oct 30, 2023
@PureWeen PureWeen merged commit a589b12 into dotnet:main Oct 30, 2023
47 checks passed
@jonathanpeppers jonathanpeppers deleted the iOSLeaks branch October 31, 2023 14:09
@github-actions github-actions bot locked and limited conversation to collaborators Dec 5, 2023
@samhouts samhouts added the fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! memory-leak 💦 Memory usage grows / objects live forever (sub: perf) platform/iOS 🍎
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants