Skip to content

[leak-fix] Fix TransformGroup.Children memory leak (Fixes #36367) #36531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
github-actions wants to merge 1 commit into from
Closed

[leak-fix] Fix TransformGroup.Children memory leak (Fixes #36367) #36531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
183 changes: 100 additions & 83 deletions src/Controls/src/Core/Shapes/TransformGroup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ namespace Microsoft.Maui.Controls.Shapes
[ContentProperty("Children")]
public sealed class TransformGroup : Transform
{
readonly Dictionary<INotifyPropertyChanged, int> _subscribedTransforms = new();

/// <summary>Bindable property for <see cref="Children"/>.</summary>
public static readonly BindableProperty ChildrenProperty =
BindableProperty.Create(nameof(Children), typeof(TransformCollection), typeof(TransformGroup), null, propertyChanged: OnChildrenChanged);
BindableProperty.Create(nameof(Children), typeof(TransformCollection), typeof(TransformGroup), null,
propertyChanged: OnTransformGroupChanged);

/// <summary>
/// Initializes a new instance of the <see cref="TransformGroup"/> class.
Expand All @@ -34,138 +33,156 @@ public TransformCollection Children
get { return (TransformCollection)GetValue(ChildrenProperty); }
}

static void OnChildrenChanged(BindableObject bindable, object oldValue, object newValue)
static void OnTransformGroupChanged(BindableObject bindable, object oldValue, object newValue)
{
var transformGroup = (TransformGroup)bindable;
transformGroup.UpdateChildren(
oldValue as TransformCollection,
newValue as TransformCollection);
(bindable as TransformGroup)?.UpdateChildren(oldValue as TransformCollection, newValue as TransformCollection);
}

void UpdateChildren(TransformCollection oldCollection, TransformCollection newCollection)
{
DetachCollection(oldCollection);
AttachCollection(newCollection);

UpdateTransformMatrix();
}
ChildrenSubscriptions _childrenSubscriptions;
NotifyCollectionChangedEventHandler _childrenCollectionChanged;
PropertyChangedEventHandler _childPropertyChanged;

void AttachCollection(TransformCollection collection)
void UpdateChildren(TransformCollection oldCollection, TransformCollection newCollection)
{
if (collection is null)
if (oldCollection != null)
{
return;
_childrenSubscriptions?.UnsubscribeAll();
// Keep the empty helper for reuse; UnsubscribeAll releases every source and child proxy.
}

collection.CollectionChanged += OnChildrenCollectionChanged;

foreach (var transform in collection)
if (newCollection != null)
{
SubscribeToTransformPropertyChanged(transform);
}
}
_childrenCollectionChanged ??= OnChildrenCollectionChanged;
_childPropertyChanged ??= OnTransformPropertyChanged;

void DetachCollection(TransformCollection collection)
{
if (collection is null)
{
return;
var subscriptions = _childrenSubscriptions ??= new ChildrenSubscriptions();
subscriptions.Subscribe(newCollection, _childrenCollectionChanged, _childPropertyChanged);
}

collection.CollectionChanged -= OnChildrenCollectionChanged;

ClearAllTransformSubscriptions();
UpdateTransformMatrix();
}

void OnChildrenCollectionChanged(object sender, NotifyCollectionChangedEventArgs args)
{
if (args.Action == NotifyCollectionChangedAction.Reset)
{
ClearAllTransformSubscriptions();

if (sender is TransformCollection collection)
{
foreach (INotifyPropertyChanged item in collection)
{
SubscribeToTransformPropertyChanged(item);
}
}
_childrenSubscriptions?.ResetChildren();
}
else
else if (args.Action != NotifyCollectionChangedAction.Move)
{
if (args.OldItems is not null)
if (args.OldItems != null)
{
foreach (INotifyPropertyChanged item in args.OldItems)
foreach (var oldItem in args.OldItems)
{
UnsubscribeFromTransformPropertyChanged(item);
if (oldItem is Transform oldTransform)
{
_childrenSubscriptions?.Remove(oldTransform);
}
}
}

if (args.NewItems is not null)
if (args.NewItems != null)
{
foreach (INotifyPropertyChanged item in args.NewItems)
_childPropertyChanged ??= OnTransformPropertyChanged;

foreach (var newItem in args.NewItems)
{
SubscribeToTransformPropertyChanged(item);
if (newItem is Transform newTransform)
{
_childrenSubscriptions?.Add(newTransform, _childPropertyChanged);
}
}
}
}

UpdateTransformMatrix();
}

void SubscribeToTransformPropertyChanged(INotifyPropertyChanged item)
void OnTransformPropertyChanged(object sender, PropertyChangedEventArgs args)
{
if (_subscribedTransforms.TryGetValue(item, out int count))
UpdateTransformMatrix();
}

void UpdateTransformMatrix()
{
var matrix = new Matrix();

if (Children is not null)
{
_subscribedTransforms[item] = count + 1;
return;
foreach (var child in Children)
{
if (child is not null)
matrix = Matrix.Multiply(matrix, child.Value);
}
}

item.PropertyChanged += OnTransformPropertyChanged;
_subscribedTransforms[item] = 1;
Value = matrix;
}

void UnsubscribeFromTransformPropertyChanged(INotifyPropertyChanged item)
// Keeps the CollectionChanged and per-child PropertyChanged subscriptions weak so a shared
// or long-lived TransformCollection cannot root the TransformGroup. The finalizer tears the
// subscriptions down, mirroring the pattern used by other WeakEventProxy owners.
sealed class ChildrenSubscriptions
{
if (!_subscribedTransforms.TryGetValue(item, out int count))
readonly WeakNotifyCollectionChangedProxy _collectionProxy = new();
readonly List<WeakNotifyPropertyChangedProxy> _childProxies = new();

~ChildrenSubscriptions() => UnsubscribeAll();

public void Subscribe(
TransformCollection source,
NotifyCollectionChangedEventHandler collectionChangedHandler,
PropertyChangedEventHandler childPropertyChangedHandler)
{
return;
_collectionProxy.Subscribe(source, collectionChangedHandler);

foreach (var child in source)
{
if (child is not null)
{
Add(child, childPropertyChangedHandler);
}
}
}

if (count > 1)
public void Add(Transform source, PropertyChangedEventHandler handler)
{
_subscribedTransforms[item] = count - 1;
return;
_childProxies.Add(new WeakNotifyPropertyChangedProxy(source, handler));
}

item.PropertyChanged -= OnTransformPropertyChanged;
_subscribedTransforms.Remove(item);
}

// Unsubscribes all tracked transforms from PropertyChanged and clears the dictionary.
void ClearAllTransformSubscriptions()
{
foreach (var item in _subscribedTransforms)
public void Remove(Transform source)
{
item.Key.PropertyChanged -= OnTransformPropertyChanged;
for (int i = _childProxies.Count - 1; i >= 0; i--)
{
var proxy = _childProxies[i];
if (proxy.TryGetSource(out var proxySource) && ReferenceEquals(proxySource, source))
{
proxy.Unsubscribe();
_childProxies.RemoveAt(i);
break;
}
}
}

_subscribedTransforms.Clear();
}

void OnTransformPropertyChanged(object sender, PropertyChangedEventArgs args)
{
UpdateTransformMatrix();
}
public void ResetChildren()
{
// TransformCollection is sealed, so Reset means the current children were cleared.
UnsubscribeChildren();
}

void UpdateTransformMatrix()
{
var matrix = new Matrix();
public void UnsubscribeAll()
{
_collectionProxy.Unsubscribe();
UnsubscribeChildren();
}

foreach (Transform child in Children)
matrix = Matrix.Multiply(matrix, child.Value);
void UnsubscribeChildren()
{
for (int i = 0; i < _childProxies.Count; i++)
_childProxies[i].Unsubscribe();

Value = matrix;
_childProxies.Clear();
}
}
}
}
}
Loading
Loading