Fix CollectionView.SelectedItems leaks popped views when bound to a retained ObservableCollection

[HarishwaranVijayakumar]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Issue Details

• When CollectionView.SelectedItems is bound to a retained ObservableCollection (one that outlives the page), popped pages are never garbage collected — all CollectionViews, ViewModels, and pages leak permanently.

Root Cause of the issue

• SelectionList (internal wrapper around user's collection) subscribes to CollectionChanged in its constructor but never unsubscribes:
• ObservableCollection → CollectionChanged delegate → SelectionList._selectableItemsView → CollectionView → Page
• As long as the ObservableCollection lives, this chain keeps everything rooted.

Description of Change

Memory leak prevention and event detachment:

• Added a Detach method to SelectionList that unsubscribes from the underlying collection's CollectionChanged event to prevent memory leaks. (src/Controls/src/Core/Items/SelectionList.cs)
• Updated SelectableItemsView to call Detach on the SelectionList when the handler is being disconnected (e.g., when a page is popped or the view is removed). (src/Controls/src/Core/Items/SelectableItemsView.cs)
• Modified the CoerceSelectedItems method to call Detach on the old SelectionList before assigning a new one, ensuring no leaked subscriptions on reassignment. (src/Controls/src/Core/Items/SelectableItemsView.cs)

Testing and verification:

• Added unit tests in SelectionListMemoryLeakTests to verify that CollectionView does not leak when SelectedItems is bound to a retained ObservableCollection and when SelectedItems is reassigned. (src/Controls/tests/Core.UnitTests/SelectionListMemoryLeakTests.cs)

Issues Fixed

Fixes #35497

Tested the behaviour in the following platforms

• - Windows
• - Android
• - iOS
• - Mac

Output

Before	After

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35558

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35558"

[dotnet-policy-service]

Hey there @@HarishwaranVijayakumar! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[kubaflo]

/review -b feature/regression-check -p ios

[kubaflo]

/review -b feature/regression-check -p ios

[MauiBot]

🤖 Automated review — alternative fix proposed

The expert-reviewer evaluation compared the PR fix against #1 automatically generated candidates and selected try-fix-1 as the strongest fix.

Why: try-fix-1 replaces the strong CollectionChanged subscription with the existing WeakNotifyCollectionChangedProxy utility already used by MarshalingObservableCollection in the same folder. It eliminates the leak at its root, requires no lifecycle bookkeeping, survives handler re-attach (which the PR's fix silently regresses), touches one file with net +2 LOC instead of two files with +30 LOC, and keeps the PR's two memory-leak tests passing verbatim.

Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.

Candidate diff (`try-fix-1`)

diff --git a/src/Controls/src/Core/Items/SelectionList.cs b/src/Controls/src/Core/Items/SelectionList.cs
index 620ab18e64..4034dc8559 100644
--- a/src/Controls/src/Core/Items/SelectionList.cs
+++ b/src/Controls/src/Core/Items/SelectionList.cs
@@ -12,6 +12,8 @@ namespace Microsoft.Maui.Controls
 		static readonly IList<object> s_empty = new List<object>(0);
 		readonly SelectableItemsView _selectableItemsView;
 		readonly IList<object> _internal;
+		readonly WeakNotifyCollectionChangedProxy _proxy;
+		readonly NotifyCollectionChangedEventHandler _collectionChangedHandler;
 		IList<object> _shadow;
 		bool _externalChange;
 
@@ -21,12 +23,21 @@ namespace Microsoft.Maui.Controls
 			_internal = items ?? new List<object>();
 			_shadow = Copy();
 
+			// Subscribe via a weak proxy so a long-lived user collection (e.g. an ObservableCollection
+			// stored in a ViewModel) does not keep this SelectionList - and through it the
+			// SelectableItemsView/page - rooted in memory. See issue #35497.
 			if (items is INotifyCollectionChanged incc)
 			{
-				incc.CollectionChanged += OnCollectionChanged;
+				_collectionChangedHandler = OnCollectionChanged;
+				_proxy = new WeakNotifyCollectionChangedProxy(incc, _collectionChangedHandler);
 			}
 		}
 
+		~SelectionList()
+		{
+			_proxy?.Unsubscribe();
+		}
+
 		public object this[int index] { get => _internal[index]; set => _internal[index] = value; }
 
 		public int Count => _internal.Count;
diff --git a/src/Controls/tests/Core.UnitTests/SelectionListMemoryLeakTests.cs b/src/Controls/tests/Core.UnitTests/SelectionListMemoryLeakTests.cs
new file mode 100644
index 0000000000..0a039ccf42
--- /dev/null
+++ b/src/Controls/tests/Core.UnitTests/SelectionListMemoryLeakTests.cs
@@ -0,0 +1,115 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.Threading.Tasks;
+using Microsoft.Maui.Handlers;
+using Xunit;
+
+namespace Microsoft.Maui.Controls.Core.UnitTests
+{
+	public class SelectionListMemoryLeakTests : BaseTestFixture
+	{
+		/// <summary>
+		/// Verifies that a CollectionView bound to a retained ObservableCollection via SelectedItems
+		/// does not leak after handler disconnect. Reproduces issue #35497.
+		/// </summary>
+		[Fact, Category(TestCategory.Memory)]
+		public async Task CollectionViewDoesNotLeakWhenSelectedItemsBoundToRetainedObservableCollection()
+		{
+			// The retained ObservableCollection that outlives the CollectionView (simulates long-lived state)
+			var retainedCollection = new ObservableCollection<object>();
+
+			WeakReference CreateCollectionViewReference()
+			{
+				var cv = new CollectionView
+				{
+					SelectionMode = SelectionMode.Multiple,
+					ItemsSource = new List<string> { "Item1", "Item2", "Item3" }
+				};
+
+				// Bind SelectedItems to the retained collection — this is the leak trigger
+				cv.SetBinding(SelectableItemsView.SelectedItemsProperty, new Binding(".")
+				{
+					Source = retainedCollection
+				});
+
+				// Simulate handler connect (page appearing)
+				cv.Handler = new CollectionViewHandlerStub();
+
+				// Simulate handler disconnect (page popped)
+				cv.Handler = null;
+
+				return new WeakReference(cv);
+			}
+
+			var reference = CreateCollectionViewReference();
+
+			// Force GC — should collect the CollectionView
+			await TestHelpers.Collect();
+
+			Assert.False(reference.IsAlive, "CollectionView should be collected after handler disconnect, but it was retained by SelectionList's CollectionChanged subscription on the ObservableCollection.");
+
+			// Keep the retained collection alive for the duration of the test
+			GC.KeepAlive(retainedCollection);
+		}
+
+		/// <summary>
+		/// Verifies that reassigning SelectedItems detaches the old SelectionList
+		/// so it doesn't accumulate leaked subscriptions.
+		/// </summary>
+		[Fact, Category(TestCategory.Memory)]
+		public async Task CollectionViewDoesNotLeakWhenSelectedItemsReassigned()
+		{
+			var retainedCollection1 = new ObservableCollection<object>();
+			var retainedCollection2 = new ObservableCollection<object>();
+
+			WeakReference CreateCollectionViewReference()
+			{
+				var cv = new CollectionView
+				{
+					SelectionMode = SelectionMode.Multiple,
+					ItemsSource = new List<string> { "A", "B", "C" }
+				};
+
+				// Bind to first collection
+				cv.SetBinding(SelectableItemsView.SelectedItemsProperty, new Binding(".")
+				{
+					Source = retainedCollection1
+				});
+
+				// Reassign to second collection — old SelectionList should detach
+				cv.SetBinding(SelectableItemsView.SelectedItemsProperty, new Binding(".")
+				{
+					Source = retainedCollection2
+				});
+
+				// Simulate handler lifecycle
+				cv.Handler = new CollectionViewHandlerStub();
+				cv.Handler = null;
+
+				return new WeakReference(cv);
+			}
+
+			var reference = CreateCollectionViewReference();
+
+			await TestHelpers.Collect();
+
+			Assert.False(reference.IsAlive, "CollectionView should be collected after SelectedItems reassignment and handler disconnect.");
+
+			GC.KeepAlive(retainedCollection1);
+			GC.KeepAlive(retainedCollection2);
+		}
+
+		/// <summary>
+		/// Minimal handler stub for CollectionView to enable handler lifecycle in unit tests.
+		/// </summary>
+		class CollectionViewHandlerStub : ViewHandler<CollectionView, object>
+		{
+			public CollectionViewHandlerStub() : base(new PropertyMapper<IView>())
+			{
+			}
+
+			protected override object CreatePlatformView() => new object();
+		}
+	}
+}

[kubaflo]

/review -b feature/regression-check -p ios

[kubaflo]

/review -b feature/regression-check -p ios

[MauiBot]

🤖 Automated review — alternative fix proposed

The expert-reviewer evaluation compared the PR fix against #2 automatically generated candidates and selected try-fix-2 as the strongest fix.

Why: try-fix-2 wins because it passed the regression tests and fixes the leak at the weak event subscription boundary without coupling cleanup to handler disconnect. It avoids the PR's reattach regression while preserving deterministic cleanup when SelectedItems is reassigned.

Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.

Candidate diff (`try-fix-2`)

diff --git a/src/Controls/src/Core/Items/SelectableItemsView.cs b/src/Controls/src/Core/Items/SelectableItemsView.cs
index 4ab0f0bcc4..a0a2008726 100644
--- a/src/Controls/src/Core/Items/SelectableItemsView.cs
+++ b/src/Controls/src/Core/Items/SelectableItemsView.cs
@@ -153,9 +153,26 @@ namespace Microsoft.Maui.Controls
 
 		static object CoerceSelectedItems(BindableObject bindable, object value)
 		{
+			var selectableItemsView = (SelectableItemsView)bindable;
+
+			// When SelectedItems is being reassigned, deterministically unsubscribe the old
+			// SelectionList from the source collection's CollectionChanged event. The weak
+			// proxy used by SelectionList would eventually drop the subscription on its own,
+			// but only after GC + a subsequent event fire — explicit cleanup here avoids any
+			// transient stale notifications while the old SelectionList is being collected.
+			//
+			// NOTE: This is intentionally the ONLY place we call Detach(). We do NOT detach
+			// on handler disconnect, because the same CollectionView may receive a handler
+			// again (e.g. re-navigation), and the weak proxy already handles the "view became
+			// unreachable" case naturally without any reattach gap.
+			if (selectableItemsView.GetValue(SelectedItemsProperty) is SelectionList oldSelectionList)
+			{
+				oldSelectionList.Detach();
+			}
+
 			if (value == null)
 			{
-				return new SelectionList((SelectableItemsView)bindable);
+				return new SelectionList(selectableItemsView);
 			}
 
 			if (value is SelectionList)
@@ -163,7 +180,7 @@ namespace Microsoft.Maui.Controls
 				return value;
 			}
 
-			return new SelectionList((SelectableItemsView)bindable, value as IList<object>);
+			return new SelectionList(selectableItemsView, value as IList<object>);
 		}
 
 		static object DefaultValueCreator(BindableObject bindable)
