[Android & iOS] TabbedPage leaks with shared GradientBrush.#35543
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35543Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35543" |
|
Hey there @@SubhikshaSf4851! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
/review -b feature/regression-check -p android |
|
/review -b feature/regression-check -p android |
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 5 findings
See inline comments for details.
|
/review -b feature/regression-check -p android |
1 similar comment
|
/review -b feature/regression-check -p android |
MauiBot
left a comment
There was a problem hiding this comment.
🤖 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 passed the Android TabbedPage regression suite and addresses both root retention paths: the shared GradientBrush no longer strongly roots TabbedPageManager through the event delegate, and shared brushes are not unconditionally assigned a Parent back-reference. The PR-based candidates passed the gate but still rely on disconnect/dispose timing to break those references.
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/Platform/Android/TabbedPageManager.cs b/src/Controls/src/Core/Platform/Android/TabbedPageManager.cs
index f885d7673f..90b3b238a7 100644
--- a/src/Controls/src/Core/Platform/Android/TabbedPageManager.cs
+++ b/src/Controls/src/Core/Platform/Android/TabbedPageManager.cs
@@ -56,6 +56,8 @@ public class TabbedPageManager
public ViewPager2 ViewPager => _viewPager;
int _tabplacementId;
Brush _currentBarBackground;
+ WeakBrushInvalidationSubscriber _barBackgroundSubscription;
+ bool _ownsBarBackgroundBrushParent;
Color _currentBarItemColor;
Color _currentBarTextColor;
Color _currentBarSelectedItemColor;
@@ -129,6 +131,8 @@ public class TabbedPageManager
_viewPager.LayoutChange -= OnLayoutChanged;
_viewPager.Adapter = null;
+
+ TeardownBarBackgroundSubscription();
}
Element = tabbedPage;
@@ -603,16 +607,34 @@ public class TabbedPageManager
if (_currentBarBackground is GradientBrush oldGradientBrush)
{
- oldGradientBrush.Parent = null;
- oldGradientBrush.InvalidateGradientBrushRequested -= OnBarBackgroundChanged;
+ if (_ownsBarBackgroundBrushParent)
+ {
+ oldGradientBrush.Parent = null;
+ _ownsBarBackgroundBrushParent = false;
+ }
+ _barBackgroundSubscription?.Unsubscribe();
+ _barBackgroundSubscription = null;
}
_currentBarBackground = Element.BarBackground;
if (_currentBarBackground is GradientBrush newGradientBrush)
{
- newGradientBrush.Parent = Element;
- newGradientBrush.InvalidateGradientBrushRequested += OnBarBackgroundChanged;
+ // Only take Parent ownership of brushes that are NOT already parented (e.g. shared
+ // app-resource brushes). Hijacking a shared brush's Parent creates a strong reference
+ // from the shared brush back to this TabbedPage, leaking the page graph.
+ if (newGradientBrush.Parent is null)
+ {
+ newGradientBrush.Parent = Element;
+ _ownsBarBackgroundBrushParent = true;
+ }
+
+ // Subscribe via a weak wrapper so the brush's invocation list does NOT hold a strong
+ // delegate reference to this TabbedPageManager. If the manager is collected before
+ // disconnect runs (e.g. deferred modal pop), the wrapper auto-unsubscribes on the
+ // next event invocation and the manager + page graph remain GC-eligible.
+ _barBackgroundSubscription = new WeakBrushInvalidationSubscriber(this, newGradientBrush);
+
if (_bottomNavigationView is not null && _bottomNavigationView.Elevation > 0)
{
_bottomNavigationView.Elevation = 0;
@@ -626,6 +648,21 @@ public class TabbedPageManager
RefreshBarBackground();
}
+ void TeardownBarBackgroundSubscription()
+ {
+ if (_currentBarBackground is GradientBrush currentGradientBrush)
+ {
+ if (_ownsBarBackgroundBrushParent)
+ {
+ currentGradientBrush.Parent = null;
+ _ownsBarBackgroundBrushParent = false;
+ }
+ }
+ _barBackgroundSubscription?.Unsubscribe();
+ _barBackgroundSubscription = null;
+ _currentBarBackground = null;
+ }
+
void OnBarBackgroundChanged(object sender, EventArgs e)
{
RefreshBarBackground();
@@ -993,6 +1030,48 @@ public class TabbedPageManager
#pragma warning restore RS0030
}
+ // Weakly subscribes to a GradientBrush's InvalidateGradientBrushRequested event so the
+ // brush's invocation list does not hold a strong reference back to TabbedPageManager.
+ // This is the primary defense against shared/app-resource brushes leaking the page graph
+ // when the TabbedPage is removed before any synchronous disconnect runs. Unsubscribe()
+ // is called from UpdateBarBackground (brush rotation) and SetElement(null) (disconnect)
+ // to clear the brush's invocation list deterministically; if neither runs, the next event
+ // invocation finds a dead target and removes the subscription itself.
+ sealed class WeakBrushInvalidationSubscriber
+ {
+ readonly WeakReference<TabbedPageManager> _ownerRef;
+ GradientBrush _brush;
+
+ public WeakBrushInvalidationSubscriber(TabbedPageManager owner, GradientBrush brush)
+ {
+ _ownerRef = new WeakReference<TabbedPageManager>(owner);
+ _brush = brush;
+ brush.InvalidateGradientBrushRequested += OnInvalidated;
+ }
+
+ void OnInvalidated(object sender, EventArgs e)
+ {
+ if (_ownerRef.TryGetTarget(out var owner))
+ {
+ owner.RefreshBarBackground();
+ }
+ else
+ {
+ Unsubscribe();
+ }
+ }
+
+ public void Unsubscribe()
+ {
+ var brush = _brush;
+ if (brush is not null)
+ {
+ brush.InvalidateGradientBrushRequested -= OnInvalidated;
+ _brush = null;
+ }
+ }
+ }
+
class Listeners : ViewPager2.OnPageChangeCallback,
#pragma warning disable CS0618 // Type or member is obsolete
TabLayout.IOnTabSelectedListener,
🤖 AI Summary
📊 Review Session —
|
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
📱 TabbedPageTests (TabbedPageDoesNotLeakGradientBrushSubscriberOnDisconnect, TabbedPageDoesNotLeakGradientBrushSubscriberWhenSetViaStyle) Category=TabbedPage |
✅ FAIL — 958s | ✅ PASS — 437s |
🔴 Without fix — 📱 TabbedPageTests (TabbedPageDoesNotLeakGradientBrushSubscriberOnDisconnect, TabbedPageDoesNotLeakGradientBrushSubscriberWhenSetViaStyle): FAIL ✅ · 958s
(truncated to last 15,000 chars)
ll.so
[122/133] System.Text.Json.dll -> System.Text.Json.dll.so
[46/133] Xamarin.AndroidX.Lifecycle.ViewModelSavedState.Android.dll -> Xamarin.AndroidX.Lifecycle.ViewModelSavedState.Android.dll.so
[123/133] System.Threading.ThreadPool.dll -> System.Threading.ThreadPool.dll.so
[124/133] System.Threading.dll -> System.Threading.dll.so
[47/133] Xamarin.AndroidX.Loader.dll -> Xamarin.AndroidX.Loader.dll.so
[125/133] System.Xml.Linq.dll -> System.Xml.Linq.dll.so
[126/133] System.Xml.ReaderWriter.dll -> System.Xml.ReaderWriter.dll.so
[48/133] Xamarin.AndroidX.Navigation.Common.Android.dll -> Xamarin.AndroidX.Navigation.Common.Android.dll.so
[127/133] System.Xml.XDocument.dll -> System.Xml.XDocument.dll.so
[128/133] System.dll -> System.dll.so
[129/133] netstandard.dll -> netstandard.dll.so
[49/133] Xamarin.AndroidX.Navigation.Fragment.dll -> Xamarin.AndroidX.Navigation.Fragment.dll.so
[130/133] Mono.Android.Runtime.dll -> Mono.Android.Runtime.dll.so
[50/133] Xamarin.AndroidX.Navigation.Runtime.Android.dll -> Xamarin.AndroidX.Navigation.Runtime.Android.dll.so
[51/133] Xamarin.AndroidX.Navigation.UI.dll -> Xamarin.AndroidX.Navigation.UI.dll.so
[131/133] Java.Interop.dll -> Java.Interop.dll.so
[52/133] Xamarin.AndroidX.RecyclerView.dll -> Xamarin.AndroidX.RecyclerView.dll.so
[53/133] Xamarin.AndroidX.SavedState.SavedState.Android.dll -> Xamarin.AndroidX.SavedState.SavedState.Android.dll.so
[54/133] Xamarin.AndroidX.SwipeRefreshLayout.dll -> Xamarin.AndroidX.SwipeRefreshLayout.dll.so
[55/133] Xamarin.AndroidX.ViewPager.dll -> Xamarin.AndroidX.ViewPager.dll.so
[56/133] Xamarin.AndroidX.ViewPager2.dll -> Xamarin.AndroidX.ViewPager2.dll.so
[57/133] Xamarin.Google.Android.Material.dll -> Xamarin.Google.Android.Material.dll.so
[58/133] Xamarin.GooglePlayServices.Base.dll -> Xamarin.GooglePlayServices.Base.dll.so
[132/133] Mono.Android.dll -> Mono.Android.dll.so
[59/133] Xamarin.GooglePlayServices.Basement.dll -> Xamarin.GooglePlayServices.Basement.dll.so
[60/133] Xamarin.GooglePlayServices.Maps.dll -> Xamarin.GooglePlayServices.Maps.dll.so
[61/133] Xamarin.GooglePlayServices.Tasks.dll -> Xamarin.GooglePlayServices.Tasks.dll.so
[62/133] Xamarin.Kotlin.StdLib.dll -> Xamarin.Kotlin.StdLib.dll.so
[63/133] Xamarin.KotlinX.Coroutines.Core.Jvm.dll -> Xamarin.KotlinX.Coroutines.Core.Jvm.dll.so
[64/133] Xamarin.KotlinX.Serialization.Core.Jvm.dll -> Xamarin.KotlinX.Serialization.Core.Jvm.dll.so
[65/133] xunit.abstractions.dll -> xunit.abstractions.dll.so
[66/133] xunit.assert.dll -> xunit.assert.dll.so
[67/133] xunit.core.dll -> xunit.core.dll.so
[68/133] xunit.execution.dotnet.dll -> xunit.execution.dotnet.dll.so
[69/133] xunit.runner.utility.netcoreapp10.dll -> xunit.runner.utility.netcoreapp10.dll.so
[70/133] System.Collections.Concurrent.dll -> System.Collections.Concurrent.dll.so
[71/133] System.Collections.Immutable.dll -> System.Collections.Immutable.dll.so
[72/133] System.Collections.NonGeneric.dll -> System.Collections.NonGeneric.dll.so
[73/133] System.Collections.Specialized.dll -> System.Collections.Specialized.dll.so
[74/133] System.Collections.dll -> System.Collections.dll.so
[75/133] System.ComponentModel.Primitives.dll -> System.ComponentModel.Primitives.dll.so
[76/133] System.ComponentModel.TypeConverter.dll -> System.ComponentModel.TypeConverter.dll.so
[77/133] System.ComponentModel.dll -> System.ComponentModel.dll.so
[133/133] System.Private.CoreLib.dll -> System.Private.CoreLib.dll.so
[78/133] System.Console.dll -> System.Console.dll.so
[79/133] System.Diagnostics.Debug.dll -> System.Diagnostics.Debug.dll.so
[80/133] System.Diagnostics.DiagnosticSource.dll -> System.Diagnostics.DiagnosticSource.dll.so
[81/133] System.Diagnostics.Process.dll -> System.Diagnostics.Process.dll.so
[82/133] System.Diagnostics.Tools.dll -> System.Diagnostics.Tools.dll.so
[83/133] System.Diagnostics.TraceSource.dll -> System.Diagnostics.TraceSource.dll.so
[84/133] System.Diagnostics.Tracing.dll -> System.Diagnostics.Tracing.dll.so
[85/133] System.Drawing.Primitives.dll -> System.Drawing.Primitives.dll.so
[86/133] System.Drawing.dll -> System.Drawing.dll.so
[87/133] System.Formats.Asn1.dll -> System.Formats.Asn1.dll.so
[88/133] System.Globalization.dll -> System.Globalization.dll.so
[89/133] System.IO.Compression.Brotli.dll -> System.IO.Compression.Brotli.dll.so
[90/133] System.IO.Compression.dll -> System.IO.Compression.dll.so
[91/133] System.IO.FileSystem.dll -> System.IO.FileSystem.dll.so
[92/133] System.IO.Pipelines.dll -> System.IO.Pipelines.dll.so
[93/133] System.IO.dll -> System.IO.dll.so
[94/133] System.Linq.Expressions.dll -> System.Linq.Expressions.dll.so
[95/133] System.Linq.dll -> System.Linq.dll.so
[96/133] System.Memory.dll -> System.Memory.dll.so
[97/133] System.Net.Http.dll -> System.Net.Http.dll.so
[98/133] System.Net.NameResolution.dll -> System.Net.NameResolution.dll.so
[99/133] System.Net.Primitives.dll -> System.Net.Primitives.dll.so
[100/133] System.Net.Requests.dll -> System.Net.Requests.dll.so
[101/133] System.Net.Sockets.dll -> System.Net.Sockets.dll.so
[102/133] System.Numerics.Vectors.dll -> System.Numerics.Vectors.dll.so
[103/133] System.ObjectModel.dll -> System.ObjectModel.dll.so
[104/133] System.Private.Uri.dll -> System.Private.Uri.dll.so
[105/133] System.Private.Xml.Linq.dll -> System.Private.Xml.Linq.dll.so
[106/133] System.Private.Xml.dll -> System.Private.Xml.dll.so
[107/133] System.Reflection.Extensions.dll -> System.Reflection.Extensions.dll.so
[108/133] System.Reflection.TypeExtensions.dll -> System.Reflection.TypeExtensions.dll.so
[109/133] System.Reflection.dll -> System.Reflection.dll.so
[110/133] System.Runtime.Extensions.dll -> System.Runtime.Extensions.dll.so
[111/133] System.Runtime.InteropServices.RuntimeInformation.dll -> System.Runtime.InteropServices.RuntimeInformation.dll.so
[112/133] System.Runtime.InteropServices.dll -> System.Runtime.InteropServices.dll.so
[113/133] System.Runtime.Loader.dll -> System.Runtime.Loader.dll.so
[114/133] System.Runtime.Numerics.dll -> System.Runtime.Numerics.dll.so
[115/133] System.Runtime.dll -> System.Runtime.dll.so
[116/133] System.Security.Cryptography.dll -> System.Security.Cryptography.dll.so
[117/133] System.Text.Encoding.dll -> System.Text.Encoding.dll.so
[118/133] System.Text.Encodings.Web.dll -> System.Text.Encodings.Web.dll.so
[119/133] System.Text.Json.dll -> System.Text.Json.dll.so
[120/133] System.Text.RegularExpressions.dll -> System.Text.RegularExpressions.dll.so
[121/133] System.Threading.Tasks.dll -> System.Threading.Tasks.dll.so
[122/133] System.Threading.Thread.dll -> System.Threading.Thread.dll.so
[123/133] System.Threading.ThreadPool.dll -> System.Threading.ThreadPool.dll.so
[124/133] System.Threading.dll -> System.Threading.dll.so
[125/133] System.Xml.Linq.dll -> System.Xml.Linq.dll.so
[126/133] System.Xml.ReaderWriter.dll -> System.Xml.ReaderWriter.dll.so
[127/133] System.Xml.XDocument.dll -> System.Xml.XDocument.dll.so
[128/133] System.dll -> System.dll.so
[129/133] netstandard.dll -> netstandard.dll.so
[130/133] Java.Interop.dll -> Java.Interop.dll.so
[131/133] Mono.Android.Runtime.dll -> Mono.Android.Runtime.dll.so
[132/133] Mono.Android.dll -> Mono.Android.dll.so
[133/133] System.Private.CoreLib.dll -> System.Private.CoreLib.dll.so
Build succeeded.
0 Warning(s)
0 Error(s)
Time Elapsed 00:11:07.77
[11.0.0-prerelease.26230.4+92962e5c46ac08a66ded4c5696209cc60f1a232f] XHarness command issued: android test --app /home/vsts/work/1/s/artifacts/bin/Controls.DeviceTests/Release/net10.0-android/com.microsoft.maui.controls.devicetests-Signed.apk --package-name com.microsoft.maui.controls.devicetests --device-id emulator-5554 -o artifacts/log --timeout 01:00:00 -v --arg TestFilter=Category=TabbedPage
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: ADBRunner using ADB.exe supplied from /home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/tools/net10.0/any/../../../runtimes/any/native/adb/linux/adb
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Full resolved path:'/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb'
^[[40m^[[32minfo^[[39m^[[22m^[[49m: Will attempt to find device supporting architectures: 'arm64-v8a', 'x86_64'
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb start-server'
^[[40m^[[37mdbug^[[39m^[[22m^[[49m:
^[[40m^[[32minfo^[[39m^[[22m^[[49m: Finding attached devices/emulators...
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb devices -l'
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Found 1 possible devices
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Evaluating output line for device serial: emulator-5554 device product:sdk_gphone_x86_64 model:sdk_gphone_x86_64 device:generic_x86_64_arm64 transport_id:2
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 shell getprop ro.product.cpu.abilist'
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Found 1 possible devices. Using 'emulator-5554'
^[[40m^[[32minfo^[[39m^[[22m^[[49m: Active Android device set to serial 'emulator-5554'
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 -s emulator-5554 shell getprop ro.product.cpu.abi'
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 -s emulator-5554 shell getprop ro.build.version.sdk'
^[[40m^[[32minfo^[[39m^[[22m^[[49m: Waiting for device to be available (max 5 minutes)
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 wait-for-device'
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 -s emulator-5554 shell getprop sys.boot_completed'
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: sys.boot_completed = '1'
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Waited 0 seconds for device boot completion
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Working with emulator-5554 (API 30)
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Check current adb install and/or package verification settings
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 shell settings get global verifier_verify_adb_installs'
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: verifier_verify_adb_installs = 0
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 shell settings get global package_verifier_enable'
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: package_verifier_enable =
^[[40m^[[1m^[[33mwarn^[[39m^[[22m^[[49m: Installing debug apks on a device might be rejected with INSTALL_FAILED_VERIFICATION_FAILURE. Make sure to set 'package_verifier_enable' to '0'
^[[40m^[[32minfo^[[39m^[[22m^[[49m: Attempting to remove apk 'com.microsoft.maui.controls.devicetests'..
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 uninstall com.microsoft.maui.controls.devicetests'
^[[40m^[[1m^[[33mwarn^[[39m^[[22m^[[49m: Hit broken pipe error; Will make one attempt to restart ADB server, and retry the uninstallation
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 kill-server'
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 start-server'
^[[40m^[[37mdbug^[[39m^[[22m^[[49m:
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 uninstall com.microsoft.maui.controls.devicetests'
^[[41m^[[30mfail^[[39m^[[22m^[[49m: Error: Exit code: 20
Std out:
Std err:
- waiting for device -
cmd: Can't find service: package
^[[40m^[[32minfo^[[39m^[[22m^[[49m: Attempting to install /home/vsts/work/1/s/artifacts/bin/Controls.DeviceTests/Release/net10.0-android/com.microsoft.maui.controls.devicetests-Signed.apk
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 install /home/vsts/work/1/s/artifacts/bin/Controls.DeviceTests/Release/net10.0-android/com.microsoft.maui.controls.devicetests-Signed.apk'
^[[41m^[[30mfail^[[39m^[[22m^[[49m: Error:
Exit code: 1
Std out:
Serving...
Performing Incremental Install
cmd: Can't find service: package
Performing Streamed Install
Std err:
adb: failed to install /home/vsts/work/1/s/artifacts/bin/Controls.DeviceTests/Release/net10.0-android/com.microsoft.maui.controls.devicetests-Signed.apk: cmd: Can't find service: package
^[[41m^[[1m^[[37mcrit^[[39m^[[22m^[[49m: Install failure: Test command cannot continue
^[[40m^[[32minfo^[[39m^[[22m^[[49m: Attempting to remove apk 'com.microsoft.maui.controls.devicetests'..
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 uninstall com.microsoft.maui.controls.devicetests'
^[[41m^[[30mfail^[[39m^[[22m^[[49m: Error: Exit code: 20
Std out:
Std err:
cmd: Can't find service: package
^[[40m^[[32minfo^[[39m^[[22m^[[49m: Attempting to remove apk 'com.microsoft.maui.controls.devicetests'..
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 uninstall com.microsoft.maui.controls.devicetests'
^[[41m^[[30mfail^[[39m^[[22m^[[49m: Error: Exit code: 20
Std out:
Std err:
cmd: Can't find service: package
XHarness exit code: 78 (PACKAGE_INSTALLATION_FAILURE)
Tests completed with exit code: 78
🟢 With fix — 📱 TabbedPageTests (TabbedPageDoesNotLeakGradientBrushSubscriberOnDisconnect, TabbedPageDoesNotLeakGradientBrushSubscriberWhenSetViaStyle): PASS ✅ · 437s
(truncated to last 15,000 chars)
kDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Application'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Behavior'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Border'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'BoxView'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Button'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'CarouselView'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'CheckBox'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'CollectionView'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Compatibility'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'ContentView'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'DatePicker'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Dispatcher'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Editor'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Element'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Entry'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'FlexLayout'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'FlyoutPage'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Frame'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Gesture'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'HybridWebView'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Image'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Label'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Layout'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Lifecycle'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'ListView'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Map'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'MenuFlyout'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Mapper'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Excluded test (filtered by Trait; 'Category':'Memory'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Modal'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'NavigationPage'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Page'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Path'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Picker'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'RadioButton'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'RefreshView'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'ScrollView'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'SearchBar'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Shape'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Shell'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Slider'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'SwipeView'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'TextInput'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Toolbar'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'TemplatedView'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'View'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'VisualElement'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'VisualElementTree'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'WebView'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Window'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'WindowOverlay'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.060 15777 15803 I DOTNET : [FILTER] Included test (filtered by Trait; 'Category':'Xaml'): [Memory] TweenersWillNotLeakDuringInfiniteAnimation
05-21 15:43:27.084 15777 15803 I DOTNET : [Test environment: 64-bit .NET .NET 10.0 [collection-per-class, non-parallel]]
05-21 15:43:27.084 15777 15803 I DOTNET : [Test framework: xUnit.net 2.9.0.0]
05-21 15:43:27.099 15777 15803 I DOTNET : Test collection for Microsoft.Maui.DeviceTests.TabbedPageTests
05-21 15:43:27.770 15777 15815 I DOTNET : [PASS] SettingJustSelectedATabColorOnBottomTabsDoesntCrash
05-21 15:43:32.055 15777 15832 I DOTNET : [PASS] Bar Text Color
05-21 15:43:32.591 15777 15838 I DOTNET : [PASS] ChangingBottomTabAttributesDoesntRecreateBottomTabs
05-21 15:43:32.630 15777 15838 I DOTNET : [PASS] ScaleConsistent
05-21 15:43:41.073 15777 15951 I DOTNET : [PASS] Selected/Unselected Color
05-21 15:43:41.428 15777 15962 I DOTNET : [PASS] SettingCurrentPageToNotBePositionZeroWorks
05-21 15:43:41.721 15777 15967 I DOTNET : [PASS] SettingCurrentPageToNotBePositionZeroWorks
05-21 15:43:41.922 15777 15972 I DOTNET : [PASS] SettingCurrentPageToNotBePositionZeroWorks
05-21 15:43:42.170 15777 15977 I DOTNET : [PASS] SettingCurrentPageToNotBePositionZeroWorks
05-21 15:43:42.193 15777 15983 I DOTNET : [PASS] RotationYConsistent
05-21 15:43:42.197 15777 15983 I DOTNET : [PASS] RotationXConsistent
05-21 15:43:42.889 15777 15990 I DOTNET : [PASS] NavigatingAwayFromTabbedPageResizesContentPage
05-21 15:43:43.523 15777 15996 I DOTNET : [PASS] NavigatingAwayFromTabbedPageResizesContentPage
05-21 15:43:46.582 15777 16001 I DOTNET : [PASS] TabbedPage renderer/manager does not leak shared GradientBrush subscriber on disconnect
05-21 15:43:46.586 15777 16001 I DOTNET : [PASS] ScaleXConsistent
05-21 15:43:56.434 15777 16006 I DOTNET : [PASS] MovingBetweenMultiplePagesWithNestedNavigationPages
05-21 15:44:03.791 15777 16011 I DOTNET : [PASS] MovingBetweenMultiplePagesWithNestedNavigationPages
05-21 15:44:11.054 15777 16016 I DOTNET : [PASS] MovingBetweenMultiplePagesWithNestedNavigationPages
05-21 15:44:20.521 15777 16028 I DOTNET : [PASS] MovingBetweenMultiplePagesWithNestedNavigationPages
05-21 15:44:20.949 15777 16033 I DOTNET : [PASS] BottomNavigationViewExtendsToScreenBottom
05-21 15:44:22.951 15777 16039 I DOTNET : [PASS] Does Not Leak
05-21 15:44:26.605 15777 16045 I DOTNET : [PASS] DisconnectEachPageHandlerAfterNavigation
05-21 15:44:29.218 15777 16050 I DOTNET : [PASS] DisconnectEachPageHandlerAfterNavigation
05-21 15:44:31.932 15777 16056 I DOTNET : [PASS] DisconnectEachPageHandlerAfterNavigation
05-21 15:44:36.090 15777 16070 I DOTNET : [PASS] DisconnectEachPageHandlerAfterNavigation
05-21 15:44:36.442 15777 16076 I DOTNET : [PASS] Custom RecyclerView Adapter Doesn't Crash
05-21 15:44:36.455 15777 16076 I DOTNET : [PASS] RotationConsistent
05-21 15:44:36.458 15777 16076 I DOTNET : [PASS] ScaleYConsistent
05-21 15:44:39.273 15777 16081 I DOTNET : [PASS] TabbedPage renderer/manager does not leak shared GradientBrush subscriber when BarBackground is set via Style
05-21 15:44:39.447 15777 16086 I DOTNET : [PASS] Using SelectedTab Color doesnt crash
05-21 15:44:40.000 15777 16091 I DOTNET : [PASS] RemovingAllPagesDoesntCrash
05-21 15:44:40.519 15777 16096 I DOTNET : [PASS] RemovingAllPagesDoesntCrash
05-21 15:44:41.029 15777 16101 I DOTNET : [PASS] RemovingAllPagesDoesntCrash
05-21 15:44:41.539 15777 16106 I DOTNET : [PASS] RemovingAllPagesDoesntCrash
05-21 15:44:42.536 15777 16111 I DOTNET : [PASS] PoppingTabbedPageDoesntCrash
05-21 15:44:43.344 15777 16116 I DOTNET : [PASS] PoppingTabbedPageDoesntCrash
05-21 15:44:44.254 15777 16121 I DOTNET : [PASS] PoppingTabbedPageDoesntCrash
05-21 15:44:45.187 15777 16126 I DOTNET : [PASS] PoppingTabbedPageDoesntCrash
05-21 15:44:45.969 15777 16133 I DOTNET : [PASS] Remove CurrentPage And Then Re-Add Doesnt Crash
05-21 15:44:46.590 15777 16138 I DOTNET : [PASS] Remove CurrentPage And Then Re-Add Doesnt Crash
05-21 15:44:47.316 15777 16143 I DOTNET : [PASS] Remove CurrentPage And Then Re-Add Doesnt Crash
05-21 15:44:48.075 15777 16148 I DOTNET : [PASS] Remove CurrentPage And Then Re-Add Doesnt Crash
05-21 15:44:48.123 15777 16148 I DOTNET : Microsoft.Maui.DeviceTests.TabbedPageTests 80.7459175 ms
05-21 15:44:48.130 15777 16148 I DOTNET : Test collection for Microsoft.Maui.DeviceTests.AlertDialogTests
05-21 15:44:48.172 15777 16148 I DOTNET : [PASS] AlertDialogButtonColorDarkTheme
05-21 15:44:48.220 15777 16148 I DOTNET : [PASS] AlertDialogButtonColorLightTheme
05-21 15:44:48.221 15777 16148 I DOTNET : Microsoft.Maui.DeviceTests.AlertDialogTests 0.0895158 ms
05-21 15:44:48.417 15777 15817 I DOTNET : Xml file was written to the provided writer.
05-21 15:44:48.417 15777 15817 I DOTNET : Tests run: 580 Passed: 44 Inconclusive: 0 Failed: 0 Ignored: 536
^[[40m^[[32minfo^[[39m^[[22m^[[49m: <<XHARNESS_RESULT_START>>
{
"version": 1,
"machineName": "runnervmfuwn7",
"exitCode": 0,
"exitCodeName": "SUCCESS",
"platform": "android",
"instrumentationExitCode": 0,
"device": "emulator-5554",
"deviceOsVersion": "API 30",
"architecture": "x86_64",
"files": [
{
"name": "testResults.xml",
"type": "test-results"
},
{
"name": "adb-logcat-com.microsoft.maui.controls.devicetests-default.log",
"type": "logcat"
}
]
}
<<XHARNESS_RESULT_END>>
^[[40m^[[32minfo^[[39m^[[22m^[[49m: Attempting to remove apk 'com.microsoft.maui.controls.devicetests'..
^[[40m^[[37mdbug^[[39m^[[22m^[[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 uninstall com.microsoft.maui.controls.devicetests'
^[[40m^[[32minfo^[[39m^[[22m^[[49m: Successfully uninstalled com.microsoft.maui.controls.devicetests
XHarness exit code: 0
Tests completed successfully
📁 Fix files reverted (4 files)
.gitignoreeng/pipelines/ci-copilot.ymlsrc/Controls/src/Core/Compatibility/Handlers/TabbedPage/iOS/TabbedRenderer.cssrc/Controls/src/Core/Platform/Android/TabbedPageManager.cs
🧪 UI Tests — TabbedPage
Detected UI test categories: TabbedPage
✅ Deep UI tests — 73 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 |
|---|---|---|
TabbedPage |
73/73 ✓ | — |
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs) |
🔍 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
Issue: #35469 - TabbedPage leaks renderer/manager when BarBackground uses shared GradientBrush resource
PR: #35543 - [WIP] [Android & iOS] TabbedPage leaks with shared GradientBrush.
Platforms Affected: Android, iOS
Files Changed: 2 implementation, 1 test
Key Findings
- Shared app-resource
GradientBrushcan retainTabbedPageManager/TabbedRendererthroughInvalidateGradientBrushRequestedafter theTabbedPageis removed. - PR fix unsubscribes only from Android
SetElement(null)and iOSDispose(true), which is correct for full disconnect/disposal but questionable for modal-pop timing covered by the added tests. - Existing inline review notes the preferred alternatives are lifecycle hooks that fire during page disappearance/removal or weak-event subscription.
- Test type: DeviceTest (
Controls.DeviceTests), Android target,TabbedPage/Memoryimpact.
Code Review Summary
Verdict: NEEDS_CHANGES
Confidence: medium
Errors: 1 | Warnings: 1 | Suggestions: 1
Key code review findings:
- ❌
src/Controls/src/Core/Platform/Android/TabbedPageManager.cs:133/src/Controls/src/Core/Compatibility/Handlers/TabbedPage/iOS/TabbedRenderer.cs:157— cleanup is tied to disconnect/dispose, but modal-pop tests exercise timing where those hooks may not run synchronously. ⚠️ src/Controls/tests/DeviceTests/Elements/TabbedPage/TabbedPageTests.cs:614— reflection helper returns[]when the backing field is missing, which can silently make the leak assertion a no-op.- 💡
src/Controls/tests/DeviceTests/Elements/TabbedPage/TabbedPageTests.cs:528—tabbedPageRefis assigned but not asserted viaWaitForGC.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35543 | Unsubscribe GradientBrush.InvalidateGradientBrushRequested and clear Parent during Android SetElement(null) and iOS Dispose(true) |
✅ PASSED (Gate) | TabbedRenderer.cs, TabbedPageManager.cs, TabbedPageTests.cs |
Original PR; gate already verified fail-without/pass-with-fix |
🔬 Code Review — Deep Analysis
Code Review — PR #35543
Independent Assessment
What this changes: Adds disconnect/dispose-time cleanup for TabbedPage.BarBackground when it is a GradientBrush, clearing its Parent and unsubscribing InvalidateGradientBrushRequested in Android TabbedPageManager.SetElement(null) and iOS TabbedRenderer.Dispose(bool). Adds device tests that push/pop a modal TabbedPage using a shared gradient brush and assert the brush has no remaining event subscribers.
Inferred motivation: A shared/app-resource GradientBrush can otherwise hold the platform manager/renderer alive through its event delegate after the TabbedPage is removed.
Reconciliation with PR Narrative
Author claims: The PR fixes the shared-gradient TabbedPage memory leak on Android and iOS and adds direct/style regression tests for issue #35469.
Agreement/disagreement: The root cause matches the code: UpdateBarBackground() subscribes to the shared brush and previously only unsubscribed when the brush changed. I agree cleanup is needed. However, the chosen cleanup hooks are tied to handler disconnect/dispose, while the new tests exercise modal pop/removal paths where disconnect/dispose timing is platform-dependent; existing inline review comments already raised this lifecycle concern, and the code paths support treating it as blocking/at least requiring confirmation.
Findings
❌ Error — Cleanup is tied to disconnect/dispose, but the added tests exercise modal-pop timing
src/Controls/src/Core/Platform/Android/TabbedPageManager.cs:133 and src/Controls/src/Core/Compatibility/Handlers/TabbedPage/iOS/TabbedRenderer.cs:157 clean the brush only when SetElement(null) / Dispose(true) runs. For Android modal pop, PopModalPlatformAsync(false) calls DialogFragment.Dismiss() and completes before ModalFragment.OnDismiss() nulls the modal handler. For iOS modal pop, PopModalPlatformAsync dismisses ControlsModalWrapper, but ControlsModalWrapper.Dispose only clears its _modal reference and does not explicitly disconnect the child handler. The new tests assert no brush subscribers after PopModalAsync, so the fix depends on asynchronous/native disposal timing rather than a deterministic removal hook. Existing MauiBot inline comments already flag this same lifecycle problem; I would not duplicate those as new GitHub comments, but I agree this needs to be addressed or empirically disproven with the device tests.
⚠️ Warning — Reflection helper can silently turn the regression assertion into a no-op
src/Controls/tests/DeviceTests/Elements/TabbedPage/TabbedPageTests.cs:614 returns [] if the private event backing field is not found. Since the test then asserts Assert.Empty(...), a field rename or custom event accessor would make the leak assertion pass without checking anything. Prefer failing loudly (for example, throw InvalidOperationException) when the field lookup fails.
💡 Suggestion — Remove or assert the unused tabbedPageRef
src/Controls/tests/DeviceTests/Elements/TabbedPage/TabbedPageTests.cs:528 creates and assigns tabbedPageRef but only waits on handlerRef. Either pass both references to WaitForGC if page collection is part of the intended coverage, or remove the unused reference to avoid implying it is asserted.
Devil's Advocate
The cleanup added in SetElement(null)/Dispose(true) is correct for explicit handler disconnect and likely covers the issue's Window.Page replacement path (Window calls oldPage.DisconnectHandlers()). The concern is specifically the PR's modal-pop regression coverage and any real removal paths where platform dismissal completes before handler disconnect/dispose. I could not run Android/iOS device tests in this Linux environment, so the lifecycle finding should be verified by running the added tests on those platforms; CI shown by gh pr checks is green but does not prove these device tests ran.
Verdict: NEEDS_CHANGES
Confidence: medium
Summary: The unsubscribe logic is directionally correct, but it is placed in hooks that are not deterministic for the modal-pop scenario covered by the new tests. The test helper also has a silent-pass reflection fallback. Required CI checks currently pass, but platform device-test evidence is still needed for the changed lifecycle behavior.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix-1 | Move Android brush event unsubscribe/resubscribe to TabbedPage Disappearing/Appearing lifecycle. |
✅ PASS | 1 file | Android TabbedPage tests passed, but post-test expert analysis found it only addressed the event delegate path and left GradientBrush.Parent ownership untouched, so it was not selected. |
| 2 | try-fix-2 | Use a weak GradientBrush.InvalidateGradientBrushRequested subscriber plus parent-ownership tracking and centralized teardown in Android TabbedPageManager. |
✅ PASS | 1 file | Selected: different root-cause strategy from PR and candidate #1; avoids strong delegate retention and avoids assigning Parent for shared/app-resource brushes. |
| PR | PR #35543 | Unsubscribe and clear brush parent during Android SetElement(null) and iOS Dispose(true). |
✅ PASSED (Gate) | 3 files | Gate already verified fail-without/pass-with-fix, but code review found cleanup depends on disconnect/dispose timing. |
Candidate Narratives
try-fix-1/content.md— lifecycle Appearing/Disappearing cleanup. PassedCategory=TabbedPageAndroid device tests: 44/44 filtered tests, 0 failed. Not selected because it left the brush parent lifecycle unaddressed.try-fix-2/content.md— weak-event subscription plus parent-aware handling. PassedCategory=TabbedPageAndroid device tests: 44/44 filtered tests, 0 failed. Self-review recorded one minor cleanup finding (OnBarBackgroundChangedbecomes dead code), not a correctness blocker.
Cross-Pollination / Learning
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 1 | Yes | Lifecycle cleanup on Disappearing/Appearing passed tests but taught that event-only cleanup may leave the shared brush Parent reference as a second retention path. |
| claude-opus-4.7 | 1 | Yes | Weak subscription plus parent-ownership tracking passed tests and addressed both strong delegate and brush parent retention paths. |
Exhausted: No — stopped because candidate #2 passed the Android regression suite and is demonstrably more robust than the PR's disconnect-only cleanup.
Selected Fix: Candidate #2 — It does not rely solely on SetElement(null)/dispose timing, avoids the shared brush holding a strong delegate to TabbedPageManager, and avoids taking Parent ownership of app-resource/shared brushes while still clearing deterministic subscriptions for the existing strict regression assertions.
📋 Report — Final Recommendation
Comparative Report - PR #35543
Candidates compared
| Candidate | Approach | Regression result | Ranking |
|---|---|---|---|
try-fix-2 |
Android weak GradientBrush event subscriber plus parent-ownership tracking and centralized teardown. |
PASS | 1 |
pr-plus-reviewer |
Raw PR implementation plus expert-reviewer test robustness fixes. | PASS by gate inheritance; reviewer changes are test-only | 2 |
pr |
Raw PR fix: unsubscribe and clear brush parent in Android SetElement(null) and iOS Dispose(true). |
PASS (gate) | 3 |
try-fix-1 |
Android unsubscribe/resubscribe around TabbedPage Disappearing/Appearing. |
PASS | 4 |
Analysis
The raw PR addresses the direct leak symptom by unsubscribing GradientBrush.InvalidateGradientBrushRequested and clearing GradientBrush.Parent during Android disconnect and iOS disposal. The gate passed, so it is a valid improvement over the broken baseline. Its weakness is that both Android and iOS still depend on disconnect/dispose happening at the relevant modal-pop/window-removal timing, and Android still allows the shared brush to hold both a strong delegate and a Parent back-reference until that cleanup runs.
pr-plus-reviewer is better than the raw PR because it applies the expert review's actionable feedback to the tests: the direct-brush test now verifies the TabbedPage weak reference as intended, and the reflection helper fails loudly if the event backing field cannot be found. Those changes improve regression coverage but do not change the implementation, so the same lifecycle-timing and shared-brush ownership concerns remain.
try-fix-1 passed the Android TabbedPage regression suite, but it only moves event unsubscription to Disappearing/Appearing. It does not address the GradientBrush.Parent retention path and couples a brush subscription to page visibility rather than handler lifetime. That makes it less complete than both PR variants despite the passing test result.
try-fix-2 is the strongest Android candidate. It passed the same Android TabbedPage regression suite and addresses both root retention paths: the shared brush no longer strongly roots TabbedPageManager through the event delegate, and parent ownership is tracked so a shared/app-resource brush is not unconditionally given a Parent back-reference to the page graph. It also keeps deterministic teardown so the strict invocation-list assertions still pass.
Winner
Winner: try-fix-2
try-fix-2 wins because it passed regression testing and provides the most robust Android fix: it does not rely solely on disconnect timing and it covers both the strong delegate and brush-parent retention paths. pr-plus-reviewer is the best PR-based candidate because it improves the tests, but it remains implementation-equivalent to the raw PR for the leak mechanism.
kubaflo
left a comment
There was a problem hiding this comment.
Could you please review ai's suggestions?
|
/review -b feature/refactor-copilot-yml |
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 2 findings
See inline comments for details.
@kubaflo The summary concerns were SetElement(null) / Dispose(true) may not run on popModalAsync . This were tested on both Android and iOS and confirm zero brush subscribers after popModalAsync with original fix. Also included Test to cover this scenario |
|
/review -b feature/refactor-copilot-yml |
MauiBot
left a comment
There was a problem hiding this comment.
🤖 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 won because it passed the Android TabbedPage regression run and fixes the modal-pop timing issue by detaching the shared GradientBrush subscription on Disappearing, rather than waiting for handler disconnect. The raw PR passed the gate but retains lifecycle timing concerns, while pr-plus-reviewer improves reviewer findings without adding the tested Android lifecycle fix.
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/Compatibility/Handlers/TabbedPage/iOS/TabbedRenderer.cs b/src/Controls/src/Core/Compatibility/Handlers/TabbedPage/iOS/TabbedRenderer.cs
index 3ab57244b6..63a2158390 100644
--- a/src/Controls/src/Core/Compatibility/Handlers/TabbedPage/iOS/TabbedRenderer.cs
+++ b/src/Controls/src/Core/Compatibility/Handlers/TabbedPage/iOS/TabbedRenderer.cs
@@ -117,80 +117,90 @@ namespace Microsoft.Maui.Controls.Handlers.Compatibility
View.SetNeedsLayout();
}
public override void ViewDidAppear(bool animated)
{
Page?.SendAppearing();
base.ViewDidAppear(animated);
}
public override void ViewDidDisappear(bool animated)
{
base.ViewDidDisappear(animated);
Page?.SendDisappearing();
}
public override void ViewDidLayoutSubviews()
{
base.ViewDidLayoutSubviews();
// On iPadOS 26.1+, ViewDidLayoutSubviews can be called during UITabBarController construction
// in narrow viewports (< 667 points) before Element is set. Guard against this.
if (Element is IView view)
view.Arrange(View.Bounds.ToRectangle());
}
protected override void Dispose(bool disposing)
{
if (disposing)
{
_tabBarAppearance?.Dispose();
_tabBarAppearance = null;
Page?.SendDisappearing();
if (Tabbed is TabbedPage tabbed)
{
tabbed.PropertyChanged -= OnPropertyChanged;
tabbed.PagesChanged -= OnPagesChanged;
}
+ if (_currentBarBackground is GradientBrush currentGradientBrush)
+ {
+ if (ReferenceEquals(currentGradientBrush.Parent, Tabbed))
+ {
+ currentGradientBrush.Parent = null;
+ }
+ currentGradientBrush.InvalidateGradientBrushRequested -= OnBarBackgroundChanged;
+ }
+ _currentBarBackground = null;
+
FinishedCustomizingViewControllers -= HandleFinishedCustomizingViewControllers;
}
base.Dispose(disposing);
}
protected virtual void OnElementChanged(VisualElementChangedEventArgs e)
{
var changed = ElementChanged;
if (changed != null)
changed(this, e);
}
UIViewController GetViewController(Page page)
{
if (page?.Handler is not IPlatformViewHandler nvh)
return null;
return nvh.ViewController;
}
void HandleFinishedCustomizingViewControllers(object sender, UITabBarCustomizeChangeEventArgs e)
{
if (e.Changed)
UpdateChildrenOrderIndex(e.ViewControllers);
}
void OnPagePropertyChanged(object sender, PropertyChangedEventArgs e)
{
// Setting TabBarItem.Title in iOS 10 causes rendering bugs
// Work around this by creating a new UITabBarItem on each change
if (e.PropertyName == Page.IconImageSourceProperty.PropertyName || e.PropertyName == Page.TitleProperty.PropertyName)
{
var page = (Page)sender;
UpdateTabBarItem(page);
}
}
public override void TraitCollectionDidChange(UITraitCollection previousTraitCollection)
{
diff --git a/src/Controls/src/Core/Platform/Android/TabbedPageManager.cs b/src/Controls/src/Core/Platform/Android/TabbedPageManager.cs
index f885d7673f..f7ee2facf0 100644
--- a/src/Controls/src/Core/Platform/Android/TabbedPageManager.cs
+++ b/src/Controls/src/Core/Platform/Android/TabbedPageManager.cs
@@ -92,80 +92,90 @@ public class TabbedPageManager
{
if (Element.IsSet(TabbedPage.UnselectedTabColorProperty))
return Element.UnselectedTabColor;
}
return null;
}
}
public Color BarSelectedItemColor
{
get
{
if (Element != null)
{
if (Element.IsSet(TabbedPage.SelectedTabColorProperty))
return Element.SelectedTabColor;
}
return null;
}
}
public virtual void SetElement(TabbedPage tabbedPage)
{
var activity = _context.GetActivity();
var themeContext = activity;
if (Element is not null)
{
Element.InternalChildren.ForEach(page => TeardownPage(page as Page));
((IPageController)Element).InternalChildren.CollectionChanged -= OnChildrenCollectionChanged;
Element.Appearing -= OnTabbedPageAppearing;
Element.Disappearing -= OnTabbedPageDisappearing;
RemoveTabs();
_viewPager.LayoutChange -= OnLayoutChanged;
_viewPager.Adapter = null;
+
+ if (_currentBarBackground is GradientBrush currentGradientBrush)
+ {
+ if (ReferenceEquals(currentGradientBrush.Parent, Element))
+ {
+ currentGradientBrush.Parent = null;
+ }
+ currentGradientBrush.InvalidateGradientBrushRequested -= OnBarBackgroundChanged;
+ }
+ _currentBarBackground = null;
}
Element = tabbedPage;
if (Element is not null)
{
_viewPager.LayoutChange += OnLayoutChanged;
Element.Appearing += OnTabbedPageAppearing;
Element.Disappearing += OnTabbedPageDisappearing;
_viewPager.Adapter = new MultiPageFragmentStateAdapter<Page>(tabbedPage, FragmentManager, _context) { CountOverride = tabbedPage.Children.Count };
if (IsBottomTabPlacement)
{
_bottomNavigationView = new BottomNavigationView(_context.Context)
{
LayoutParameters = new CoordinatorLayout.LayoutParams(AppBarLayout.LayoutParams.MatchParent, AppBarLayout.LayoutParams.WrapContent)
{
Gravity = (int)GravityFlags.Bottom
}
};
}
else
{
if (_tabLayout == null)
{
var layoutInflater = Element.Handler.MauiContext.GetLayoutInflater();
_tabLayout = new TabLayout(_context.Context)
{
TabMode = TabLayout.ModeFixed,
TabGravity = TabLayout.GravityFill,
LayoutParameters = new AppBarLayout.LayoutParams(AppBarLayout.LayoutParams.MatchParent, AppBarLayout.LayoutParams.WrapContent)
};
}
}
OnChildrenCollectionChanged(null, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
ScrollToCurrentPage();
@@ -185,92 +195,95 @@ public class TabbedPageManager
void RemoveTabs()
{
_pendingFragment?.Dispose();
_pendingFragment = null;
if (_tabLayoutFragment != null)
{
var fragment = _tabLayoutFragment;
_tabLayoutFragment = null;
var fragmentManager =
_context
.GetNavigationRootManager()
.FragmentManager;
if (!fragmentManager.IsDestroyed(_context?.Context))
{
SetContentBottomMargin(0);
if (_context?.Context is Context c)
{
_pendingFragment =
fragmentManager
.RunOrWaitForResume(c, fm =>
{
fm
.BeginTransaction()
.Remove(fragment)
.SetReorderingAllowed(true)
.Commit();
});
}
}
_tabplacementId = 0;
}
}
protected virtual void OnTabbedPageDisappearing(object sender, EventArgs e)
{
+ DetachBarBackgroundBrush();
+
// Don't remove tabs during modal navigation — tabs should remain visible so they
// restore properly when the modal is dismissed.
// For non-modal navigation (e.g. NavigationPage.PushAsync), tabs must be removed so that
// the newly pushed page can use the full available height.
if (Element?.Navigation?.ModalStack?.Count > 0)
return;
RemoveTabs();
}
protected virtual void OnTabbedPageAppearing(object sender, EventArgs e)
{
+ AttachBarBackgroundBrush();
SetTabLayout();
}
protected virtual void RootViewChanged(object sender, EventArgs e)
{
if (sender is NavigationRootManager rootManager)
{
rootManager.RootViewChanged -= RootViewChanged;
SetTabLayout();
}
}
internal void SetTabLayout()
{
_pendingFragment?.Dispose();
_pendingFragment = null;
int id;
var rootManager =
_context.GetNavigationRootManager();
_tabItemStyleLoaded = false;
if (rootManager.RootView == null)
{
rootManager.RootViewChanged += RootViewChanged;
return;
}
if (IsBottomTabPlacement)
{
id = Resource.Id.navigationlayout_bottomtabs;
if (_tabplacementId == id)
return;
SetContentBottomMargin(_context.Context.Resources.GetDimensionPixelSize(Resource.Dimension.design_bottom_navigation_height));
}
else
{
id = Resource.Id.navigationlayout_toptabs;
if (_tabplacementId == id)
@@ -581,80 +594,97 @@ public class TabbedPageManager
if (tintColor == null)
_tabLayout.BackgroundTintMode = null;
else
{
_tabLayout.BackgroundTintMode = PorterDuff.Mode.Src;
_tabLayout.BackgroundTintList = ColorStateList.ValueOf(tintColor.ToPlatform());
}
}
}
public virtual void UpdateBarBackground()
{
if (_currentBarBackground == Element.BarBackground)
return;
if (_currentBarBackground is GradientBrush oldGradientBrush)
{
oldGradientBrush.Parent = null;
oldGradientBrush.InvalidateGradientBrushRequested -= OnBarBackgroundChanged;
}
_currentBarBackground = Element.BarBackground;
if (_currentBarBackground is GradientBrush newGradientBrush)
{
newGradientBrush.Parent = Element;
newGradientBrush.InvalidateGradientBrushRequested += OnBarBackgroundChanged;
if (_bottomNavigationView is not null && _bottomNavigationView.Elevation > 0)
{
_bottomNavigationView.Elevation = 0;
}
}
else if (_currentBarBackground is SolidColorBrush && _bottomNavigationView is not null && _bottomNavigationView.Elevation == 0)
{
_bottomNavigationView.Elevation = _bottomNavigationView.Context.Resources.GetDimension(Resource.Dimension.design_bottom_navigation_elevation);
}
RefreshBarBackground();
}
+ void DetachBarBackgroundBrush()
+ {
+ if (_currentBarBackground is GradientBrush currentGradientBrush)
+ {
+ currentGradientBrush.InvalidateGradientBrushRequested -= OnBarBackgroundChanged;
+ }
+ }
+
+ void AttachBarBackgroundBrush()
+ {
+ if (_currentBarBackground is GradientBrush currentGradientBrush)
+ {
+ currentGradientBrush.InvalidateGradientBrushRequested -= OnBarBackgroundChanged;
+ currentGradientBrush.InvalidateGradientBrushRequested += OnBarBackgroundChanged;
+ }
+ }
+
void OnBarBackgroundChanged(object sender, EventArgs e)
{
RefreshBarBackground();
}
protected virtual void RefreshBarBackground()
{
if (IsBottomTabPlacement)
_bottomNavigationView.UpdateBackground(_currentBarBackground);
else
_tabLayout.UpdateBackground(_currentBarBackground);
}
protected virtual ColorStateList GetItemTextColorStates()
{
if (_originalTabTextColors is null)
_originalTabTextColors = IsBottomTabPlacement ? _bottomNavigationView.ItemTextColor : _tabLayout.TabTextColors;
Color barItemColor = BarItemColor;
Color barTextColor = Element.BarTextColor;
Color barSelectedItemColor = BarSelectedItemColor;
if (barItemColor is null && barTextColor is null && barSelectedItemColor is null)
return _originalTabTextColors;
if (_newTabTextColors is not null)
return _newTabTextColors;
int checkedColor;
// The new default color to use may have a color if BarItemColor is not null or the original colors for text
// are not null either. If it does not happens, this variable will be null and the ColorStateList of the
// original colors is used.
int? defaultColor = null;
if (barTextColor is not null)
{
checkedColor = barTextColor.ToPlatform().ToArgb();
defaultColor = checkedColor;
}
diff --git a/src/Controls/src/Core/Toolbar/Toolbar.Android.cs b/src/Controls/src/Core/Toolbar/Toolbar.Android.cs
index 34946e48b3..057e1e3538 100644
--- a/src/Controls/src/Core/Toolbar/Toolbar.Android.cs
+++ b/src/Controls/src/Core/Toolbar/Toolbar.Android.cs
@@ -1,76 +1,87 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using Android.Content;
using Android.Runtime;
using Android.Views;
using Google.Android.Material.AppBar;
using Microsoft.Maui.Controls.Platform;
using Microsoft.Maui.Handlers;
using AToolbar = AndroidX.AppCompat.Widget.Toolbar;
using LP = Android.Views.ViewGroup.LayoutParams;
namespace Microsoft.Maui.Controls
{
public partial class Toolbar
{
IViewHandler? _platformTitleViewHandler;
Container? _platformTitleView;
List<IMenuItem> _currentMenuItems = new List<IMenuItem>();
List<ToolbarItem> _currentToolbarItems = new List<ToolbarItem>();
- Brush _currentBarBackground;
+ Brush? _currentBarBackground;
private int? _defaultStartInset;
NavigationRootManager? NavigationRootManager =>
Handler?.MauiContext?.GetNavigationRootManager();
MaterialToolbar PlatformView => Handler?.PlatformView as MaterialToolbar ?? throw new InvalidOperationException("Native View not set");
partial void OnHandlerChanging(IElementHandler oldHandler, IElementHandler newHandler)
{
if (newHandler == null)
{
if (_platformTitleView != null)
_platformTitleView.Child = null;
+ if (_currentBarBackground is GradientBrush currentGradientBrush)
+ {
+ if (ReferenceEquals(currentGradientBrush.Parent, this))
+ {
+ currentGradientBrush.Parent = null;
+ }
+
+ currentGradientBrush.InvalidateGradientBrushRequested -= OnBarBackgroundChanged;
+ }
+ _currentBarBackground = null;
+
Controls.Platform.ToolbarExtensions.DisposeMenuItems(
oldHandler?.PlatformView as AToolbar,
ToolbarItems,
OnToolbarItemPropertyChanged);
}
else if (newHandler.MauiContext?.Context != oldHandler?.MauiContext?.Context)
{
// Platform Title View is from a previous activity context so we just want to null it out
_platformTitleView = null;
}
}
void OnToolbarItemPropertyChanged(object? sender, PropertyChangedEventArgs e)
{
var toolbarItems = ToolbarItems;
List<ToolbarItem> newToolBarItems = new List<ToolbarItem>();
if (toolbarItems != null)
newToolBarItems.AddRange(toolbarItems);
if (sender is ToolbarItem ti)
PlatformView.OnToolbarItemPropertyChanged(e, ti, newToolBarItems, MauiContext!, BarTextColor, OnToolbarItemPropertyChanged, _currentMenuItems, _currentToolbarItems, UpdateMenuItemIcon);
}
void UpdateMenuItemIcon(Context context, IMenuItem menuItem, ToolbarItem toolBarItem)
{
_ = MauiContext ?? throw new ArgumentNullException(nameof(MauiContext));
MauiContext.UpdateMenuItemIcon(menuItem, toolBarItem, null);
}
void UpdateMenu()
{
_ = MauiContext ?? throw new ArgumentNullException(nameof(MauiContext));
if (_currentMenuItems == null)
return;
PlatformView.UpdateMenuItems(ToolbarItems, MauiContext, BarTextColor, OnToolbarItemPropertyChanged, _currentMenuItems, _currentToolbarItems, UpdateMenuItemIcon);
}
void UpdateTitleView()
diff --git a/src/Controls/tests/DeviceTests/Elements/TabbedPage/TabbedPageTests.cs b/src/Controls/tests/DeviceTests/Elements/TabbedPage/TabbedPageTests.cs
index f8ddc4ac8c..a46e60c6ca 100644
--- a/src/Controls/tests/DeviceTests/Elements/TabbedPage/TabbedPageTests.cs
+++ b/src/Controls/tests/DeviceTests/Elements/TabbedPage/TabbedPageTests.cs
@@ -1,44 +1,45 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
+using System.Reflection;
using System.Text;
using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Maui;
using Microsoft.Maui.Controls;
using Microsoft.Maui.Controls.Handlers;
using Microsoft.Maui.Controls.Handlers.Compatibility;
using Microsoft.Maui.DeviceTests.Stubs;
using Microsoft.Maui.Graphics;
using Microsoft.Maui.Handlers;
using Microsoft.Maui.Hosting;
using Microsoft.Maui.Platform;
using Xunit;
#if IOS
using TabbedViewHandler = Microsoft.Maui.Controls.Handlers.Compatibility.TabbedRenderer;
#endif
#if WINDOWS
using WSolidColorBrush = Microsoft.UI.Xaml.Media.SolidColorBrush;
#endif
namespace Microsoft.Maui.DeviceTests
{
[Category(TestCategory.TabbedPage)]
public partial class TabbedPageTests : ControlsHandlerTestBase
{
void SetupBuilder(Action<MauiAppBuilder> additionalCreationActions = null)
{
EnsureHandlerCreated(builder =>
{
builder.ConfigureMauiHandlers(handlers =>
{
handlers.AddHandler(typeof(VerticalStackLayout), typeof(LayoutHandler));
handlers.AddHandler(typeof(Toolbar), typeof(ToolbarHandler));
handlers.AddHandler(typeof(Button), typeof(ButtonHandler));
handlers.AddHandler<Page, PageHandler>();
handlers.AddHandler<Label, LabelHandler>();
#if IOS || MACCATALYST
handlers.AddHandler(typeof(TabbedPage), typeof(TabbedRenderer));
@@ -341,81 +342,81 @@ namespace Microsoft.Maui.DeviceTests
foreach (var navigationPage in pages)
{
tabbedPage.CurrentPage = navigationPage;
await OnNavigatedToAsync(navigationPage.CurrentPage);
await OnLoadedAsync((navigationPage.CurrentPage as ContentPage).Content);
await Task.Delay(200);
await navigationPage.PopAsync();
await OnNavigatedToAsync(navigationPage.CurrentPage);
await OnLoadedAsync((navigationPage.CurrentPage as ContentPage).Content);
}
});
}
#if !WINDOWS
[Theory]
[ClassData(typeof(TabbedPagePivots))]
public async Task RemovingAllPagesDoesntCrash(bool bottomTabs, bool isSmoothScrollEnabled)
{
SetupBuilder();
var tabbedPage = CreateBasicTabbedPage(bottomTabs, isSmoothScrollEnabled);
var secondPage = new NavigationPage(new ContentPage()) { Title = "Second Page" };
tabbedPage.Children.Add(secondPage);
var firstPage = tabbedPage.Children[0];
await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(tabbedPage), async (handler) =>
{
await OnNavigatedToAsync(firstPage);
tabbedPage.Children.Remove(firstPage);
tabbedPage.Children.Remove(secondPage);
await OnUnloadedAsync(secondPage);
tabbedPage.Children.Insert(0, secondPage);
await OnNavigatedToAsync(secondPage);
Assert.Equal(tabbedPage.CurrentPage, secondPage);
});
}
#endif
-#if IOS
+#if IOS
[Theory(Skip = "Test doesn't work on iOS yet; probably because of https://github.com/dotnet/maui/issues/10591")]
#elif WINDOWS
[Theory(Skip = "Test doesn't work on Windows")]
#elif MACCATALYST
[Theory(Skip = "Test doesn't work on Catalyst macos 26")]
#else
[Theory]
#endif
#if ANDROID
[InlineData(true)]
#endif
[InlineData(false)]
public async Task NavigatingAwayFromTabbedPageResizesContentPage(bool bottomTabs)
{
SetupBuilder();
var tabbedPage = CreateBasicTabbedPage(bottomTabs);
var navPage = new NavigationPage(tabbedPage);
var layout1 = new VerticalStackLayout()
{
Background = SolidColorBrush.Green
};
(tabbedPage.CurrentPage as ContentPage).Content = layout1;
await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async (handler) =>
{
await OnFrameSetToNotEmpty(layout1);
var pageHeight = tabbedPage.CurrentPage.Height;
var newPage = new ContentPage()
{
Background = SolidColorBrush.Purple,
Content = new VerticalStackLayout()
};
await navPage.PushAsync(newPage);
await OnFrameSetToNotEmpty(newPage);
Assert.True(newPage.Content.Height > pageHeight);
});
@@ -466,42 +467,201 @@ namespace Microsoft.Maui.DeviceTests
pageReference = new WeakReference(page);
await navPage.Navigation.PushAsync(page);
await navPage.Navigation.PopAsync();
});
await AssertionExtensions.WaitForGC(pageReference);
}
TabbedPage CreateBasicTabbedPage(bool bottomTabs = false, bool isSmoothScrollEnabled = true, IEnumerable<Page> pages = null)
{
pages = pages ?? new List<Page>()
{
new ContentPage() { Title = "Page 1", IconImageSource = "white.png" }
};
var tabs = new TabbedPage()
{
Title = "Tabbed Page",
Background = SolidColorBrush.Green
};
foreach (var page in pages)
{
tabs.Children.Add(page);
}
if (bottomTabs)
{
Controls.PlatformConfiguration.AndroidSpecific.TabbedPage.SetToolbarPlacement(tabs, Controls.PlatformConfiguration.AndroidSpecific.ToolbarPlacement.Bottom);
}
else
{
Controls.PlatformConfiguration.AndroidSpecific.TabbedPage.SetToolbarPlacement(tabs,
Controls.PlatformConfiguration.AndroidSpecific.ToolbarPlacement.Top);
}
Controls.PlatformConfiguration.AndroidSpecific.TabbedPage.SetIsSmoothScrollEnabled(tabs, isSmoothScrollEnabled);
return tabs;
}
+
+ // https://github.com/dotnet/maui/issues/35469
+ // When a TabbedPage.BarBackground is set from a shared GradientBrush (e.g. via an app-resource Style),
+ // removing the TabbedPage from the window must not leave a live subscription on the shared brush.
+ // The renderer/manager subscribes to GradientBrush.InvalidateGradientBrushRequested; if it fails to
+ // unsubscribe on disconnect the brush keeps the renderer alive, preventing GC.
+ [Fact(DisplayName = "TabbedPage renderer/manager does not leak shared GradientBrush subscriber on disconnect")]
+ public async Task TabbedPageDoesNotLeakGradientBrushSubscriberOnDisconnect()
+ {
+ SetupBuilder();
+
+ // Simulate a shared app-resource brush — this object lives beyond the TabbedPage.
+ var sharedBrush = new LinearGradientBrush(
+ new GradientStopCollection
+ {
+ new GradientStop(Colors.Teal, 0f),
+ new GradientStop(Colors.CornflowerBlue, 1f)
+ },
+ new Graphics.Point(0, 0),
+ new Graphics.Point(1, 1));
+
+ var tabbedPageRef = new WeakReference(null);
+ var handlerRef = new WeakReference(null);
+
+ var navPage = new NavigationPage(new ContentPage { Title = "Home" });
+
+ await CreateHandlerAndAddToWindow(new Window(navPage), async () =>
+ {
+ var tabbedPage = new TabbedPage
+ {
+ Title = "Tabbed",
+ BarBackground = sharedBrush,
+ };
+ tabbedPage.Children.Add(new ContentPage { Title = "Tab 1" });
+ tabbedPage.Children.Add(new ContentPage { Title = "Tab 2" });
+
+ // Push so the TabbedPage handler is created and subscribes to the brush.
+ await navPage.Navigation.PushModalAsync(tabbedPage);
+ await OnLoadedAsync(tabbedPage.Children[0]);
+
+ tabbedPageRef.Target = tabbedPage;
+ handlerRef.Target = tabbedPage.Handler;
+
+ // Pop — this should cause the renderer/manager to disconnect and unsubscribe.
+ await navPage.Navigation.PopModalAsync();
+ });
+
+ // After full GC the renderer/manager and the page itself should be collected.
+ await AssertionExtensions.WaitForGC(handlerRef);
+ await AssertionExtensions.WaitForGC(tabbedPageRef);
+
+ // The shared brush must have zero subscribers to InvalidateGradientBrushRequested.
+ // If the renderer/manager leaked, it would still be subscribed here.
+ var brushInvocationList = GetGradientBrushInvocationList(sharedBrush);
+ Assert.Empty(brushInvocationList);
+ }
+
+ // https://github.com/dotnet/maui/issues/35469
+ // Variant: BarBackground applied via a Style (the exact scenario from the bug report).
+ [Fact(DisplayName = "TabbedPage renderer/manager does not leak shared GradientBrush subscriber when BarBackground is set via Style")]
+ public async Task TabbedPageDoesNotLeakGradientBrushSubscriberWhenSetViaStyle()
+ {
+ SetupBuilder();
+
+ var sharedBrush = new LinearGradientBrush(
+ new GradientStopCollection
+ {
+ new GradientStop(Colors.DarkGreen, 0f),
+ new GradientStop(Colors.SteelBlue, 1f)
+ },
+ new Graphics.Point(0, 0),
+ new Graphics.Point(1, 1));
+
+ var barBackgroundStyle = new Style(typeof(TabbedPage))
+ {
+ Setters = { new Setter { Property = TabbedPage.BarBackgroundProperty, Value = sharedBrush } }
+ };
+
+ var handlerRef = new WeakReference(null);
+
+ var navPage = new NavigationPage(new ContentPage { Title = "Home" });
+
+ await CreateHandlerAndAddToWindow(new Window(navPage), async () =>
+ {
+ var tabbedPage = new TabbedPage { Style = barBackgroundStyle };
+ tabbedPage.Children.Add(new ContentPage { Title = "Tab 1" });
+ tabbedPage.Children.Add(new ContentPage { Title = "Tab 2" });
+
+ await navPage.Navigation.PushModalAsync(tabbedPage);
+ await OnLoadedAsync(tabbedPage.Children[0]);
+
+ handlerRef.Target = tabbedPage.Handler;
+
+ await navPage.Navigation.PopModalAsync();
+ });
+
+ await AssertionExtensions.WaitForGC(handlerRef);
+
+ var brushInvocationList = GetGradientBrushInvocationList(sharedBrush);
+ Assert.Empty(brushInvocationList);
+ }
+
+
+ // https://github.com/dotnet/maui/issues/35469
+ // Verifies that after a modal TabbedPage is popped, the shared GradientBrush
+ // has zero subscribers. DisconnectHandlers is NOT called on modal pop, so the
+ // fix must explicitly unsubscribe via the Disappearing/ViewDidDisappear lifecycle hook.
+ [Fact(DisplayName = "TabbedPage GradientBrush subscriber is removed after modal pop")]
+ public async Task TabbedPageGradientBrushSubscriberRemovedAfterModalPop()
+ {
+ SetupBuilder();
+
+ var sharedBrush = new LinearGradientBrush(
+ new GradientStopCollection
+ {
+ new GradientStop(Colors.Purple, 0f),
+ new GradientStop(Colors.Orange, 1f)
+ },
+ new Graphics.Point(0, 0),
+ new Graphics.Point(1, 1));
+
+ var navPage = new NavigationPage(new ContentPage { Title = "Home" });
+
+ await CreateHandlerAndAddToWindow(new Window(navPage), async () =>
+ {
+ var tabbedPage = new TabbedPage
+ {
+ Title = "Tabbed",
+ BarBackground = sharedBrush,
+ };
+ tabbedPage.Children.Add(new ContentPage { Title = "Tab 1" });
+ tabbedPage.Children.Add(new ContentPage { Title = "Tab 2" });
+
+ await navPage.Navigation.PushModalAsync(tabbedPage);
+ await OnLoadedAsync(tabbedPage.Children[0]);
+
+ // Confirm the brush has exactly one subscriber while the page is live.
+ Assert.Single(GetGradientBrushInvocationList(sharedBrush));
+
+ await navPage.Navigation.PopModalAsync();
+
+ // After pop completes, Disappearing/ViewDidDisappear must have fired
+ // and explicitly unsubscribed from the brush.
+ var invocationList = GetGradientBrushInvocationList(sharedBrush);
+ Assert.Empty(invocationList);
+ });
+ }
+
+ static System.Delegate[] GetGradientBrushInvocationList(GradientBrush brush)
+ {
+ var field = typeof(GradientBrush).GetField(
+ "InvalidateGradientBrushRequested",
+ BindingFlags.Instance | BindingFlags.NonPublic);
+
+ if (field is null)
+ return [];
+
+ return (field.GetValue(brush) as MulticastDelegate)?.GetInvocationList() ?? [];
+ }
}
}
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the ai's suggestions?
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 whether this change resolves your issue. Thank you!
This pull request addresses a memory leak issue involving
GradientBrushsubscriptions in theTabbedPagerenderer/manager. It ensures that when aTabbedPageis removed from the window, any event subscriptions to a sharedGradientBrushare properly cleaned up, preventing the renderer/manager from being kept alive unintentionally. The changes also add tests to verify this behavior.Description of Change
Memory leak prevention and event unsubscription:
Disposemethod inTabbedRenderer.cs(iOS) and theSetElementmethod inTabbedPageManager.cs(Android) now explicitly unsubscribe from theGradientBrush.InvalidateGradientBrushRequestedevent and clear the brush's parent, ensuring no lingering references when theTabbedPageis disconnected. [1] [2]Automated tests for leak prevention:
TabbedPageTests.csthat verify the renderer/manager does not leak a sharedGradientBrushsubscription when aTabbedPageis removed, both when the brush is set directly and when applied via aStyle. These tests use reflection to check for remaining event subscribers and assert that none remain after GC.InvalidateGradientBrushRequestedevent on aGradientBrushinstance.System.Reflectionimport to support the new test utilities.Issues Fixed
Fixes #35469
Tested the behavior in the following platforms
beforeFixAndroid35496.mov
AfterFixAndroid35496.mov
BeforeFixiOS35469.mov
AfterFixiOS35469.mov