[windows] fix memory leak when CollectionView.ItemsSource changes#13530
[windows] fix memory leak when CollectionView.ItemsSource changes#13530jonathanpeppers merged 3 commits intodotnet:mainfrom
CollectionView.ItemsSource changes#13530Conversation
|
Ok Android works, because there is a callback for "recycling": I'll continue looking tomorrow, it appears that Windows uses this |
Fixes: dotnet#13393 Context: https://github.com/ivan-todorov-progress/maui-collection-view-memory-leak-bug Debugging the above sample, I found that `ItemsView._logicalChildren` grew indefinitely in size if you replace a `CollectionView`'s `ItemsSource` over and over. I could fix this by creating a new `internal` `ItemsView.ClearLogicalChildren()` method and call it from the Windows `ItemsViewHandler`. I could reproduce this issue in a Device Test running on Windows. It appears the problem does not occur on Android or iOS due to the recycling behavior of the underlying platforms. For example, Android calls `OnViewRecycled()` which calls `RemoveLogicalChild(view)`: * https://github.com/dotnet/maui/blob/53f6e393750a3df05b12fcde442daf3616c216f8/src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs#L52-L57 * https://github.com/dotnet/maui/blob/53f6e393750a3df05b12fcde442daf3616c216f8/src/Controls/src/Core/Handlers/Items/Android/TemplatedItemViewHolder.cs#L37-L45 On Windows, we merely have a `ItemContentControl` and there is no callback for when things are recycled. We *do* get a callback for when the `FormsDataContext` value changes, so an option is to clear the list in this case. After this change, my test passes -- but it was already passing on iOS and Android.
9339789 to
c7837db
Compare
Apparently iOS is only creating 1:
Assert.Equal() Failure\nExpected: 3\nActual: 1
|
Thank you for your pull request. We are auto-formating your source code to follow our code guidelines. |
|
Thank you @jonathanpeppers for this fix. I am afraid I do not know the merging strategy but to my understanding the main branch will become available as 8-preview. Is there any chance these important memory-related fixes go backported into net-7? Or what is my chance to test it within my projects in the foreseeable future? |
|
I don't know how the backporting works for MAUI (I am on the Android team). This one seems like very minimal risk; I put the label so they can take a look. |
|
Thank you! |
|
/backport to net7.0 |
|
Started backporting to net7.0: https://github.com/dotnet/maui/actions/runs/4315685016 |
…changes (#13648) Backport of #13530 to net7.0 /cc @PureWeen @jonathanpeppers


Fixes #13393
Context: https://github.com/ivan-todorov-progress/maui-collection-view-memory-leak-bug
Debugging the above sample, I found that
ItemsView._logicalChildrengrew indefinitely in size if you replace aCollectionView'sItemsSourceover and over.I could fix this by creating a new
internalItemsView.ClearLogicalChildren()method and call it from the WindowsItemsViewHandler.I could reproduce this issue in a Device Test running on Windows.
It appears the problem does not occur on Android or iOS due to the recycling behavior of the underlying platforms.
For example, Android calls
OnViewRecycled()which callsRemoveLogicalChild(view):maui/src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs
Lines 52 to 57 in 53f6e39
maui/src/Controls/src/Core/Handlers/Items/Android/TemplatedItemViewHolder.cs
Lines 37 to 45 in 53f6e39
On Windows, we merely have a
ItemContentControland there is no callback for when things are recycled. We do get a callback for when theFormsDataContextvalue changes, so an option is to clear the list in this case.After this change, my test passes -- but it was already passing on iOS and Android.