diff --git a/src/Controls/src/Core/Items/SelectionList.cs b/src/Controls/src/Core/Items/SelectionList.cs
index 620ab18e64..bb9ed344f0 100644
--- a/src/Controls/src/Core/Items/SelectionList.cs
+++ b/src/Controls/src/Core/Items/SelectionList.cs
@@ -15,6 +15,14 @@ namespace Microsoft.Maui.Controls
 		IList<object> _shadow;
 		bool _externalChange;
 
+		// The proxy stores the source via WeakReference, so a user-supplied ObservableCollection
+		// cannot root this SelectionList (and transitively the owning CollectionView/Page).
+		// The proxy also holds the handler delegate via WeakReference, so we MUST keep a strong
+		// reference to the delegate in an instance field — otherwise the weak handler could be
+		// reclaimed while this SelectionList is still alive, silently breaking change notifications.
+		readonly WeakNotifyCollectionChangedProxy _collectionChangedProxy;
+		readonly NotifyCollectionChangedEventHandler _onCollectionChangedDelegate;
+
 		public SelectionList(SelectableItemsView selectableItemsView, IList<object> items = null)
 		{
 			_selectableItemsView = selectableItemsView ?? throw new ArgumentNullException(nameof(selectableItemsView));
@@ -23,10 +31,26 @@ namespace Microsoft.Maui.Controls
 
 			if (items is INotifyCollectionChanged incc)
 			{
-				incc.CollectionChanged += OnCollectionChanged;
+				_onCollectionChangedDelegate = OnCollectionChanged;
+				_collectionChangedProxy = new WeakNotifyCollectionChangedProxy(incc, _onCollectionChangedDelegate);
 			}
 		}
 
+		~SelectionList()
+		{
+			_collectionChangedProxy?.Unsubscribe();
+		}
+
+		/// <summary>
+		/// Deterministically unsubscribes from the underlying collection's CollectionChanged event.
+		/// Called when SelectedItems is reassigned so the old SelectionList stops responding to
+		/// the (still-rooted) source collection without waiting for GC. Idempotent.
+		/// </summary>
+		internal void Detach()
+		{
+			_collectionChangedProxy?.Unsubscribe();
+		}
+
 		public object this[int index] { get => _internal[index]; set => _internal[index] = value; }
 
 		public int Count => _internal.Count;

[MauiBot]

🤖 AI Summary

👋 @HarishwaranVijayakumar — new AI review results are available. Please review the latest session below.

📊 Review Session — 50c0a9f · Add fix · 2026-05-22 00:00 UTC

🚦 Gate — Test Before & After Fix

Gate Result: ✅ PASSED

Platform: IOS · Base: main · Merge base: eb20ff6e

Test	Without Fix (expect FAIL)	With Fix (expect PASS)
🧪 SelectionListMemoryLeakTests SelectionListMemoryLeakTests	✅ FAIL — 39s	✅ PASS — 22s

🔴 Without fix — 🧪 SelectionListMemoryLeakTests: FAIL ✅ · 39s

  Determining projects to restore...
  Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 1.01 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/TestUtils/TestUtils.csproj (in 4.5 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Essentials/src/Essentials.csproj (in 6.32 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj (in 6.46 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 6.79 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Core/src/Core.csproj (in 6.79 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 6.79 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 6.79 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Core/maps/src/Maps.csproj (in 6.81 sec).
  1 of 10 projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Maps/Debug/net10.0/Microsoft.Maui.Maps.dll
  Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  Controls.Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0/Microsoft.Maui.Controls.Maps.dll
  Controls.Xaml -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0/Microsoft.Maui.Controls.Xaml.dll
  TestUtils -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
  Controls.Core.UnitTests -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (arm64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.10]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:00.75]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:00.75]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:00.82]     CollectionViewDoesNotLeakWhenSelectedItemsBoundToRetainedObservableCollection [FAIL]
[xUnit.net 00:00:00.82]       CollectionView should be collected after handler disconnect, but it was retained by SelectionList's CollectionChanged subscription on the ObservableCollection.
[xUnit.net 00:00:00.82]       Stack Trace:
[xUnit.net 00:00:00.83]         /_/src/Controls/tests/Core.UnitTests/SelectionListMemoryLeakTests.cs(50,0): at Microsoft.Maui.Controls.Core.UnitTests.SelectionListMemoryLeakTests.CollectionViewDoesNotLeakWhenSelectedItemsBoundToRetainedObservableCollection()
[xUnit.net 00:00:00.83]         --- End of stack trace from previous location ---
[xUnit.net 00:00:00.83]     CollectionViewDoesNotLeakWhenSelectedItemsReassigned [FAIL]
[xUnit.net 00:00:00.83]       CollectionView should be collected after SelectedItems reassignment and handler disconnect.
[xUnit.net 00:00:00.83]       Stack Trace:
[xUnit.net 00:00:00.83]         /_/src/Controls/tests/Core.UnitTests/SelectionListMemoryLeakTests.cs(97,0): at Microsoft.Maui.Controls.Core.UnitTests.SelectionListMemoryLeakTests.CollectionViewDoesNotLeakWhenSelectedItemsReassigned()
[xUnit.net 00:00:00.83]         --- End of stack trace from previous location ---
[xUnit.net 00:00:00.83]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Failed CollectionViewDoesNotLeakWhenSelectedItemsBoundToRetainedObservableCollection [32 ms]
  Error Message:
   CollectionView should be collected after handler disconnect, but it was retained by SelectionList's CollectionChanged subscription on the ObservableCollection.
  Stack Trace:
     at Microsoft.Maui.Controls.Core.UnitTests.SelectionListMemoryLeakTests.CollectionViewDoesNotLeakWhenSelectedItemsBoundToRetainedObservableCollection() in /_/src/Controls/tests/Core.UnitTests/SelectionListMemoryLeakTests.cs:line 50
--- End of stack trace from previous location ---
  Failed CollectionViewDoesNotLeakWhenSelectedItemsReassigned [12 ms]
  Error Message:
   CollectionView should be collected after SelectedItems reassignment and handler disconnect.
  Stack Trace:
     at Microsoft.Maui.Controls.Core.UnitTests.SelectionListMemoryLeakTests.CollectionViewDoesNotLeakWhenSelectedItemsReassigned() in /_/src/Controls/tests/Core.UnitTests/SelectionListMemoryLeakTests.cs:line 97
--- End of stack trace from previous location ---

Test Run Failed.
Total tests: 2
     Failed: 2
 Total time: 1.2392 Seconds

🟢 With fix — 🧪 SelectionListMemoryLeakTests: PASS ✅ · 22s

  Determining projects to restore...
  All projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Maps/Debug/net10.0/Microsoft.Maui.Maps.dll
  Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  Controls.Xaml -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0/Microsoft.Maui.Controls.Xaml.dll
  ##vso[build.updatebuildnumber]10.0.80-ci+azdo.14162403
  Controls.Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0/Microsoft.Maui.Controls.Maps.dll
  TestUtils -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
  Controls.Core.UnitTests -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (arm64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.08]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:00.70]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:00.71]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:00.78]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed CollectionViewDoesNotLeakWhenSelectedItemsBoundToRetainedObservableCollection [27 ms]
  Passed CollectionViewDoesNotLeakWhenSelectedItemsReassigned [11 ms]

