Skip to content

[net7.0] [windows] fix memory leak when CollectionView.ItemsSource changes#13648

Merged
PureWeen merged 3 commits intonet7.0from
backport/pr-13530-to-net7.0
Mar 2, 2023
Merged

[net7.0] [windows] fix memory leak when CollectionView.ItemsSource changes#13648
PureWeen merged 3 commits intonet7.0from
backport/pr-13530-to-net7.0

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot commented Mar 2, 2023

Backport of #13530 to net7.0

/cc @PureWeen @jonathanpeppers

jonathanpeppers and others added 3 commits March 2, 2023 15:41
Fixes: #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.
Apparently iOS is only creating 1:

    Assert.Equal() Failure\nExpected: 3\nActual:   1
@PureWeen
Copy link
Copy Markdown
Member

PureWeen commented Mar 2, 2023

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

@jsuarezruiz jsuarezruiz added t/housekeeping ♻︎ area-controls-collectionview CollectionView, CarouselView, IndicatorView labels Mar 2, 2023
@PureWeen PureWeen enabled auto-merge March 2, 2023 16:56
@PureWeen PureWeen merged commit d6d21b2 into net7.0 Mar 2, 2023
@PureWeen PureWeen deleted the backport/pr-13530-to-net7.0 branch March 2, 2023 22:24
@rmarinho
Copy link
Copy Markdown
Member

/backport to release/7.0.2xx

@github-actions
Copy link
Copy Markdown
Contributor Author

Started backporting to release/7.0.2xx: https://github.com/dotnet/maui/actions/runs/4469316800

@github-actions github-actions bot locked and limited conversation to collaborators Dec 12, 2023
@samhouts samhouts added the fixed-in-7.0.92 Look for this fix in 7.0.92! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-controls-collectionview CollectionView, CarouselView, IndicatorView fixed-in-7.0.92 Look for this fix in 7.0.92! platform/windows t/housekeeping ♻︎

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants