Skip to content

Commit 2317119

Browse files
[android] fix memory leak in TabbedPage (#21218)
* [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 * [windows] skip test I spent 2.5 hours on this, and did not figure it out! Comment for now as original issue is Android. * Somehow duplicate [Fact]
1 parent 1b423ab commit 2317119

File tree

2 files changed

+32
-0
lines changed

2 files changed

+32
-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: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,37 @@ IEnumerator IEnumerable.GetEnumerator()
426426
}
427427
}
428428

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

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

0 commit comments

Comments
 (0)