Port CollectionView performance fixes from .NET MAUI (#15697)

Xamarin.Forms.Core.UnitTests/BindingUnitTests.cs

@@ -1,8 +1,10 @@
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.ComponentModel;
 using System.Globalization;
 using System.Linq;
+using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Threading.Tasks;
 using NUnit.Framework;
@@ -1237,6 +1239,82 @@ public void SourceAndTargetAreWeakComplexPath(BindingMode mode)
 				Assert.IsFalse(weakBindable.IsAlive, "Bindable wasn't collected");
 		}
 
+		[Category("[Binding] Complex paths")]
+		[TestCase(BindingMode.OneWay)]
+		[TestCase(BindingMode.OneWayToSource)]
+		[TestCase(BindingMode.TwoWay)]
+		public async Task WeakPropertyChangedProxyDoesNotLeak(BindingMode mode)
+		{
+			var proxies = new List<WeakReference>();
+			WeakReference weakViewModel = null, weakBindable = null;
+
+			int i = 0;
+			void create()
+			{
+				if (i++ < 1024)
+				{
+					create();
+					return;
+				}
+
+				var binding = new Binding("Model.Model[1]");
+				var bindable = new MockBindable();
+				weakBindable = new WeakReference(bindable);
+
+				var viewmodel = new ComplexMockViewModel
+				{
+					Model = new ComplexMockViewModel
+					{
+						Model = new ComplexMockViewModel()
+					}
+				};
+
+				weakViewModel = new WeakReference(viewmodel);
+
+				bindable.BindingContext = viewmodel;
+				bindable.SetBinding(MockBindable.TextProperty, binding);
+
+				// Access private members:
+				// WeakPropertyChangedProxy proxy = binding._expression._parts[i]._listener;
+				var flags = BindingFlags.NonPublic | BindingFlags.Instance;
+				var expression = binding.GetType().GetField("_expression", flags).GetValue(binding);
+				Assert.NotNull(expression);
+				var parts = expression.GetType().GetField("_parts", flags).GetValue(expression) as IEnumerable;
+				Assert.NotNull(parts);
+				foreach (var part in parts)
+				{
+					var listener = part.GetType().GetField("_listener", flags).GetValue(part);
+					if (listener == null)
+						continue;
+					proxies.Add(new WeakReference(listener));
+				}
+
+				Assert.IsNotEmpty(proxies); // Should be at least 1
+			};
+
+			create();
+
+			await Task.Yield();
+			GC.Collect();
+			GC.WaitForPendingFinalizers();
+
+			if (mode == BindingMode.TwoWay || mode == BindingMode.OneWay)
+				Assert.False(weakViewModel.IsAlive, "ViewModel wasn't collected");
+
+			if (mode == BindingMode.TwoWay || mode == BindingMode.OneWayToSource)
+				Assert.False(weakBindable.IsAlive, "Bindable wasn't collected");
+
+			// WeakPropertyChangedProxy won't go away until the second GC, BindingExpressionPart unsubscribes in its finalizer
+			await Task.Yield();
+			GC.Collect();
+			GC.WaitForPendingFinalizers();
+
+			foreach (var proxy in proxies)
+			{
+				Assert.False(proxy.IsAlive, "WeakPropertyChangedProxy wasn't collected");
+			}
+		}
+
 		// Mono doesn't handle the GC properly until the stack frame where the object is created is popped.
 		// This means calling another method and not just using lambda as works in real .NET
 		void HackAroundMonoSucking(int i, BindableProperty property, Binding binding, out WeakReference weakViewModel, out WeakReference weakBindable)

Xamarin.Forms.Core/BindableObject.cs

@@ -19,15 +19,15 @@ public virtual IDispatcher Dispatcher
 
 		readonly Dictionary<BindableProperty, BindablePropertyContext> _properties = new Dictionary<BindableProperty, BindablePropertyContext>(4);
 		bool _applying;
-		object _inheritedContext;
+		WeakReference _inheritedContext;
 
 		public static readonly BindableProperty BindingContextProperty =
 			BindableProperty.Create(nameof(BindingContext), typeof(object), typeof(BindableObject), default(object),
 									BindingMode.OneWay, null, BindingContextPropertyChanged, null, null, BindingContextPropertyBindingChanging);
 
 		public object BindingContext
 		{
-			get => _inheritedContext ?? GetValue(BindingContextProperty);
+			get => _inheritedContext?.Target ?? GetValue(BindingContextProperty);
 			set => SetValue(BindingContextProperty, value);
 		}
 
@@ -225,7 +225,7 @@ public static void SetInheritedBindingContext(BindableObject bindable, object va
 			if (bpContext != null && ((bpContext.Attributes & BindableContextAttributes.IsManuallySet) != 0))
 				return;
 
-			object oldContext = bindable._inheritedContext;
+			object oldContext = bindable._inheritedContext?.Target;
 
 			if (ReferenceEquals(oldContext, value))
 				return;
@@ -240,7 +240,7 @@ public static void SetInheritedBindingContext(BindableObject bindable, object va
 			}
 			else
 			{
-				bindable._inheritedContext = value;
+				bindable._inheritedContext = new WeakReference(value);
 			}
 
 			bindable.ApplyBindings(skipBindingContext: false, fromBindingContextChanged: true);
@@ -533,7 +533,7 @@ internal void ApplyBindings(bool skipBindingContext, bool fromBindingContextChan
 
 		static void BindingContextPropertyBindingChanging(BindableObject bindable, BindingBase oldBindingBase, BindingBase newBindingBase)
 		{
-			object context = bindable._inheritedContext;
+			object context = bindable._inheritedContext?.Target;
 			var oldBinding = oldBindingBase as Binding;
 			var newBinding = newBindingBase as Binding;
 

Xamarin.Forms.Core/BindingExpression.cs

@@ -652,7 +652,7 @@ public void SubscribeTo(INotifyPropertyChanged source, PropertyChangedEventHandl
 				_listener.SetTarget(listener);
 			}
 
-			public void Unsubscribe()
+			public void Unsubscribe(bool finalizer = false)
 			{
 				INotifyPropertyChanged source;
 				if (_source.TryGetTarget(out source) && source != null)
@@ -661,6 +661,10 @@ public void Unsubscribe()
 				if (bo != null)
 					bo.BindingContextChanged -= _bchandler;
 
+				// If we are called from a finalizer, WeakReference<T>.SetTarget() can throw InvalidOperationException
+				if (finalizer)
+					return;
+
 				_source.SetTarget(null);
 				_listener.SetTarget(null);
 			}
@@ -685,6 +689,8 @@ class BindingExpressionPart
 			readonly PropertyChangedEventHandler _changeHandler;
 			WeakPropertyChangedProxy _listener;
 
+			~BindingExpressionPart() => _listener?.Unsubscribe(finalizer: true);
+
 			public BindingExpressionPart(BindingExpression expression, string content, bool isIndexer = false)
 			{
 				_expression = expression;