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

Fixes various memory leaks in listview with group headers (#6761) #12535

Closed
wants to merge 12 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
<?xml version="1.0" encoding="utf-8" ?>
<controls:TestContentPage xmlns:controls="clr-namespace:Xamarin.Forms.Controls"
xmlns="http://xamarin.com/schemas/2014/forms"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
xmlns:d="http://xamarin.com/schemas/2014/forms/design"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" mc:Ignorable="d"
x:Class="Xamarin.Forms.Controls.Issues.Issue6761">
<StackLayout>
<StackLayout Orientation="Horizontal">
<Button AutomationId="Issue6761Refresh" Clicked="RefreshItems" Text="Refresh" BackgroundColor="Cyan"/>
<Button AutomationId="Issue6761GC" Clicked="GCClick" Text="GC" BackgroundColor="Cyan"/>
</StackLayout>
<Label Text="Scroll the list, tap Refresh button a few times." />
<Label AutomationId="Issue6761Label" Text="{Binding DisposeString}"/>
<Label AutomationId="Issue6761Mem" Text="{Binding TotalMemory}"/>
<StackLayout Orientation="Horizontal">
<ListView
ItemsSource="{Binding ItemGroups}"
IsGroupingEnabled="True"
GroupDisplayBinding="{Binding GroupName}">
<ListView.ItemTemplate>
<DataTemplate>
<ViewCell>
<StackLayout Margin="20">
<Label Text="{Binding Name}" />
</StackLayout>
</ViewCell>
</DataTemplate>
</ListView.ItemTemplate>
</ListView>
<ListView
ItemsSource="{Binding ItemGroups}"
IsGroupingEnabled="True">
<ListView.GroupHeaderTemplate>
<DataTemplate>
<ViewCell>
<StackLayout Padding="15, 7, 15, 7" Orientation="Horizontal" BackgroundColor="#ffdab0">

<Frame
CornerRadius="14"
BackgroundColor="#cf7806"
Padding="2"
Margin="0"
HasShadow="false" >

<Frame
CornerRadius="12"
BackgroundColor="#ffdab0"
Padding="0"
Margin="0"
HasShadow="false" >

<Label
Text="{Binding GroupName}"
Margin="10, 5, 10, 5"
TextColor="#cf7806"
HorizontalTextAlignment="Center"
HorizontalOptions="Center"
VerticalTextAlignment="Center"
VerticalOptions="Center" />
</Frame>
</Frame>
</StackLayout>
</ViewCell>
</DataTemplate>
</ListView.GroupHeaderTemplate>
<ListView.ItemTemplate>
<DataTemplate>
<ViewCell>
<StackLayout Margin="20">
<Label Text="{Binding Name}" />
</StackLayout>
</ViewCell>
</DataTemplate>
</ListView.ItemTemplate>
</ListView>
</StackLayout>
</StackLayout>
</controls:TestContentPage>
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
using Xamarin.Forms.CustomAttributes;
using Xamarin.Forms.Internals;
using System;
using System.Collections.ObjectModel;
using System.Runtime.CompilerServices;
using System.ComponentModel;
using System.Collections.Generic;


namespace Xamarin.Forms.Controls.Issues
{
[Preserve(AllMembers = true)]
[Issue(IssueTracker.Github, 6761, "Memory leak in ListView with group headers on IOS", PlatformAffected.Default)]
public partial class Issue6761 : TestContentPage
{
public Issue6761()
{
#if APP
InitializeComponent();
#endif
BindingContext = _viewModel = new ViewModelIssue6761();

}

protected override void Init()
{
}

int _count;
ViewModelIssue6761 _viewModel;
IList<WeakReference<ModelIssue6761Group>> _weakList = new List<WeakReference<ModelIssue6761Group>>();
private void RefreshItems(object sender, EventArgs e)
{
foreach (var item in this._viewModel.ItemGroups)
{
item.Clear();
}
this._viewModel.ItemGroups.Clear();

for (int i = 0; i < 30; i++)
{
_count++;
var g1 = new ModelIssue6761Group("Group " + _count);
_weakList.Add(new WeakReference<ModelIssue6761Group>(g1));
this._viewModel.ItemGroups.Add(g1);
}

CleanAndReport();
}

private void GCClick(object sender, EventArgs e)
{
CleanAndReport();
}
void CleanAndReport()
{
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();

CleanWeakList(_weakList);
_viewModel.DisposeString = $"{_weakList.Count} object(s) alive";

string report = $"MEMORY:{GC.GetTotalMemory(true)}";
_viewModel.TotalMemory = report;
}

private void CleanWeakList(IList<WeakReference<ModelIssue6761Group>> weakList)
{
ModelIssue6761Group item;
for (int i = weakList.Count-1; i >= 0; i--)
{
if (!weakList[i].TryGetTarget(out item))
{
weakList.RemoveAt(i);
}
}
}
}

[Preserve(AllMembers = true)]
public class ViewModelIssue6761 : INotifyPropertyChanged
{
public ObservableCollection<ModelIssue6761Group> ItemGroups { get; set; } = new ObservableCollection<ModelIssue6761Group>();
public event System.ComponentModel.PropertyChangedEventHandler PropertyChanged;

private void NotifyPropertyChanged([CallerMemberName] string propertyName = null)
{
this.PropertyChanged?.Invoke(this, new System.ComponentModel.PropertyChangedEventArgs(propertyName));
}


string _DisposeString;
public string DisposeString
{
get { return _DisposeString; }
set
{
_DisposeString = value;
NotifyPropertyChanged();
}
}

string _totalMemory;
public string TotalMemory
{
get => _totalMemory;
set
{
_totalMemory = value;
NotifyPropertyChanged();
}
}

public ViewModelIssue6761()
{

}
}

[Preserve(AllMembers = true)]
public class ModelIssue6761Group : ObservableCollection<ModelIssue6761>
{
public ModelIssue6761Group(string name)
{
GroupName = name;
}
public string GroupName { get; set; }

}

[Preserve(AllMembers = true)]
public class ModelIssue6761
{
public ModelIssue6761(string name)
{
Name = name;
}
public string Name { get; set; }

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
<DependentUpon>Issue11794.xaml</DependentUpon>
</Compile>
<Compile Include="$(MSBuildThisFileDirectory)Issue11769.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue6761.xaml.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue12079.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue12060.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue12344.xaml.cs">
Expand Down Expand Up @@ -1668,6 +1669,9 @@
<Compile Include="$(MSBuildThisFileDirectory)Issue11081.xaml.cs">
<DependentUpon>Issue11081.xaml</DependentUpon>
</Compile>
<Compile Update="$(MSBuildThisFileDirectory)Issue6761.xaml.cs">
<DependentUpon>Issue6761.xaml</DependentUpon>
</Compile>
<Compile Include="$(MSBuildThisFileDirectory)Issue12372.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue12374.xaml.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue12222.cs" />
Expand Down Expand Up @@ -1823,6 +1827,9 @@
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue4040.xaml">
<Generator>MSBuild:UpdateDesignTimeXaml</Generator>
</EmbeddedResource>
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue6761.xaml">
<Generator>MSBuild:UpdateDesignTimeXaml</Generator>
</EmbeddedResource>
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue8787.xaml" />
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue9771.xaml">
<SubType>Designer</SubType>
Expand Down
17 changes: 9 additions & 8 deletions Xamarin.Forms.Core/TemplatedItemsList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -231,17 +231,16 @@ public void Dispose()
return;

_itemsView.PropertyChanged -= BindableOnPropertyChanged;
ListProxy.CollectionChanged -= OnProxyCollectionChanged;

TItem header = HeaderContent;
if (header != null)
UnhookItem(header);

for (var i = 0; i < _templatedObjects.Count; i++)
if (header != null && header.BindingContext != null)
{
TItem item = _templatedObjects[i];
if (item != null)
UnhookItem(item);
UnhookItem(header);
}

HeaderContent = null;
UnhookAndClear();

_disposed = true;
}
Expand Down Expand Up @@ -710,7 +709,7 @@ void GroupedReset()
_groupedItems.Clear();
}

_templatedObjects.Clear();
UnhookAndClear();

var i = 0;
foreach (object item in ListProxy)
Expand Down Expand Up @@ -815,6 +814,7 @@ void OnCollectionChangedGrouped(NotifyCollectionChangedEventArgs e)
til.CollectionChanged -= OnInnerCollectionChanged;
oldItems.Add(til);
_groupedItems.RemoveAt(index);
UnhookItem(_templatedObjects[index]);
_templatedObjects.RemoveAt(index);
til.Dispose();
}
Expand All @@ -839,6 +839,7 @@ void OnCollectionChangedGrouped(NotifyCollectionChangedEventArgs e)
oldItems.Add(til);

