From d178a458ee1cdae63e1ffaf6f5445000f7b9cd0e Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Tue, 4 Oct 2016 11:47:02 -0600 Subject: [PATCH] Unhook drawer listeners so MDP renderer and pages can be collected (#412) Null out page in custom MDP renderer in Control Gallery so it can be collected Checkpoint Checkpoint Checkpoint Checkpoint Checkpoint Checkpoint --- .../CustomRenderers.cs | 9 + .../Bugzilla44166.cs | 206 +++++++++++++++++++++ .../Xamarin.Forms.Controls.Issues.Shared.projitems | 1 + .../AppCompat/MasterDetailContainer.cs | 34 +++- .../AppCompat/MasterDetailPageRenderer.cs | 5 + .../AppCompat/NavigationPageRenderer.cs | 1 + 6 files changed, 255 insertions(+), 1 deletion(-) create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla44166.cs diff --git a/Xamarin.Forms.ControlGallery.Android/CustomRenderers.cs b/Xamarin.Forms.ControlGallery.Android/CustomRenderers.cs index a550212..0530fc3 100644 --- a/Xamarin.Forms.ControlGallery.Android/CustomRenderers.cs +++ b/Xamarin.Forms.ControlGallery.Android/CustomRenderers.cs @@ -34,6 +34,7 @@ namespace Xamarin.Forms.ControlGallery.Android public class NativeDroidMasterDetail : Xamarin.Forms.Platform.Android.AppCompat.MasterDetailPageRenderer { MasterDetailPage _page; + bool _disposed; protected override void OnElementChanged(VisualElement oldElement, VisualElement newElement) { @@ -61,10 +62,18 @@ namespace Xamarin.Forms.ControlGallery.Android protected override void Dispose(bool disposing) { + if (_disposed) + { + return; + } + + _disposed = true; + if (disposing && _page != null) { _page.LayoutChanged -= Page_LayoutChanged; _page.PropertyChanged -= Page_PropertyChanged; + _page = null; } base.Dispose(disposing); diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla44166.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla44166.cs new file mode 100644 index 0000000..4006faa --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla44166.cs @@ -0,0 +1,206 @@ +using System; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; +using Xamarin.Forms.CustomAttributes; +using Xamarin.Forms.Internals; + +#if UITEST +using Xamarin.UITest; +using NUnit.Framework; +#endif + +namespace Xamarin.Forms.Controls +{ + [Preserve(AllMembers = true)] + [Issue(IssueTracker.Bugzilla, 44166, "MasterDetailPage instances do not get disposed upon GC")] + public class Bugzilla44166 : TestContentPage + { + protected override void Init() + { + var label = new Label() { Text = "Testing..." }; + + var goButton = new Button { Text = "Go" }; + goButton.Clicked += (sender, args) => Application.Current.MainPage = new _44166MDP(); + + var gcButton = new Button { Text = "GC" }; + gcButton.Clicked += (sender, args) => + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + + if (_44166MDP.Counter > 0) + { + Debug.WriteLine($">>>>>>>> Post-GC, {_44166MDP.Counter} {nameof(_44166MDP)} allocated"); + } + + if (_44166Master.Counter > 0) + { + Debug.WriteLine($">>>>>>>> Post-GC, {_44166Master.Counter} {nameof(_44166Master)} allocated"); + } + + if (_44166Detail.Counter > 0) + { + Debug.WriteLine($">>>>>>>> Post-GC, {_44166Detail.Counter} {nameof(_44166Detail)} allocated"); + } + + if (_44166NavContent.Counter > 0) + { + Debug.WriteLine($">>>>>>>> Post-GC, {_44166NavContent.Counter} {nameof(_44166NavContent)} allocated"); + } + + if (_44166NavContent.Counter + _44166Detail.Counter + _44166Master.Counter + _44166MDP.Counter == 0) + { + label.Text = "Success"; + } + }; + + Content = new StackLayout + { + Children = { label, goButton, gcButton } + }; + } + + #if UITEST + [Test] + public void Bugzilla44166Test() + { + RunningApp.WaitForElement(q => q.Marked("Go")); + RunningApp.Tap(q => q.Marked("Go")); + + RunningApp.WaitForElement(q => q.Marked("Back")); + RunningApp.Tap(q => q.Marked("Back")); + + for(int n = 0; n < 10; n++) + { + RunningApp.WaitForElement(q => q.Marked("GC")); + RunningApp.Tap(q => q.Marked("GC")); + + if (RunningApp.Query(q => q.Marked("Success")).Length > 0) + { + return; + } + } + + var pageStats = string.Empty; + + if (_44166MDP.Counter > 0) + { + pageStats += $"{_44166MDP.Counter} {nameof(_44166MDP)} allocated; "; + } + + if (_44166Master.Counter > 0) + { + pageStats += $"{_44166Master.Counter} {nameof(_44166Master)} allocated; "; + } + + if (_44166Detail.Counter > 0) + { + pageStats += $"{_44166Detail.Counter} {nameof(_44166Detail)} allocated; "; + } + + if (_44166NavContent.Counter > 0) + { + pageStats += $"{_44166NavContent.Counter} {nameof(_44166NavContent)} allocated; "; + } + + Assert.Fail($"At least one of the pages was not collected: {pageStats}"); + } + #endif + } + + [Preserve(AllMembers = true)] + public class _44166MDP : MasterDetailPage + { + public static int Counter; + + public _44166MDP() + { + Interlocked.Increment(ref Counter); + Debug.WriteLine($"++++++++ {nameof(_44166MDP)} constructor, {Counter} allocated"); + + Master = new _44166Master(); + Detail = new _44166Detail(); + } + + ~_44166MDP() + { + Interlocked.Decrement(ref Counter); + Debug.WriteLine($"-------- {nameof(_44166MDP)} destructor, {Counter} allocated"); + } + } + + [Preserve(AllMembers = true)] + public class _44166Master : ContentPage + { + public static int Counter; + + public _44166Master() + { + Interlocked.Increment(ref Counter); + Debug.WriteLine($"++++++++ {nameof(_44166Master)} constructor, {Counter} allocated"); + + Title = "Master"; + var goButton = new Button { Text = "Back" }; + goButton.Clicked += (sender, args) => Application.Current.MainPage = new Bugzilla44166(); + + Content = new StackLayout + { + Children = { goButton } + }; + } + + ~_44166Master() + { + Interlocked.Decrement(ref Counter); + Debug.WriteLine($"-------- {nameof(_44166Master)} destructor, {Counter} allocated"); + } + } + + [Preserve(AllMembers = true)] + public class _44166Detail : NavigationPage + { + public static int Counter; + + public _44166Detail() + { + Interlocked.Increment(ref Counter); + Debug.WriteLine($"++++++++ {nameof(_44166Detail)} constructor, {Counter} allocated"); + + Title = "Detail"; + PushAsync(new _44166NavContent()); + } + + ~_44166Detail() + { + Interlocked.Decrement(ref Counter); + Debug.WriteLine($"-------- {nameof(_44166Detail)} destructor, {Counter} allocated"); + } + } + + [Preserve(AllMembers = true)] + public class _44166NavContent : ContentPage + { + public static int Counter; + + public _44166NavContent () + { + Interlocked.Increment(ref Counter); + Debug.WriteLine($"++++++++ {nameof(_44166NavContent)} constructor, {Counter} allocated"); + + var goButton = new Button { Text = "Back" }; + goButton.Clicked += (sender, args) => Application.Current.MainPage = new Bugzilla44166(); + + Content = new StackLayout + { + Children = { goButton } + }; + } + + ~_44166NavContent () + { + Interlocked.Decrement(ref Counter); + Debug.WriteLine($"-------- {nameof(_44166NavContent)} destructor, {Counter} allocated"); + } + } +} \ No newline at end of file diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems index eb9424b..51171be 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems @@ -125,6 +125,7 @@ + diff --git a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs index 102bac5..1b85b42 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs @@ -11,8 +11,9 @@ namespace Xamarin.Forms.Platform.Android.AppCompat PageContainer _pageContainer; FragmentManager _fragmentManager; readonly bool _isMaster; - readonly MasterDetailPage _parent; + MasterDetailPage _parent; Fragment _currentFragment; + bool _disposed; public MasterDetailContainer(MasterDetailPage parent, bool isMaster, Context context) : base(parent, isMaster, context) { @@ -106,6 +107,37 @@ namespace Xamarin.Forms.Platform.Android.AppCompat } } + protected override void Dispose(bool disposing) + { + if (_disposed) + { + return; + } + + _disposed = true; + + if (disposing) + { + if (_currentFragment != null) + { + FragmentTransaction transaction = FragmentManager.BeginTransaction(); + transaction.DisallowAddToBackStack(); + transaction.Remove(_currentFragment); + transaction.SetTransition((int)FragmentTransit.None); + transaction.CommitAllowingStateLoss(); + FragmentManager.ExecutePendingTransactions(); + + _currentFragment = null; + } + + _parent = null; + _pageContainer = null; + _fragmentManager = null; + } + + base.Dispose(disposing); + } + public void SetFragmentManager(FragmentManager fragmentManager) { if (_fragmentManager == null) diff --git a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailPageRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailPageRenderer.cs index 6740285..ccf01f2 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailPageRenderer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/MasterDetailPageRenderer.cs @@ -188,24 +188,29 @@ namespace Xamarin.Forms.Platform.Android.AppCompat if (_detailLayout != null) { + RemoveView(_detailLayout); _detailLayout.Dispose(); _detailLayout = null; } if (_masterLayout != null) { + RemoveView(_masterLayout); _masterLayout.Dispose(); _masterLayout = null; } Device.Info.PropertyChanged -= DeviceInfoPropertyChanged; + RemoveDrawerListener(this); + if (Element != null) { MasterDetailPageController.BackButtonPressed -= OnBackButtonPressed; Element.PropertyChanged -= HandlePropertyChanged; Element.Appearing -= MasterDetailPageAppearing; Element.Disappearing -= MasterDetailPageDisappearing; + Element.ClearValue(Android.Platform.RendererProperty); Element = null; } diff --git a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs index 97b2029..b77063e 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs @@ -173,6 +173,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat if (_drawerLayout != null && _drawerListener != null) { _drawerLayout.RemoveDrawerListener(_drawerListener); + _drawerListener = null; } _drawerToggle = null; -- 2.7.4