Unhook drawer listeners so MDP renderer and pages can be collected (#412)
authorE.Z. Hart <hartez@users.noreply.github.com>
Tue, 4 Oct 2016 17:47:02 +0000 (11:47 -0600)
committerRui Marinho <me@ruimarinho.net>
Tue, 4 Oct 2016 17:47:02 +0000 (18:47 +0100)
Null out page in custom MDP renderer in Control Gallery so it can be collected

Checkpoint

Checkpoint

Checkpoint

Checkpoint

Checkpoint

Checkpoint

Xamarin.Forms.ControlGallery.Android/CustomRenderers.cs
Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla44166.cs [new file with mode: 0644]
Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
Xamarin.Forms.Platform.Android/AppCompat/MasterDetailContainer.cs
Xamarin.Forms.Platform.Android/AppCompat/MasterDetailPageRenderer.cs
Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs

index a550212..0530fc3 100644 (file)
@@ -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 (file)
index 0000000..4006faa
--- /dev/null
@@ -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
index eb9424b..51171be 100644 (file)
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla42364.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla42519.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla43516.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Bugzilla44166.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)CarouselAsync.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla34561.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla34727.cs" />
index 102bac5..1b85b42 100644 (file)
@@ -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)
index 6740285..ccf01f2 100644 (file)
@@ -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;
                                }
index 97b2029..b77063e 100644 (file)
@@ -173,6 +173,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
                                if (_drawerLayout != null && _drawerListener != null)
                                {
                                        _drawerLayout.RemoveDrawerListener(_drawerListener);
+                                       _drawerListener = null;
                                }
 
                                _drawerToggle = null;