[ios] fix memory leak in CollectionView cells#15831
Merged
rmarinho merged 1 commit intodotnet:mainfrom Jun 26, 2023
Merged
Conversation
Context: dotnet#14664 Context: https://github.com/nacompllo/MemoryLeakEverywhere/tree/bugfix/memoryLeakItemsSource In reviewing our latest changes in main with the above customer sample, I found that there appeared to be leaks related to MAUI's `UITableViewCell` subclasses when using `CollectionView`. I was able to reproduce the issue in a test, such as: // DataTemplate saves WeakReference to the View in a list collectionView.ItemTemplate = new DataTemplate(() => { var label = new Label(); labels.Add(new(label)); return label; }); // Create a cell and bind it to the template: cell = new VerticalCell(CGRect.Empty); cell.Bind(collectionView.ItemTemplate, bindingContext, collectionView); // Check we have no leaks foreach (var reference in labels) { Assert.False(reference.IsAlive, "View should not be alive!"); } After isolating the issue, I found the issue was the `TemplatedCell.PlatformHandler` property: internal IPlatformViewHandler PlatformHandler { get; private set; } This stores a copy of the `LabelHandler` in our test/example. The problem with `UITableViewCell` is that UIKit holds onto these and reuses them. This means that UIKit may keep the `LabelHandler` alive longer than needed. It also appears to be a somewhat complex circular reference: * `CollectionView` -> handlers / etc. -> `TemplatedCell` -> `LabelHandler` -> `Label` -> `CollectionView` I made the `PlatformHandler` use a `WeakReference` as its backing field and the problem goes away! I will retest dotnet#14664 to verify if it is fully solved.
rmarinho
approved these changes
Jun 26, 2023
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context: #14664
Context: https://github.com/nacompllo/MemoryLeakEverywhere/tree/bugfix/memoryLeakItemsSource
In reviewing our latest changes in main with the above customer sample, I found that there appeared to be leaks related to MAUI's
UITableViewCellsubclasses when usingCollectionView.I was able to reproduce the issue in a test, such as:
After isolating the issue, I found the issue was the
TemplatedCell.PlatformHandlerproperty:This stores a copy of the
LabelHandlerin our test/example.The problem with
UITableViewCellis that UIKit holds onto these and reuses them. This means that UIKit may keep theLabelHandleralive longer than needed.It also appears to be a somewhat complex circular reference:
CollectionView-> handlers / etc. ->TemplatedCell->LabelHandler->Label->CollectionViewI made the
PlatformHandleruse aWeakReferenceas its backing field and the problem goes away!I will retest #14664 to verify if it is fully solved.