diff --git a/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt index 9e3830e2ecfa..2c954d25d2a7 100644 --- a/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt @@ -21,4 +21,5 @@ override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.OnH override Microsoft.Maui.Controls.SwipeItemView.IsEnabledCore.get -> bool ~override Microsoft.Maui.Controls.RadioButton.OnPropertyChanged(string propertyName = null) -> void override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void +~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void Microsoft.Maui.Controls.Label.~Label() -> void diff --git a/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt index 3d09cf130b8e..17ec31bb6eea 100644 --- a/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt @@ -15,4 +15,5 @@ override Microsoft.Maui.Controls.SwipeItemView.IsEnabledCore.get -> bool ~override Microsoft.Maui.Controls.RadioButton.OnPropertyChanged(string propertyName = null) -> void override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void override Microsoft.Maui.Controls.Handlers.Items2.StructuredItemsViewController2.UpdateFlowDirection() -> void +~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void Microsoft.Maui.Controls.Label.~Label() -> void diff --git a/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt index 3d09cf130b8e..17ec31bb6eea 100644 --- a/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt @@ -15,4 +15,5 @@ override Microsoft.Maui.Controls.SwipeItemView.IsEnabledCore.get -> bool ~override Microsoft.Maui.Controls.RadioButton.OnPropertyChanged(string propertyName = null) -> void override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void override Microsoft.Maui.Controls.Handlers.Items2.StructuredItemsViewController2.UpdateFlowDirection() -> void +~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void Microsoft.Maui.Controls.Label.~Label() -> void diff --git a/src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt index a46a926c776a..5dd193844cfa 100644 --- a/src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt @@ -3,4 +3,5 @@ override Microsoft.Maui.Controls.Shapes.Shape.OnPropertyChanged(string? property ~override Microsoft.Maui.Controls.RadioButton.OnPropertyChanged(string propertyName = null) -> void 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 diff --git a/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt index 8699fea1fce2..65e1a3eca2b1 100644 --- a/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt @@ -4,4 +4,5 @@ override Microsoft.Maui.Controls.Shapes.Shape.OnPropertyChanged(string? property ~override Microsoft.Maui.Controls.RadioButton.OnPropertyChanged(string propertyName = null) -> void 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 diff --git a/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt index 5889921a04d8..8c967a27bcc1 100644 --- a/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt @@ -4,4 +4,5 @@ override Microsoft.Maui.Controls.Shapes.Shape.OnPropertyChanged(string? property override Microsoft.Maui.Controls.GraphicsView.OnBindingContextChanged() -> void override Microsoft.Maui.Controls.SwipeItemView.IsEnabledCore.get -> bool override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void +~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void Microsoft.Maui.Controls.Label.~Label() -> void diff --git a/src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt index 5889921a04d8..8c967a27bcc1 100644 --- a/src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt @@ -4,4 +4,5 @@ override Microsoft.Maui.Controls.Shapes.Shape.OnPropertyChanged(string? property override Microsoft.Maui.Controls.GraphicsView.OnBindingContextChanged() -> void override Microsoft.Maui.Controls.SwipeItemView.IsEnabledCore.get -> bool override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void +~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void Microsoft.Maui.Controls.Label.~Label() -> void diff --git a/src/Controls/src/Core/SwipeView/SwipeItems.cs b/src/Controls/src/Core/SwipeView/SwipeItems.cs index ca5153a074f5..066ad57b1d2d 100644 --- a/src/Controls/src/Core/SwipeView/SwipeItems.cs +++ b/src/Controls/src/Core/SwipeView/SwipeItems.cs @@ -16,6 +16,42 @@ public class SwipeItems : Element, IList, INotifyCollectionChanged { readonly ObservableCollection _swipeItems; + /// + /// Notifies the owning (resolved via ) + /// that something in this collection has changed so the handler can refresh the platform UI. + /// + /// + /// Using (rather than a captured-closure subscription on the + /// SwipeView side) avoids leaking the owning SwipeView when a SwipeItems instance is + /// cached/shared across multiple SwipeView instances. When SwipeItems is reassigned to a + /// new owner, AddLogicalChild reassigns so notifications + /// always go to the current owner. + /// + void NotifyOwner() + { + if (Parent is not SwipeView swipeView) + { + return; + } + + if (this == swipeView.LeftItems) + { + swipeView.Handler?.UpdateValue(nameof(SwipeView.LeftItems)); + } + else if (this == swipeView.RightItems) + { + swipeView.Handler?.UpdateValue(nameof(SwipeView.RightItems)); + } + else if (this == swipeView.TopItems) + { + swipeView.Handler?.UpdateValue(nameof(SwipeView.TopItems)); + } + else if (this == swipeView.BottomItems) + { + swipeView.Handler?.UpdateValue(nameof(SwipeView.BottomItems)); + } + } + /// /// Initializes a new instance of the class with the specified swipe items. /// @@ -33,6 +69,22 @@ public SwipeItems(IEnumerable swipeItems) _swipeItems.CollectionChanged += OnSwipeItemsChanged; } + // Override OnPropertyChanged so we can notify the owning SwipeView when Mode / + // SwipeBehaviorOnInvoked change. Filtering by property name avoids a Handler.UpdateValue + // storm on every base-Element property change (Parent, BindingContext, Style, etc.). + // The notification goes through Parent (a weak reference managed by AddLogicalChild), + // so the owning SwipeView is never rooted by this SwipeItems instance (issue #35481). + protected override void OnPropertyChanged([System.Runtime.CompilerServices.CallerMemberName] string propertyName = null) + { + base.OnPropertyChanged(propertyName); + + if (propertyName == ModeProperty.PropertyName || + propertyName == SwipeBehaviorOnInvokedProperty.PropertyName) + { + NotifyOwner(); + } + } + /// /// Initializes a new instance of the class. /// @@ -157,6 +209,9 @@ void OnSwipeItemsChanged(object sender, NotifyCollectionChangedEventArgs notifyC } CollectionChanged?.Invoke(this, notifyCollectionChangedEventArgs); + + // Notify the owning SwipeView so its handler can update the platform UI. + NotifyOwner(); } IEnumerator IEnumerable.GetEnumerator() diff --git a/src/Controls/src/Core/SwipeView/SwipeView.cs b/src/Controls/src/Core/SwipeView/SwipeView.cs index 18a27764e189..2277b9a9e9f3 100644 --- a/src/Controls/src/Core/SwipeView/SwipeView.cs +++ b/src/Controls/src/Core/SwipeView/SwipeView.cs @@ -163,44 +163,21 @@ static void OnSwipeItemsChanged(BindableObject bindable, object oldValue, object if (oldValue is SwipeItems oldItems) { - oldItems.CollectionChanged -= SwipeItemsCollectionChanged; - oldItems.PropertyChanged -= SwipeItemsPropertyChanged; swipeView.RemoveLogicalChild(oldItems); } if (newValue is SwipeItems newItems) { - newItems.CollectionChanged += SwipeItemsCollectionChanged; - newItems.PropertyChanged += SwipeItemsPropertyChanged; + // AddLogicalChild reassigns newItems.Parent to this SwipeView. When SwipeItems is + // cached/shared across multiple SwipeView instances, subsequent assignments + // reparent it away from the previous owner, so previous SwipeViews are not held + // alive through this collection. SwipeItems itself self-subscribes to its own + // CollectionChanged / PropertyChanged events and uses Parent (this SwipeView) to + // notify the handler — we intentionally do NOT add closure-based subscriptions + // here, since those captured `swipeView` and prevented GC when SwipeItems was + // shared across SwipeViews (issue #35481). swipeView.AddLogicalChild(newItems); } - - void SwipeItemsPropertyChanged(object sender, PropertyChangedEventArgs e) - { - if (sender is SwipeItems swipeItems) - SendChange(swipeItems); - } - - void SwipeItemsCollectionChanged(object sender, System.Collections.Specialized.NotifyCollectionChangedEventArgs e) - { - if (sender is SwipeItems swipeItems) - SendChange(swipeItems); - } - - void SendChange(SwipeItems swipeItems) - { - if (swipeItems == swipeView.LeftItems) - swipeView?.Handler?.UpdateValue(nameof(LeftItems)); - - if (swipeItems == swipeView.RightItems) - swipeView?.Handler?.UpdateValue(nameof(RightItems)); - - if (swipeItems == swipeView.TopItems) - swipeView?.Handler?.UpdateValue(nameof(TopItems)); - - if (swipeItems == swipeView.BottomItems) - swipeView?.Handler?.UpdateValue(nameof(BottomItems)); - } } public event EventHandler SwipeStarted; @@ -295,13 +272,24 @@ bool ISwipeView.IsOpen protected override void OnChildAdded(Element child) { base.OnChildAdded(child); - child.PropertyChanged += OnPropertyChanged; + + // Skip SwipeItems children: they are logical children for visual-tree purposes only; + // subscribing to their PropertyChanged would create a strong reference from a + // potentially cached/long-lived SwipeItems back to this SwipeView. + if (child is not SwipeItems) + { + child.PropertyChanged += OnPropertyChanged; + } } protected override void OnChildRemoved(Element child, int oldLogicalIndex) { base.OnChildRemoved(child, oldLogicalIndex); - child.PropertyChanged -= OnPropertyChanged; + + if (child is not SwipeItems) + { + child.PropertyChanged -= OnPropertyChanged; + } } void OnPropertyChanged(object? sender, System.ComponentModel.PropertyChangedEventArgs e) diff --git a/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs b/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs index 2118199daec3..71f3a5fb1b18 100644 --- a/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs +++ b/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs @@ -1,3 +1,5 @@ +using System; +using System.Collections.Generic; using Microsoft.Maui.Graphics; using Microsoft.Maui.Platform; using Xunit; @@ -601,5 +603,94 @@ public void SwipeViewRediscoversScrollParentWhenTemplateRootIsReparented() var scrollView2 = new ScrollView { Content = contentView }; Assert.Equal(scrollView2, GetPrivateField(swipeView, "_scrollParent")); } + + // Regression test for https://github.com/dotnet/maui/issues/35481 + // A SwipeItems instance that is cached (e.g. in a static dictionary) and shared + // across multiple SwipeViews must not keep prior SwipeView instances alive. + [Fact] + public void CachedSwipeItemsDoesNotKeepSwipeViewAlive() + { + // Simulate the repro: one long-lived SwipeItems cache shared across many SwipeViews. + var cachedSwipeItems = new SwipeItems { new SwipeItem { Text = "Delete" } }; + + // Allocate the SwipeViews in a non-inlined helper so the locals do not remain + // rooted on the test method's stack frame in Debug builds. + var swipeViewRefs = CreateSwipeViewsSharingCachedItems(cachedSwipeItems, 20); + + // Mutate the cached SwipeItems after the SwipeViews are no longer referenced. + // Before the fix, the CollectionChanged/PropertyChanged subscriptions on + // SwipeItems held a strong reference back to every SwipeView that had ever + // used this cache, so this mutation would keep them all alive. + cachedSwipeItems.Add(new SwipeItem { Text = "Archive" }); + + ForceFullGC(); + + GC.KeepAlive(cachedSwipeItems); + + Assert.All(swipeViewRefs, r => Assert.False(r.IsAlive, + "SwipeView was kept alive by the cached SwipeItems — issue #35481 regression.")); + } + + [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.NoInlining)] + static List CreateSwipeViewsSharingCachedItems(SwipeItems shared, int count) + { + var refs = new List(count); + for (int i = 0; i < count; i++) + { + var sv = new SwipeView { RightItems = shared }; + // Simulate a CollectionView recycling the row: the SwipeView reassigns its + // RightItems before going out of scope, releasing the Parent back-reference + // from the cached SwipeItems. Before the fix, the closure-based + // CollectionChanged/PropertyChanged subscriptions on the cached SwipeItems + // still held this SwipeView alive even after this reassignment. + sv.RightItems = new SwipeItems(); + refs.Add(new WeakReference(sv)); + } + return refs; + } + + // Regression test for https://github.com/dotnet/maui/issues/35481 + // Replacing RightItems with a new SwipeItems instance must release any back-reference + // from the previous (cached) SwipeItems to the SwipeView. Without the fix, the cached + // SwipeItems' CollectionChanged/PropertyChanged delegates kept the SwipeView alive + // even after it was logically unhooked. + [Fact] + public void ReplacingCachedSwipeItemsReleasesPreviousOwnerReference() + { + // Cache: held by external code (the user's static dictionary in the repro). + var cachedRightItems = new SwipeItems { new SwipeItem { Text = "Done" } }; + + var swipeViewRef = CreateSwipeViewAssignThenReplace(cachedRightItems); + + // Mutate the cached SwipeItems after the SwipeView is unhooked, just like the repro. + cachedRightItems.Add(new SwipeItem { Text = "Archive" }); + + ForceFullGC(); + + GC.KeepAlive(cachedRightItems); + + Assert.False(swipeViewRef.IsAlive, + "Replaced SwipeView was kept alive by its previously-assigned cached SwipeItems — issue #35481 regression."); + } + + [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.NoInlining)] + static WeakReference CreateSwipeViewAssignThenReplace(SwipeItems cachedRightItems) + { + var sv = new SwipeView { RightItems = cachedRightItems }; + // Replace the assignment — the previous (cached) SwipeItems is now logically + // unhooked but kept alive by the caller. It must not retain the SwipeView. + sv.RightItems = new SwipeItems { new SwipeItem { Text = "Replaced" } }; + return new WeakReference(sv); + } + + static void ForceFullGC() + { + for (int i = 0; i < 5; i++) + { + GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, blocking: true, compacting: true); + GC.WaitForPendingFinalizers(); + GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, blocking: true, compacting: true); + } + } } }