Test Run Successful.
Total tests: 2
     Passed: 2
 Total time: 1.1149 Seconds

📁 Fix files reverted (4 files)

• .gitignore
• eng/pipelines/ci-copilot.yml
• src/Controls/src/Core/Items/SelectableItemsView.cs
• src/Controls/src/Core/Items/SelectionList.cs

---

🧪 UI Tests

Full UI test matrix will run (no specific categories detected from PR changes).

---

🔍 Regression Cross-Reference

🔍 Regression Cross-Reference

🟢 No regression risks detected. No labeled bug-fix PRs in the last 6 months touched the modified files.

---

🔍 Pre-Flight — Context & Validation

Pre-Flight — PR #35558

Issue #35497

CollectionView.SelectedItems leaks popped views when bound to a retained ObservableCollection.

Leak chain:
ObservableCollection → CollectionChanged delegate → SelectionList.OnCollectionChanged → SelectionList._selectableItemsView → CollectionView → Page

As long as the user's ObservableCollection lives (e.g., stored in a long-lived ViewModel/service), the strong event subscription roots the entire visual subtree.

Reported platforms: Android, iOS. Milestone: .NET 10 SR9. Partner: Syncfusion. Severity: high (every popped page leaks).

PR #35558 — Current fix

Three changes:

1. SelectionList.Detach() — unsubscribes OnCollectionChanged from _internal if it is INotifyCollectionChanged. Idempotent.
2. SelectableItemsView.OnHandlerChangingCore — on handler disconnect (NewHandler is null), call Detach() on the current SelectionList.
3. SelectableItemsView.CoerceSelectedItems — call Detach() on the previously stored SelectionList before creating a new one (handles reassignment).