_groupedItems.RemoveAt(index);
UnhookItem(_templatedObjects[index]);
_templatedObjects.RemoveAt(index);

newItems.Add(InsertGrouped(e.NewItems[i], index));
Expand Down
2 changes: 1 addition & 1 deletion Xamarin.Forms.Platform.iOS/Cells/CellTableViewCell.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public class CellTableViewCell : UITableViewCell, INativeElementView
{
Cell _cell;

public Action<object, PropertyChangedEventArgs> PropertyChanged;
public event PropertyChangedEventHandler PropertyChanged;

bool _disposed;

Expand Down
2 changes: 1 addition & 1 deletion Xamarin.Forms.Platform.iOS/Cells/TextCellRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public override UITableViewCell GetCell(Cell item, UITableViewCell reusableCell,
SetRealCell(item, tvc);

tvc.Cell = textCell;
tvc.PropertyChanged = HandleCellPropertyChanged;
tvc.PropertyChanged += HandleCellPropertyChanged;

tvc.TextLabel.Text = textCell.Text;
tvc.DetailTextLabel.Text = textCell.Detail;
Expand Down
23 changes: 18 additions & 5 deletions Xamarin.Forms.Platform.iOS/Renderers/ListViewRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,7 @@ public override UIView GetViewForHeader(UITableView tableView, nint section)
header.Cell = cell;

var renderer = (CellRenderer)Internals.Registrar.Registered.GetHandlerForObject<IRegisterable>(cell);
header.SetTableViewCell(renderer.GetCell(cell, null, tableView));
header.SetTableViewCell(renderer.GetCell(cell, header.GetTableViewCell(), tableView));

return header;
}
Expand All @@ -1173,8 +1173,13 @@ public override void HeaderViewDisplayingEnded(UITableView tableView, UIView hea

if (headerView is HeaderWrapperView wrapper)
{
wrapper.Cell?.SendDisappearing();
wrapper.Cell = null;
if (wrapper.Cell != null)
{
wrapper.Cell.SendDisappearing();
wrapper.SetTableViewCell(null);
wrapper.Cell.DisposeModalAndChildRenderers();
wrapper.Cell = null;
}
}
}

Expand Down Expand Up @@ -1514,10 +1519,18 @@ public HeaderWrapperView(string reuseIdentifier) : base((NSString)reuseIdentifie

public void SetTableViewCell(UITableViewCell value)
{
if (ReferenceEquals(_tableViewCell, value)) return;
if (ReferenceEquals(_tableViewCell, value))
return;

_tableViewCell?.RemoveFromSuperview();
_tableViewCell = value;
AddSubview(value);
if (value != null)
AddSubview(value);
}

public UITableViewCell GetTableViewCell()
{
return _tableViewCell;
}

public override void LayoutSubviews()
Expand Down