[controls] fix memory leak in VisualElement.Clip

src/Controls/src/Core/VisualElement.cs

@@ -98,34 +98,61 @@ public partial class VisualElement : NavigableElement, IAnimatable, IVisualEleme
 
 		void NotifyClipChanges()
 		{
-			if (Clip != null)
+			var clip = Clip;
+			if (clip != null)
 			{
-				Clip.PropertyChanged += OnClipChanged;
-
-				if (Clip is GeometryGroup geometryGroup)
-					geometryGroup.InvalidateGeometryRequested += InvalidateGeometryRequested;
+				var proxy = _clipProxy ??= new();
+				proxy.Subscribe(clip, (sender, e) => OnPropertyChanged(nameof(Clip)));
 			}
 		}
 
 		void StopNotifyingClipChanges()
 		{
-			if (Clip != null)
+			_clipProxy?.Unsubscribe();
+		}
+
+		class WeakClipChangedProxy : WeakEventProxy<Geometry, EventHandler>
+		{
+			void OnClipChanged(object sender, EventArgs e)
 			{
-				Clip.PropertyChanged -= OnClipChanged;
+				if (TryGetHandler(out var handler))
+				{
+					handler(sender, e);
+				}
+				else
+				{
+					Unsubscribe();
+				}
+			}
+
+			public override void Subscribe(Geometry source, EventHandler handler)
+			{
+				if (TryGetSource(out var s))
+				{
+					s.PropertyChanged -= OnClipChanged;
 
-				if (Clip is GeometryGroup geometryGroup)
-					geometryGroup.InvalidateGeometryRequested -= InvalidateGeometryRequested;
+					if (s is GeometryGroup g)
+						g.InvalidateGeometryRequested -= OnClipChanged;
+				}
+
+				source.PropertyChanged += OnClipChanged;
+				if (source is GeometryGroup geometryGroup)
+					geometryGroup.InvalidateGeometryRequested += OnClipChanged;
+
+				base.Subscribe(source, handler);
 			}
-		}
 
-		void OnClipChanged(object sender, PropertyChangedEventArgs e)
-		{
-			OnPropertyChanged(nameof(Clip));
-		}
+			public override void Unsubscribe()
+			{
+				if (TryGetSource(out var s))
+				{
+					s.PropertyChanged -= OnClipChanged;
 
-		void InvalidateGeometryRequested(object sender, EventArgs e)
-		{
-			OnPropertyChanged(nameof(Clip));
+					if (s is GeometryGroup g)
+						g.InvalidateGeometryRequested -= OnClipChanged;
+				}
+				base.Unsubscribe();
+			}
 		}
 
 		/// <include file="../../docs/Microsoft.Maui.Controls/VisualElement.xml" path="//Member[@MemberName='VisualProperty']/Docs/*" />
@@ -247,9 +274,14 @@ static void OnTransformChanged(BindableObject bindable, object oldValue, object
 					(bindable as VisualElement)?.NotifyBackgroundChanges();
 			});
 
-		readonly WeakBackgroundChangedProxy _backgroundProxy = new();
+		WeakBackgroundChangedProxy _backgroundProxy = null;
+		WeakClipChangedProxy _clipProxy = null;
 
-		~VisualElement() => _backgroundProxy.Unsubscribe();
+		~VisualElement()
+		{
+			_clipProxy?.Unsubscribe();
+			_backgroundProxy?.Unsubscribe();
+		}
 
 		void NotifyBackgroundChanges()
 		{
@@ -260,7 +292,8 @@ void NotifyBackgroundChanges()
 			if (background != null)
 			{
 				SetInheritedBindingContext(background, BindingContext);
-				_backgroundProxy.Subscribe(background, (sender, e) => OnPropertyChanged(nameof(Background)));
+				var proxy = _backgroundProxy ??= new();
+				proxy.Subscribe(background, (sender, e) => OnPropertyChanged(nameof(Background)));
 			}
 		}
 
@@ -273,7 +306,7 @@ void StopNotifyingBackgroundChanges()
 			if (background != null)
 			{
 				SetInheritedBindingContext(background, null);
-				_backgroundProxy.Unsubscribe();
+				_backgroundProxy?.Unsubscribe();
 			}
 		}
 

src/Controls/tests/Core.UnitTests/VisualElementTests.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Threading.Tasks;
+using Microsoft.Maui.Controls.Shapes;
 using Microsoft.Maui.Graphics;
 using Microsoft.Maui.Primitives;
 using Xunit;
@@ -142,5 +143,37 @@ public void GradientBrushSubscribed()
 			gradient.GradientStops.Add(new GradientStop(Colors.CornflowerBlue, 1));
 			Assert.True(fired, "PropertyChanged did not fire!");
 		}
+
+		[Theory]
+		[InlineData(typeof(RectangleGeometry))]
+		[InlineData(typeof(EllipseGeometry))]
+		public async Task ClipDoesNotLeak(Type type)
+		{
+			var geometry = (Geometry)Activator.CreateInstance(type);
+			var reference = new WeakReference(new VisualElement { Clip = geometry });
+
+			await Task.Yield();
+			GC.Collect();
+			GC.WaitForPendingFinalizers();
+
+			Assert.False(reference.IsAlive, "VisualElement should not be alive!");
+		}
+
+		[Fact]
+		public void RectangleGeometrySubscribed()
+		{
+			var geometry = new RectangleGeometry();
+			var visual = new VisualElement { Clip = geometry };
+
+			bool fired = false;
+			visual.PropertyChanged += (sender, e) =>
+			{
+				if (e.PropertyName == nameof(VisualElement.Clip))
+					fired = true;
+			};
+
+			geometry.Rect = new Rect(1, 2, 3, 4);
+			Assert.True(fired, "PropertyChanged did not fire!");
+		}
 	}
 }