Use weak events for FormattedString span subscriptions

src/Controls/src/Core/FormattedString.cs

@@ -17,6 +17,14 @@ public class FormattedString : Element
 		readonly SpanCollection _spans = new SpanCollection();
 		readonly WeakEventManager _weakEventManager = new WeakEventManager();
 
+		// _spans is owned by this FormattedString, so subscribing to it directly cannot outlive us.
+		// Individual spans are a different matter: a Span the app holds on to -- or shares between
+		// FormattedStrings -- would root this instance through its PropertyChanged/PropertyChanging
+		// handlers, so those subscriptions go through weak proxies instead.
+		readonly Dictionary<Span, (WeakNotifyPropertyChangedProxy Changed, WeakNotifyPropertyChangingProxy Changing)> _subscribedSpans = new();
+		PropertyChangedEventHandler _itemPropertyChanged;
+		PropertyChangingEventHandler _itemPropertyChanging;
+
 		internal event NotifyCollectionChangedEventHandler SpansCollectionChanged
 		{
 			add => _weakEventManager.AddEventHandler(value, nameof(SpansCollectionChanged));
@@ -26,6 +34,15 @@ internal event NotifyCollectionChangedEventHandler SpansCollectionChanged
 		/// <summary>Initializes a new instance of the FormattedString class.</summary>
 		public FormattedString() => _spans.CollectionChanged += OnCollectionChanged;
 
+		~FormattedString()
+		{
+			foreach (var proxies in _subscribedSpans.Values)
+			{
+				proxies.Changed.Unsubscribe();
+				proxies.Changing.Unsubscribe();
+			}
+		}
+
 		protected override void OnBindingContextChanged()
 		{
 			base.OnBindingContextChanged();
@@ -53,8 +70,7 @@ void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
 					if (bo != null)
 					{
 						bo.Parent?.RemoveLogicalChild(bo);
-						bo.PropertyChanging -= OnItemPropertyChanging;
-						bo.PropertyChanged -= OnItemPropertyChanged;
+						DetachSpan(bo);
 					}
 
 				}
@@ -68,8 +84,7 @@ void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
 					if (bo != null)
 					{
 						this.AddLogicalChild(bo);
-						bo.PropertyChanging += OnItemPropertyChanging;
-						bo.PropertyChanged += OnItemPropertyChanged;
+						AttachSpan(bo);
 					}
 
 				}
@@ -79,6 +94,37 @@ void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
 			_weakEventManager.HandleEvent(sender, e, nameof(SpansCollectionChanged));
 		}
 
+		void AttachSpan(Span span)
+		{
+			// The same Span instance can legally appear more than once in the collection; subscribe once.
+			if (_subscribedSpans.ContainsKey(span))
+				return;
+
+			_itemPropertyChanged ??= OnItemPropertyChanged;
+			_itemPropertyChanging ??= OnItemPropertyChanging;
+
+			var changed = new WeakNotifyPropertyChangedProxy();
+			changed.Subscribe(span, _itemPropertyChanged);
+
+			var changing = new WeakNotifyPropertyChangingProxy();
+			changing.Subscribe(span, _itemPropertyChanging);
+
+			_subscribedSpans[span] = (changed, changing);
+		}
+
+		void DetachSpan(Span span)
+		{
+			// Only tear the subscription down once the last occurrence has gone.
+			if (_spans.Contains(span))
+				return;
+
+			if (_subscribedSpans.Remove(span, out var proxies))
+			{
+				proxies.Changed.Unsubscribe();
+				proxies.Changing.Unsubscribe();
+			}
+		}
+
 		void OnItemPropertyChanged(object sender, PropertyChangedEventArgs e) => OnPropertyChanged(nameof(Spans));
 
 		void OnItemPropertyChanging(object sender, PropertyChangingEventArgs e) => OnPropertyChanging(nameof(Spans));

src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt

@@ -25,3 +25,4 @@ override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.OnH
 override Microsoft.Maui.Controls.Handlers.Items.MauiRecyclerView<TItemsView, TAdapter, TItemsViewSource>.OnSizeChanged(int w, int h, int oldw, int oldh) -> void
 ~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void
 Microsoft.Maui.Controls.Label.~Label() -> void
+Microsoft.Maui.Controls.FormattedString.~FormattedString() -> void

src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt

@@ -16,3 +16,4 @@ override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void
 Microsoft.Maui.Controls.Label.~Label() -> void
 ~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.TraitCollectionDidChange(UIKit.UITraitCollection previousTraitCollection) -> void
 ~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.ViewWillTransitionToSize(CoreGraphics.CGSize toSize, UIKit.IUIViewControllerTransitionCoordinator coordinator) -> void
+Microsoft.Maui.Controls.FormattedString.~FormattedString() -> void

src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt

@@ -16,3 +16,4 @@ override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void
 Microsoft.Maui.Controls.Label.~Label() -> void
 ~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.TraitCollectionDidChange(UIKit.UITraitCollection previousTraitCollection) -> void
 ~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.ViewWillTransitionToSize(CoreGraphics.CGSize toSize, UIKit.IUIViewControllerTransitionCoordinator coordinator) -> void
