Skip to content

Commit 9f011e5

Browse files
CopilotPureWeen
authored andcommitted
Fix memory leak in CollectionViewHandler2.SubscribeToItemsLayoutPropertyChanged
Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
1 parent 6a2c09a commit 9f011e5

File tree

2 files changed

+75
-11
lines changed

2 files changed

+75
-11
lines changed

src/Controls/src/Core/Handlers/Items2/CollectionViewHandler2.iOS.cs

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#nullable disable
22
using System;
33
using System.Collections.Generic;
4+
using System.ComponentModel;
45
using System.Text;
56
using Foundation;
67
using Microsoft.Maui.Handlers;
@@ -63,10 +64,19 @@ public CollectionViewHandler2(PropertyMapper mapper = null) : base(mapper ?? Map
6364

6465
public partial class CollectionViewHandler2 : ItemsViewHandler2<ReorderableItemsView>
6566
{
67+
IItemsLayout _currentItemsLayout;
68+
PropertyChangedEventHandler _itemsLayoutPropertyChangedHandler;
69+
6670
// Reorderable
6771
protected override ItemsViewController2<ReorderableItemsView> CreateController(ReorderableItemsView itemsView, UICollectionViewLayout layout)
6872
=> new ReorderableItemsViewController2<ReorderableItemsView>(itemsView, layout);
6973

74+
protected override void DisconnectHandler(UIView platformView)
75+
{
76+
UnsubscribeFromItemsLayoutPropertyChanged();
77+
base.DisconnectHandler(platformView);
78+
}
79+
7080
public static void MapCanReorderItems(CollectionViewHandler2 handler, ReorderableItemsView itemsView)
7181
{
7282
(handler.Controller as ReorderableItemsViewController2<ReorderableItemsView>)?.UpdateCanReorderItems();
@@ -202,21 +212,41 @@ public static void MapItemSizingStrategy(CollectionViewHandler2 handler, Structu
202212

203213
void SubscribeToItemsLayoutPropertyChanged(IItemsLayout itemsLayout)
204214
{
215+
// Unsubscribe from the previous ItemsLayout if it exists
216+
UnsubscribeFromItemsLayoutPropertyChanged();
217+
205218
if (itemsLayout is not null)
206219
{
207-
itemsLayout.PropertyChanged += (sender, args) =>
220+
// Create the handler if it doesn't exist
221+
if (_itemsLayoutPropertyChangedHandler == null)
208222
{
209-
if (args.PropertyName == nameof(ItemsLayout.SnapPointsAlignment) ||
210-
args.PropertyName == nameof(ItemsLayout.SnapPointsType) ||
211-
args.PropertyName == nameof(GridItemsLayout.VerticalItemSpacing) ||
212-
args.PropertyName == nameof(GridItemsLayout.HorizontalItemSpacing) ||
213-
args.PropertyName == nameof(GridItemsLayout.Span) ||
214-
args.PropertyName == nameof(LinearItemsLayout.ItemSpacing))
215-
223+
_itemsLayoutPropertyChangedHandler = (sender, args) =>
216224
{
217-
UpdateLayout();
218-
}
219-
};
225+
if (args.PropertyName == nameof(ItemsLayout.SnapPointsAlignment) ||
226+
args.PropertyName == nameof(ItemsLayout.SnapPointsType) ||
227+
args.PropertyName == nameof(GridItemsLayout.VerticalItemSpacing) ||
228+
args.PropertyName == nameof(GridItemsLayout.HorizontalItemSpacing) ||
229+
args.PropertyName == nameof(GridItemsLayout.Span) ||
230+
args.PropertyName == nameof(LinearItemsLayout.ItemSpacing))
231+
232+
{
233+
UpdateLayout();
234+
}
235+
};
236+
}
237+
238+
// Subscribe to the new ItemsLayout
239+
itemsLayout.PropertyChanged += _itemsLayoutPropertyChangedHandler;
240+
_currentItemsLayout = itemsLayout;
241+
}
242+
}
243+
244+
void UnsubscribeFromItemsLayoutPropertyChanged()
245+
{
246+
if (_currentItemsLayout is not null && _itemsLayoutPropertyChangedHandler is not null)
247+
{
248+
_currentItemsLayout.PropertyChanged -= _itemsLayoutPropertyChangedHandler;
249+
_currentItemsLayout = null;
220250
}
221251
}
222252
}

src/Controls/tests/DeviceTests/Elements/CollectionView/CollectionViewTests.iOS.cs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,40 @@ public void IndexPathValidTest()
251251
Assert.False(source.IsIndexPathValid(invalidSection));
252252
}
253253

254+
[Fact(DisplayName = "CollectionView Unsubscribes From ItemsLayout PropertyChanged")]
255+
public async Task CollectionViewUnsubscribesFromItemsLayoutPropertyChanged()
256+
{
257+
SetupBuilder();
258+
259+
var collectionView = new CollectionView
260+
{
261+
ItemsSource = new List<string> { "Item 1", "Item 2", "Item 3" },
262+
ItemTemplate = new DataTemplate(() => new Label()),
263+
ItemsLayout = new LinearItemsLayout(ItemsLayoutOrientation.Vertical)
264+
};
265+
266+
await CreateHandlerAndAddToWindow<CollectionViewHandler2>(collectionView, async handler =>
267+
{
268+
// Verify handler is created
269+
Assert.NotNull(handler);
270+
271+
// Change the ItemsLayout to trigger a new subscription
272+
var newLayout = new LinearItemsLayout(ItemsLayoutOrientation.Horizontal);
273+
collectionView.ItemsLayout = newLayout;
274+
275+
// Give the handler time to process the change
276+
await Task.Delay(100);
277+
278+
// Disconnect the handler - this should unsubscribe from the ItemsLayout
279+
((IElementHandler)handler).DisconnectHandler();
280+
281+
// Verify that changing the ItemsLayout properties doesn't cause issues after disconnect
282+
// If the handler is not properly unsubscribed, this could cause a crash or memory leak
283+
newLayout.SnapPointsAlignment = SnapPointsAlignment.End;
284+
newLayout.SnapPointsType = SnapPointsType.Mandatory;
285+
});
286+
}
287+
254288
/// <summary>
255289
/// Simulates what a developer might do with a Page/View
256290
/// </summary>

0 commit comments

Comments
 (0)