Fix GradientBrush.GradientStops.Clear() leaks brushes when stops are shared

src/Controls/src/Core/GradientBrush.cs

@@ -1,5 +1,6 @@
 #nullable disable
 using System;
+using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.ComponentModel;
 
@@ -9,6 +10,8 @@
 	[ContentProperty(nameof(GradientStops))]
 	public abstract class GradientBrush : Brush
 	{
+		readonly Dictionary<GradientStop, int> _subscriptionRefCounts = new(ReferenceEqualityComparer.Instance);
+
 		/// <summary>Initializes a new instance of the <see cref="GradientBrush"/> class.</summary>
 		public GradientBrush()
 		{
@@ -42,64 +45,162 @@
 			base.OnBindingContextChanged();
 
 			foreach (var gradientStop in GradientStops)
+			{
 				SetInheritedBindingContext(gradientStop, BindingContext);
+			}
 		}
 
 		void UpdateGradientStops(GradientStopCollection oldCollection, GradientStopCollection newCollection)
 		{
-			if (oldCollection != null)
+			DetachCollection(oldCollection);
+			AttachCollection(newCollection);
+			Invalidate();
+		}
+
+		void AttachCollection(GradientStopCollection collection)
+		{
+			if (collection is null)
 			{
-				oldCollection.CollectionChanged -= OnGradientStopCollectionChanged;
+				return;
+			}
 
-				foreach (var oldStop in oldCollection)
-				{
-					oldStop.Parent = null;
-					oldStop.PropertyChanged -= OnGradientStopPropertyChanged;
-				}
+			collection.CollectionChanged += OnGradientStopCollectionChanged;
+
+			foreach (var stop in collection)
+			{
+				SubscribeToGradientStop(stop);
 			}
+		}
 
-			if (newCollection == null)
+		void DetachCollection(GradientStopCollection collection)
+		{
+			if (collection is null)
+			{
 				return;
+			}
 
-			newCollection.CollectionChanged += OnGradientStopCollectionChanged;
+			collection.CollectionChanged -= OnGradientStopCollectionChanged;
+			UnsubscribeFromAllGradientStops();
+		}
 
-			foreach (var newStop in newCollection)
+		void OnGradientStopCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
+		{
+			switch (e.Action)
 			{
-				if (newStop is not null)
-				{
-					newStop.Parent = this;
-					newStop.PropertyChanged += OnGradientStopPropertyChanged;
-				}
+				case NotifyCollectionChangedAction.Add:
+					if (e.NewItems is not null)
+					{
+						foreach (GradientStop stop in e.NewItems)
+						{
+							SubscribeToGradientStop(stop);
+						}
+					}
+					break;
+
+				case NotifyCollectionChangedAction.Remove:
+					if (e.OldItems is not null)
+					{
+						foreach (GradientStop stop in e.OldItems)
+						{
+							UnsubscribeFromGradientStop(stop);
+						}
+					}
+					break;
+
+				case NotifyCollectionChangedAction.Replace:
+					if (e.OldItems is not null)
+					{
+						foreach (GradientStop stop in e.OldItems)
+						{
+							UnsubscribeFromGradientStop(stop);
+						}
+					}
+					if (e.NewItems is not null)
+					{
+						foreach (GradientStop stop in e.NewItems)
+						{
+							SubscribeToGradientStop(stop);
+						}
+					}
+					break;
+
+				case NotifyCollectionChangedAction.Move:
+					// No subscription changes required.
+					break;
+
+				case NotifyCollectionChangedAction.Reset:
+					ResubscribeCollection(sender as GradientStopCollection);
+					break;
 			}
+
+			Invalidate();
 		}
 
-		void OnGradientStopCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
+		void SubscribeToGradientStop(GradientStop stop)
 		{
-			if (e.OldItems != null)
+			if (stop is null)
 			{
-				foreach (var oldItem in e.OldItems)
-				{
-					if (!(oldItem is GradientStop oldStop))
-						continue;
-
-					oldStop.Parent = null;
-					oldStop.PropertyChanged -= OnGradientStopPropertyChanged;
-				}
+				return;
 			}
 
-			if (e.NewItems != null)
+			if (_subscriptionRefCounts.TryGetValue(stop, out var count))
 			{
-				foreach (var newItem in e.NewItems)
-				{
-					if (!(newItem is GradientStop newStop))
-						continue;
-
-					newStop.Parent = this;
-					newStop.PropertyChanged += OnGradientStopPropertyChanged;
-				}
+				_subscriptionRefCounts[stop] = count + 1;
+				return;
 			}
 
-			Invalidate();
+			_subscriptionRefCounts[stop] = 1;
+			stop.Parent = this;
+			stop.PropertyChanged += OnGradientStopPropertyChanged;
+		}
+
+		void UnsubscribeFromGradientStop(GradientStop stop)
+		{
+			if (stop is null)
+			{
+				return;
+			}
+
+			if (!_subscriptionRefCounts.TryGetValue(stop, out var count))
+			{
+				return;
+			}
+
+			if (count > 1)
+			{
+				_subscriptionRefCounts[stop] = count - 1;
+				return;
+			}
+
+			_subscriptionRefCounts.Remove(stop);
+			stop.Parent = null;
+			stop.PropertyChanged -= OnGradientStopPropertyChanged;
+		}
+
+		void UnsubscribeFromAllGradientStops()
+		{
+			foreach (var stop in _subscriptionRefCounts.Keys)
+			{
+				stop.Parent = null;
+				stop.PropertyChanged -= OnGradientStopPropertyChanged;
+			}
+
+			_subscriptionRefCounts.Clear();
+		}
+
+		void ResubscribeCollection(GradientStopCollection collection)
+		{
+			UnsubscribeFromAllGradientStops();
+
+			if (collection is null)
+			{
+				return;
+			}
+
+			foreach (var stop in collection)
+			{
+				SubscribeToGradientStop(stop);
+			}
 		}
 
 		void OnGradientStopPropertyChanged(object sender, PropertyChangedEventArgs e)

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

@@ -237,5 +237,33 @@ public void TestHasTransparencyLinearGradientBrush()
 			};
 			Assert.True(Brush.HasTransparency(allSemiTransparentBrush));
 		}
+
+		[Fact]
+		public void ClearUnsubscribesPreviousGradientStopsFromPropertyChanged()
+		{
+			var brush = new LinearGradientBrush();
+			var oldStop = new GradientStop { Color = Colors.Red, Offset = 0.0f };
+			var newStop = new GradientStop { Color = Colors.Blue, Offset = 1.0f };
+			var invalidations = 0;
+
+			brush.InvalidateGradientBrushRequested += (_, _) => invalidations++;
+
+			brush.GradientStops.Add(oldStop);
+			invalidations = 0;
+
+			brush.GradientStops.Clear();
+			invalidations = 0;
+
+			// If Reset handling does not unsubscribe old stops, mutating oldStop
+			// would incorrectly invalidate the brush.
+			oldStop.Color = Colors.Green;
+			Assert.Equal(0, invalidations);
+
+			brush.GradientStops.Add(newStop);
+			invalidations = 0;
+
+			newStop.Color = Colors.Purple;
+			Assert.Equal(1, invalidations);
+		}
 	}
 }