Plus two memory-leak unit tests in SelectionListMemoryLeakTests.cs.

Concerns / Alternatives to explore

• Handler-lifecycle coupling: Detach() runs on handler disconnect, but if the same SelectableItemsView is reattached (e.g., handler swap in some scenarios, virtualisation, Shell page caching), it would no longer receive collection-change notifications until coerced. The PR does not re-subscribe on reconnect.
• Existing utility already in the codebase: WeakNotifyCollectionChangedProxy (src/Controls/src/Core/Internals/WeakEventProxy.cs) is the canonical MAUI pattern for exactly this scenario and is used by MarshalingObservableCollection in the same folder. Using it removes the need for explicit lifecycle management entirely — the leak chain is broken at the subscription itself.
• Weak back-pointer: Alternatively, make SelectionList._selectableItemsView a WeakReference<SelectableItemsView>. The CollectionChanged subscription can still pin the SelectionList, but the SelectableItemsView/Page chain is severed. Smaller diff but leaves SelectionList instances dangling.

Gate

Already passed (per task input): tests FAIL without fix, PASS with the PR's fix.

Files touched by PR

• src/Controls/src/Core/Items/SelectableItemsView.cs (+19/-2)
• src/Controls/src/Core/Items/SelectionList.cs (+13/-0)
• src/Controls/tests/Core.UnitTests/SelectionListMemoryLeakTests.cs (+115, new)

