Skip to content

[ios] fix memory leaks in various controls#18434

Merged
PureWeen merged 1 commit intodotnet:mainfrom
jonathanpeppers:iOSLeaks
Oct 30, 2023
Merged

[ios] fix memory leaks in various controls#18434
PureWeen merged 1 commit intodotnet:mainfrom
jonathanpeppers:iOSLeaks

Conversation

@jonathanpeppers
Copy link
Copy Markdown
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 perf/memory-leak 💦 Memory usage grows / objects live forever (sub: perf) platform/ios labels Oct 30, 2023
public class MauiActivityIndicator : UIActivityIndicatorView, IUIViewLifeCycleEvents
{
IActivityIndicator? _virtualView;
readonly WeakReference<IActivityIndicator>? _virtualView;
Copy link
Copy Markdown
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
@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! perf/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