[UWP] CollectionView Memory Leak

[YZahringer]

Description of Change

Unsubscribe from oldElement on TearDownOldElement instead of current Layout/ItemsViewthat is already null.

Issues Resolved

• fixes
  • [Bug] CollectionView Memory Leak on Navigate #14721
  • [Bug] UWP CollectionView memory leak #8928

API Changes

None

Platforms Affected

• UWP

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

Case 1: Nav back from CollectionView

1. Take a Memory Snapshot
2. Navigate to a View that contains a CollectionView
3. Go back
4. Take another Memory Snapshot and compare it with the first one
5. Check that the CollectionView, BindingContext VM, Items VM are released

Case 2: Change ItemsSource of CollectionView

1. Take a Memory Snapshot
2. Add items to the CollectionView
3. Replace the ItemsSource with an empty collection
4. Take another Memory Snapshot and compare it with the first one
5. Check that the Items VM are released

Simple project available here: https://github.com/MADSENSE/Madsense.XamarinForms.Sample/tree/collectionview-memory-leak

PR Checklist

• Targets the correct branch
• Tests are passing (or failures are unrelated)

[YZahringer]

I'm not sure about this null assignment, but if we don't do it, the memory leak persists.
I also tried calling SetInheritedBindingContext(oldStructuredItemsView.ItemsLayout, null) instead, without success.

@jsuarezruiz maybe you have an idea or can validate that this should not produce side effects?

[jfversluis]

Hey Yann! Sorry I only got to this now. I will try and think how we can validate this. I assume you tested this yourself? Are you using this code now in any of your apps by any chance?

[YZahringer]

Hi Gerald, I use the simple project available here with VS Diagnostics Tools to reproduce/check the fix.

For the moment, on production we use an extended renderer as workaround on many apps. Not exactly the same code, because there are private/internal on the base renderer, but the result is the same:

public class ExtendedCollectionViewRenderer : Xamarin.Forms.Platform.UWP.CollectionViewRenderer
{
    protected override void Dispose(bool disposing)
    {
        base.TearDownOldElement(Element);

        if (Element.ItemsLayout is LinearItemsLayout linearItemsLayout)
        {
            Element.ItemsLayout = new LinearItemsLayout(linearItemsLayout.Orientation);
        }
        else if (Element.ItemsLayout is GridItemsLayout gridItemsLayout)
        {
            Element.ItemsLayout = new GridItemsLayout(gridItemsLayout.Span, gridItemsLayout.Orientation);
        }
        else
        {
            Element.ItemsLayout = null;
        }

        base.Dispose(disposing);
    }

    protected override void CleanUpCollectionViewSource()
    {
        var itemContentControls = ListViewBase.GetChildren<ItemContentControl>();
        foreach (var itemContentControl in itemContentControls)
        {
            itemContentControl.FormsDataContext = null;
            itemContentControl.DataContext = null;
        }

        base.CleanUpCollectionViewSource();
    }
}

[jfversluis]

When the build completes there should be NuGets produced, would you maybe be able to try them out on projects where you have this problem and see if this works as expected and doesn't break anything else? The instructions can be found here. That would be very helpful, thanks!

[YZahringer]

I did some tests with the nuget produced on an application with multiple CollectionView on different views, the Memory Leak is solved.
By the way, I also updated the test project.

[jfversluis]

Thanks for the cooperation @YZahringer !

[jfversluis]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).