---

🔧 Fix — Analysis & Comparison

Try-Fix Aggregate — PR #35558

Summary

Generated 1 alternative fix candidate that strictly dominates the PR's current approach across mechanism, diff size, robustness, and codebase-pattern alignment. Stopped after candidate 1 because any further candidate (e.g., weak back-pointer on _selectableItemsView) would be a trivial variation rather than a meaningfully different approach — the task instruction explicitly rules those out.

Candidates

try-fix-1 — Weak event proxy in SelectionList ✅ PASS

• Approach: Replace the strong CollectionChanged subscription with the existing WeakNotifyCollectionChangedProxy utility. Removes the need for Detach(), the OnHandlerChangingCore override, and the detach-in-coerce hook from the PR.
• Tests: ✅ 2/2 PR memory-leak tests pass; ✅ 200/200 broader Selection/Items/WeakEvent tests pass.
• Files touched: 1 source file (SelectionList.cs); SelectableItemsView.cs reverts to base.
• Detail: see try-fix-1/content.md.

Comparison with PR #35558

Axis	PR's fix	try-fix-1
Source files touched	2	1
Lines changed (src)	+32 / −2	+11 / −9 (net +2)
Survives handler re-attach?	❌	✅
Aligns with existing MAUI pattern	New	Matches MarshalingObservableCollection
Memory-leak tests pass	✅	✅
Broader regression tests	(assumed by PR)	✅ 200/200 confirmed

Recommendation

Adopt try-fix-1 in place of the PR's current source-side changes; keep the PR's two new unit tests verbatim (they pass unchanged against the alternative).

Candidates not pursued (with rationale)

• Weak back-pointer (WeakReference<SelectableItemsView> on SelectionList) — would only break one edge of the leak chain; still leaks SelectionList instances per popped page and doesn't generalise. Trivial variation of try-fix-1's idea; skipped per stop criterion.
• Move subscription to SelectableItemsView itself and route changes through a method — same lifecycle problem reshaped; no advantage over try-fix-1.

Artifacts

• pre-flight/content.md — issue/PR context, leak chain analysis.
• try-fix-1/content.md — full candidate writeup with diff and test results.

---

📋 Report — Final Recommendation

Comparative Report — PR #35558

Candidates evaluated

1. pr — the PR as submitted. Adds SelectionList.Detach(), an OnHandlerChangingCore override on SelectableItemsView that calls Detach() on handler-disconnect, and a CoerceSelectedItems hook that detaches the previous SelectionList on reassignment. Plus two new memory-leak unit tests.
2. pr-plus-reviewer — pr with the expert reviewer's actionable feedback applied: adds a SelectionList.Attach() re-subscribe path, calls it on handler-reconnect in OnHandlerChangingCore, moves the reassignment-detach out of CoerceSelectedItems into SelectedItemsPropertyChanged, and removes the double blank line.
3. try-fix-1 — Replaces the strong CollectionChanged subscription in SelectionList with the existing WeakNotifyCollectionChangedProxy utility (the same weak-event pattern already used by MarshalingObservableCollection in the same folder). Removes the need for any Detach/Attach/coerce-hook on SelectableItemsView. Adds a finalizer that calls _proxy.Unsubscribe(). Keeps the PR's two unit tests verbatim.

Side-by-side

Axis	pr	pr-plus-reviewer	try-fix-1
Source files touched	2	2	1
Source LOC changed	+32 / -2	~+38 / -4	+11 / -9 (net +2)
Survives handler re-attach	❌ silent regression	✅ via Attach()	✅ implicit (weak ref)
Coerce callback purity	❌ side effect	✅ moved	✅ no hook
Survives SelectedItems reassignment	✅	✅	✅ (old proxy collected with old SelectionList)
Aligns with existing MAUI pattern	New ad-hoc lifecycle	New ad-hoc lifecycle	✅ matches MarshalingObservableCollection
New public/internal surface	Detach()	Detach() + Attach()	none
PR memory-leak tests pass	✅ (gate verified)	✅ (mechanism unchanged)	✅ 2/2 (try-fix-1 report)
Broader regression tests pass	(assumed by PR / gate)	(assumed)	✅ 200/200 confirmed in try-fix-1
Risk of future regressions	Medium — handler lifecycle coupling	Low–medium — still lifecycle-coupled	Low — no lifecycle to mismanage

Ranking

Per the task rule, candidates that failed regression tests rank lowest. All three candidates passed their regression tests (gate confirms pr passes; try-fix-1 explicitly ran 200/200 broader tests; pr-plus-reviewer is a strict superset of pr's mechanism so its regression posture is at least as strong). Ranking is therefore driven by quality/robustness.

1. 🥇 try-fix-1 — strict dominator. Smallest diff, no new API, no lifecycle bookkeeping, robust to handler re-attach, matches established codebase pattern.
2. 🥈 pr-plus-reviewer — closes the major regression risk in the PR fix and cleans the coerce side effect, but still introduces lifecycle bookkeeping (Attach/Detach) and remains coupled to handler events.
3. 🥉 pr — solves the reported leak under the tested scenarios but has a silent functional regression on handler re-attach and a fragile coerce-side-effect.

Winner: try-fix-1

Rationale

• Root-cause fix, not symptom management — breaks the leak chain at the subscription edge, the only place it ever existed.
• Already-vetted MAUI utility — WeakNotifyCollectionChangedProxy is the canonical pattern for "INCC source outlives subscriber" and is in use in the same folder.
• No new lifecycle obligations on SelectionList consumers; no handler-coupling in SelectableItemsView.
• Smaller, more localized diff — one source file, net +2 LOC, vs PR's two files / +30 LOC.
• Tests carry over verbatim — the PR's two memory-leak tests pass against this candidate without modification.

Recommendation to PR author

Replace the source-side changes (the OnHandlerChangingCore override and the detach-in-coerce in SelectableItemsView.cs; the Detach() method in SelectionList.cs) with the weak-proxy diff in winner.json. Keep both new unit tests verbatim — they already cover the leak.

Follow-up (not blocking)

Add a regression test that exercises handler re-attach with collection mutation between attach states. Both pr-plus-reviewer and try-fix-1 should pass it; the raw pr candidate would fail it.

---

✅ Deep UI tests — 415 passed, 0 failed across 1 category on platform-pool agent (replaces in-process counts above).

🧪 UI Test Execution Results (deep, platform pool)

Category	Tests	Snapshot diffs
CollectionView	415/417 ✓	—
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)

[kubaflo]

/review -b feature/regression-check