Skip to content

[iOS] Review possible memory leak with CollectionViewHandler2.SubscribeToItemsLayoutPropertyChanged #29619

@PureWeen

Description

@PureWeen

Description

Reviewing some memory dumps and it seems like there's a possible leak with CollectionViewHandler2

Image

void SubscribeToItemsLayoutPropertyChanged(IItemsLayout itemsLayout)
{
if (itemsLayout is not null)
{
itemsLayout.PropertyChanged += (sender, args) =>
{
if (args.PropertyName == nameof(ItemsLayout.SnapPointsAlignment) ||
args.PropertyName == nameof(ItemsLayout.SnapPointsType) ||
args.PropertyName == nameof(GridItemsLayout.VerticalItemSpacing) ||
args.PropertyName == nameof(GridItemsLayout.HorizontalItemSpacing) ||
args.PropertyName == nameof(GridItemsLayout.Span) ||
args.PropertyName == nameof(LinearItemsLayout.ItemSpacing))
{
UpdateLayout();
}
};
}
}

I'm not sure off hand the most reasonable place to unsubscribe from this.
We could/should do it in disconnecthandler but it'd be nice to do it somewhere else that we always know it'll fire from.

Maybe we can unsubscribe/subscribe to this when the view is attached/removed from the window?

Maybe there's a way to subscribe to it from the level of the CollectionView itself and we have it fire a mapper that triggers the UpdateLayout call?

I'm a little curious why we're subscribing to events on a static variable, that feels like it won't do anything useful.

Workaround

This issue comes from the default ItemsLayout because a static instance which means if you subscribe to it you won't get GC'd

<CollectionView>
                <CollectionView.ItemsLayout>
                    <LinearItemsLayout Orientation="Vertical" />
                </CollectionView.ItemsLayout>
            </CollectionView>

Metadata

Metadata

Assignees

Labels

p/0Work that we can't release withouts/triagedIssue has been reviewed

Type

No type

Projects

Status

In Progress

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions