Skip to content

Commit

Permalink
WIP test shows leak in CollectionView
Browse files Browse the repository at this point in the history
I added some logging for the sample:

* dotnet#14664
* https://github.com/nacompllo/MemoryLeakEverywhere/tree/bugfix/memoryLeakItemsSource

I found the "logical children" grow indefinitely on iOS/Catalyst, after
adding some logging it quickly got up to 71 items:

    2023-07-06 09:04:27.745 MemoryLeakEverywhere[93127:7440192] Microsoft.Maui.Controls.CollectionView added child -> Microsoft.Maui.Controls.Image, count: 71

I was able to reproduce in a test, after I added some code to allow the
`CollectionView` to create items (using `WidthRequest`/`HeightRequest`).

When you replace an `ItemsSource` on iOS, in this way:

    var newCollection = new ObservableCollection<string>();
    collectionView.ItemsSource = newCollection;
    foreach (var item in data)
    {
        newCollection.Add(item);
    }

It appears that the old items are not removed, the test fails with:

    Expected: 3
    Actual: 6

But passes on Windows & Android.

Somewhere, we need to call one of:

* `ItemsView.ClearLogicalChildren()`
* `ItemsView.RemoveLogicalChild()`

But I've not been successful yet.
  • Loading branch information
jonathanpeppers committed Jul 6, 2023
1 parent 4c096c1 commit bbea059
Showing 1 changed file with 34 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,36 +41,51 @@ void SetupBuilder()
[Fact]
public async Task ItemsSourceDoesNotLeak()
{
const int expectedSize = 3;
SetupBuilder();

IList logicalChildren = null;
WeakReference weakReference = null;

var collectionView = new CollectionView
{
ItemTemplate = new DataTemplate(() => new Label())
ItemTemplate = new DataTemplate(() => new Label { HeightRequest = 100 }),
};
var layout = new Grid { IgnoreSafeArea = true, HeightRequest = 500, WidthRequest = 500 };
layout.Add(collectionView);

await CreateHandlerAndAddToWindow<CollectionViewHandler>(collectionView, async handler =>
{
var data = new ObservableCollection<string>()
{
"Item 1",
"Item 2",
"Item 3"
};
weakReference = new WeakReference(data);
var data = new ObservableCollection<string>();
GenerateItems(expectedSize, data);
collectionView.ItemsSource = data;
await Task.Delay(100);

// Get ItemsView._logicalChildren
var flags = BindingFlags.NonPublic | BindingFlags.Instance;
logicalChildren = typeof(Element).GetField("_internalChildren", flags).GetValue(collectionView) as IList;
Assert.NotNull(logicalChildren);
var frame = collectionView.Frame;
await CreateHandlerAndAddToWindow<LayoutHandler>(layout, async handler =>
{
weakReference = new WeakReference(data);

// Replace with cloned collection
collectionView.ItemsSource = new ObservableCollection<string>(data);
await Task.Delay(100);
});
await WaitForUIUpdate(frame, collectionView);
frame = collectionView.Frame;

// Get ItemsView._logicalChildren
var flags = BindingFlags.NonPublic | BindingFlags.Instance;
logicalChildren = typeof(Element).GetField("_internalChildren", flags).GetValue(collectionView) as IList;
Assert.NotNull(logicalChildren);
Assert.Equal(data.Count, logicalChildren.Count);

// Replace with ObservableCollection, calling newCollection.Add()
var newCollection = new ObservableCollection<string>();
collectionView.ItemsSource = newCollection;
foreach (var item in data)
{
newCollection.Add(item);
}
await Task.Delay(100);
Assert.Equal(data.Count, logicalChildren.Count);
});

// data is out of scope here
}

await Task.Yield();
GC.Collect();
Expand All @@ -79,7 +94,7 @@ await CreateHandlerAndAddToWindow<CollectionViewHandler>(collectionView, async h
Assert.NotNull(weakReference);
Assert.False(weakReference.IsAlive, "ObservableCollection should not be alive!");
Assert.NotNull(logicalChildren);
Assert.True(logicalChildren.Count <= 3, "_logicalChildren should not grow in size!");
Assert.Equal(expectedSize, logicalChildren.Count);
}

[Theory]
Expand Down

0 comments on commit bbea059

Please sign in to comment.