Skip to content

Commit 2a10342

Browse files
[android] fix memory leak in TabbedPage
Fixes: #17959 Context: https://github.com/VoznichkaSviatoslav1/TabbedPageMemoryLeak In the sample app, I added some `GC.Collect()` calls to force the GC to run, but the following log message never prints: public partial class MyTabbedPage : TabbedPage { public MyTabbedPage() { InitializeComponent(); Console.WriteLine("Constructor was called"); } // Doesn't print!!! ~MyTabbedPage() => Console.WriteLine("Destructor was called"); I took a memory snapshot with `dotnet-gcdump` and found a tree like: SampleApp.MyTabbedPage -> Microsoft.Maui.Controls.Platform.MultiPageFragmentStateAdapter<Microsoft.Maui.Controls.Page> -> Action<Microsoft.Maui.Controls.Platform.AdapterItemKey> I didn't find an explanation of what was holding `MultiPageFragmentStateAdapter`. I tried eliminating the `Action<T>`, but that didn't solve the underlying issue; it just made the object tree shorter: SampleApp.MyTabbedPage -> Microsoft.Maui.Controls.Platform.MultiPageFragmentStateAdapter<Microsoft.Maui.Controls.Page> I could also reproduce the problem in a new device test. Reviewing the code, I found we were doing: _viewPager.Adapter = new MultiPageFragmentStateAdapter<Page>(tabbedPage, FragmentManager, _context) { CountOverride = tabbedPage.Children.Count }; But then in the cleanup code, we never set `Adapter` to `null`: _viewPager.Adapter = null; After this change, the memory is released as expected. I am not exactly sure *why* this leaked, but the change does seem appropriate and fixes the problem. Now the new device test passes & the sample app prints: 03-14 15:06:32.394 6055 6055 I DOTNET : Constructor was called 03-14 15:06:37.008 6055 6089 I DOTNET : Destructor was called
1 parent a648f08 commit 2a10342

File tree

2 files changed

+28
-0
lines changed

2 files changed

+28
-0
lines changed

src/Controls/src/Core/Platform/Android/TabbedPageManager.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ internal void SetElement(TabbedPage tabbedPage)
123123
Element.Disappearing -= OnTabbedPageDisappearing;
124124
RemoveTabs();
125125
_viewPager.LayoutChange -= OnLayoutChanged;
126+
_viewPager.Adapter = null;
126127
}
127128

128129
Element = tabbedPage;

src/Controls/tests/DeviceTests/Elements/TabbedPage/TabbedPageTests.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,33 @@ IEnumerator IEnumerable.GetEnumerator()
426426
}
427427
}
428428

429+
[Fact(DisplayName = "Does Not Leak")]
430+
public async Task DoesNotLeak()
431+
{
432+
SetupBuilder();
433+
WeakReference pageReference = null;
434+
var navPage = new NavigationPage(new ContentPage { Title = "Page 1" });
435+
436+
await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async (handler) =>
437+
{
438+
var page = new TabbedPage
439+
{
440+
Children =
441+
{
442+
new ContentPage
443+
{
444+
Content = new VerticalStackLayout { new Label() },
445+
}
446+
}
447+
};
448+
pageReference = new WeakReference(page);
449+
await navPage.Navigation.PushAsync(page);
450+
await navPage.Navigation.PopAsync();
451+
});
452+
453+
await AssertionExtensions.WaitForGC(pageReference);
454+
}
455+
429456

430457
TabbedPage CreateBasicTabbedPage(bool bottomTabs = false, bool isSmoothScrollEnabled = true, IEnumerable<Page> pages = null)
431458
{

0 commit comments

Comments
 (0)