+Microsoft.Maui.Controls.FormattedString.~FormattedString() -> void

src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt

@@ -5,3 +5,4 @@ override Microsoft.Maui.Controls.GraphicsView.OnBindingContextChanged() -> void
 override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void
 ~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void
 Microsoft.Maui.Controls.Label.~Label() -> void
+Microsoft.Maui.Controls.FormattedString.~FormattedString() -> void

src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt

@@ -7,3 +7,4 @@ override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void
 override Microsoft.Maui.Controls.Handlers.Items.CarouselViewHandler.UpdateEmptyViewVisibility() -> void
 ~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void
 Microsoft.Maui.Controls.Label.~Label() -> void
+Microsoft.Maui.Controls.FormattedString.~FormattedString() -> void

src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt

@@ -5,3 +5,4 @@ override Microsoft.Maui.Controls.GraphicsView.OnBindingContextChanged() -> void
 override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void
 ~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void
 Microsoft.Maui.Controls.Label.~Label() -> void
+Microsoft.Maui.Controls.FormattedString.~FormattedString() -> void

src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt

@@ -5,3 +5,4 @@ override Microsoft.Maui.Controls.GraphicsView.OnBindingContextChanged() -> void
 override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void
 ~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void
 Microsoft.Maui.Controls.Label.~Label() -> void
+Microsoft.Maui.Controls.FormattedString.~FormattedString() -> void

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

@@ -0,0 +1,116 @@
+using System;
+using System.Runtime.CompilerServices;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace Microsoft.Maui.Controls.Core.UnitTests
+{
+	// FormattedString owns its Spans collection, so subscribing to that collection cannot outlive it.
+	// A Span is different: the app can hold one, or share it between FormattedStrings, and a non-weak
+	// PropertyChanged/PropertyChanging subscription then roots the FormattedString.
+	public class FormattedStringWeakEventTests : BaseTestFixture
+	{
+		// https://github.com/dotnet/maui/issues/36289
+		[Fact]
+		public async Task RetainedSpanDoesNotRootFormattedString()
+		{
+			var shared = new Span { Text = "shared" };
+
+			var reference = CreateFormattedString(shared);
+
+			Assert.False(await reference.WaitForCollect(), "FormattedString should not be alive!");
+			GC.KeepAlive(shared);
+		}
+
+		// A span removed from the collection must stop notifying entirely.
+		[Fact]
+		public async Task RemovedSpanDoesNotRootFormattedString()
+		{
+			var shared = new Span { Text = "shared" };
+
+			var reference = CreateFormattedStringAndRemove(shared);
+
+			Assert.False(await reference.WaitForCollect(), "FormattedString should not be alive after removal!");
+			GC.KeepAlive(shared);
+		}
+
+		[Fact]
+		public void SpanChangesStillRaisePropertyChanged()
+		{
+			var span = new Span { Text = "a" };
+			var formatted = new FormattedString();
+			formatted.Spans.Add(span);
+
+			bool raised = false;
+			formatted.PropertyChanged += (s, e) =>
+			{
+				if (e.PropertyName == nameof(FormattedString.Spans))
+					raised = true;
+			};
+
+			span.Text = "b";
+
+			Assert.True(raised);
+			GC.KeepAlive(formatted);
+		}
+
+		[Fact]
+		public void RemovedSpanNoLongerRaisesPropertyChanged()
+		{
+			var span = new Span { Text = "a" };
+			var formatted = new FormattedString();
+			formatted.Spans.Add(span);
+			formatted.Spans.Remove(span);
+
+			bool raised = false;
+			formatted.PropertyChanged += (s, e) =>
+			{
+				if (e.PropertyName == nameof(FormattedString.Spans))
+					raised = true;
+			};
+
+			span.Text = "b";
+
+			Assert.False(raised);
+			GC.KeepAlive(formatted);
+		}
+
+		[Fact]
+		public void ClearDetachesSpans()
+		{
+			var span = new Span { Text = "a" };
+			var formatted = new FormattedString();
+			formatted.Spans.Add(span);
+			formatted.Spans.Clear();
+
+			bool raised = false;
+			formatted.PropertyChanged += (s, e) =>
+			{
+				if (e.PropertyName == nameof(FormattedString.Spans))
+					raised = true;
+			};
+
+			span.Text = "b";
+
+			Assert.False(raised);
+			GC.KeepAlive(formatted);
+		}
+
+		[MethodImpl(MethodImplOptions.NoInlining)]
+		static WeakReference CreateFormattedString(Span span)
+		{
+			var formatted = new FormattedString();
+			formatted.Spans.Add(span);
+			return new WeakReference(formatted);
+		}
+
+		[MethodImpl(MethodImplOptions.NoInlining)]
+		static WeakReference CreateFormattedStringAndRemove(Span span)
+		{
+			var formatted = new FormattedString();
+			formatted.Spans.Add(span);
+			formatted.Spans.Remove(span);
+			return new WeakReference(formatted);
+		}
+	}
+}