Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Bug] UWP - ListView does not recycle cells [HUUGE MEMORY ISSUE] Part 2 #14529

Open
gentledepp opened this issue Aug 24, 2021 · 0 comments
Open
Labels
a/listview Problems with the ListView/TableView p/UWP t/bug 🐛
Projects

Comments

@gentledepp
Copy link
Contributor

gentledepp commented Aug 24, 2021

Description

On UWP, the listviewrenderer does not dispose all the UI elements.
This is Part II of our UWP ListView memoryleak issue hunt as nobody seems to care about this platform.
See here for Part I #10473

Steps to Reproduce

  1. creating a listview with a datatemplateselector which returns ~5 different templates
  2. Ensure, that the templates return 5 different custom viewcell types. (because then it is easier to see the leak)
  3. add an itemssource with about 100 items, so that the listview cannot render them all at once. This will trigger recycling on UWP which is important!
  4. Open the listview, then scroll down to the end
  5. close the page and force GC

Expected Behavior

All viewcells should be disposed of and their "ondisappearing" should be fired.

Actual Behavior

Only the visible viewcells are disposed.

Basic Information

  • Version with issue: 4.8 (and I bet even MAUI)
  • Last known good version: none....
  • Platform Target Frameworks:
    • UWP: all
  • Affected Devices: UWP

Reason for the memory leak

The ListViewRenderer on UWP finds all descendant views of the UWP list and cleans them up:
image

However: As the UWP listview recycles views, not all the views are in the visible tree as they are cached on the sidelines. Therefore these are never disposed of!

Solution

ATTENTION: I do not claim that this is the perfect solition. If any of the Xamarin.Forms core developers has a better idea please come forward!

In UWP.CellControl.SetCell(object newContext), the newly created cells get their .Parent set to the listview.
However, they are never added to the list of ListView._logicalChildren
image

So we added a method AddLogicalChild() to do this. (Also, we added a method RemoveLogicalChild for cleaning up afterwards.
image

Then, in the ListViewRenderer.Dispose method, we explicitly iterate over all the ListViewLogicalChildren and

  1. call SendDisappearing which is especially important for using frameworks like ReactiveUI as its bindings must be disposed
  2. Call the VisualElementExtensions.Cleanup() method, so that all UWP platform renderers for the viewcell and all its dependents are disposed of
  3. finally remove the cell from the logicalchildren of the listview

image

This effectively fixes the issue.
You can also have a look at my PR in our Xamarin.Forms fork.

@bruzkovsky fyi!

@gentledepp gentledepp added s/unverified New report that has yet to be verified t/bug 🐛 labels Aug 24, 2021
gentledepp added a commit to Opti-Q/Xamarin.Forms that referenced this issue Aug 24, 2021
…iew in the UWP ListViewRenderer.Dispose method
gentledepp added a commit to Opti-Q/Xamarin.Forms that referenced this issue Aug 24, 2021
…iew in the UWP ListViewRenderer.Dispose method
gentledepp added a commit to Opti-Q/Xamarin.Forms that referenced this issue Aug 24, 2021
Fixed xamarin#14529 by disposing all ever created cells by of a listview in …
@jsuarezruiz jsuarezruiz added p/UWP a/listview Problems with the ListView/TableView labels Aug 24, 2021
@jsuarezruiz jsuarezruiz added this to New in Triage via automation Aug 24, 2021
@jsuarezruiz jsuarezruiz moved this from New to Needs Estimate in Triage Sep 6, 2021
@jsuarezruiz jsuarezruiz removed the s/unverified New report that has yet to be verified label Sep 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/listview Problems with the ListView/TableView p/UWP t/bug 🐛
Projects
Triage
  
Needs Estimate
Development

No branches or pull requests

2 participants