Fix memory leak caused by BaseCellView and RendererHolder (#311)

4 files changed, 123 insertions, 7 deletions

diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla35132.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla35132.cs
new file mode 100644
index 00000000..917c608c
--- /dev/null
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla35132.cs
@@ -0,0 +1,90 @@
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+using System.Threading;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+#if UITEST
+using Xamarin.UITest;
+using NUnit.Framework;
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Bugzilla, 35132, "Pages are not collected when using a Navigationpage (FormsApplicationActivity)")]
+	public class Bugzilla35132 : TestNavigationPage
+	{
+		protected override void Init()
+		{
+			PushAsync(new RootPage());
+		}
+
+		[Preserve(AllMembers = true)]
+		public class BugPage : ContentPage
+		{
+			public static int Livecount;
+
+			public BugPage(IEnumerable<string> items)
+			{
+				Interlocked.Increment(ref Livecount);
+
+				Content = new StackLayout
+				{
+					Children =
+					{
+						new Label { Text = items.Count() < 3 ? "Running" : Livecount < 3 ? "Success" : "Failure" },
+						new ListView { ItemsSource = items }
+					}
+				};
+			}
+
+			~BugPage()
+			{
+				Debug.WriteLine(">>>>>>>> BugPage Finalized");
+				Interlocked.Decrement(ref Livecount);
+			}
+		}
+
+		[Preserve(AllMembers = true)]
+		public class RootPage : ContentPage
+		{
+			readonly List<string> _items = new List<string>();
+
+			public RootPage()
+			{
+				var button = new Button { Text = "Open" };
+				button.Clicked += Button_Clicked;
+				Content = button;
+			}
+
+			async void Button_Clicked(object sender, EventArgs e)
+			{
+				Debug.WriteLine(">>>>>>>> Invoking Garbage Collector");
+				GC.Collect();
+				GC.WaitForPendingFinalizers();
+
+				_items.Add((BugPage.Livecount).ToString());
+				await Navigation.PushAsync(new BugPage(_items));
+			}
+		}
+
+#if UITEST 
+		[Test]
+		public void Issue1Test ()
+		{
+			RunningApp.WaitForElement (q => q.Marked ("Open"));
+			RunningApp.Tap(q => q.Marked ("Open"));
+			RunningApp.Back();
+			RunningApp.WaitForElement (q => q.Marked ("Open"));
+			RunningApp.Tap(q => q.Marked ("Open"));
+			RunningApp.Back();
+			RunningApp.WaitForElement (q => q.Marked ("Open"));
+			RunningApp.Tap(q => q.Marked ("Open"));
+			RunningApp.WaitForElement (q => q.Marked ("Success"));
+		}
+#endif
+	}
+}
\ 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 88cfb0bd..7a46c880 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
@@ -62,6 +62,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla34007.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla35078.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla35127.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Bugzilla35132.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla35157.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla35294.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla35472.cs" />
diff --git a/Xamarin.Forms.Platform.Android/Cells/BaseCellView.cs b/Xamarin.Forms.Platform.Android/Cells/BaseCellView.cs
index 74c56dbd..c4d10cc4 100644
--- a/Xamarin.Forms.Platform.Android/Cells/BaseCellView.cs
+++ b/Xamarin.Forms.Platform.Android/Cells/BaseCellView.cs
@@ -18,7 +18,7 @@ namespace Xamarin.Forms.Platform.Android
 		public const double DefaultMinHeight = 44;
 
 		readonly Color _androidDefaultTextColor;
-		readonly Cell _cell;
+		Cell _cell;
 		readonly TextView _detailText;
 		readonly ImageView _imageView;
 		readonly TextView _mainText;
@@ -179,6 +179,12 @@ namespace Xamarin.Forms.Platform.Android
 			LayoutParameters = new LayoutParams(ViewGroup.LayoutParams.MatchParent, (int)(height == -1 ? ViewGroup.LayoutParams.WrapContent : height));
 		}
 
+		protected override void OnDetachedFromWindow()
+		{
+			base.OnDetachedFromWindow();
+			_cell = null;
+		}
+
 		async void UpdateBitmap(ImageSource source, ImageSource previousSource = null)
 		{
 			if (Equals(source, previousSource))
diff --git a/Xamarin.Forms.Platform.Android/Cells/CellRenderer.cs b/Xamarin.Forms.Platform.Android/Cells/CellRenderer.cs
index e62a8d4e..fd7cd89e 100644
--- a/Xamarin.Forms.Platform.Android/Cells/CellRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/Cells/CellRenderer.cs
@@ -24,7 +24,7 @@ namespace Xamarin.Forms.Platform.Android
 		public AView GetCell(Cell item, AView convertView, ViewGroup parent, Context context)
 		{
 			Performance.Start();
-
+			
 			Cell = item;
 			Cell.PropertyChanged -= PropertyChangedHandler;
 
@@ -33,14 +33,18 @@ namespace Xamarin.Forms.Platform.Android
 			if (convertView != null)
 			{
 				Object tag = convertView.Tag;
-				var renderHolder = tag as RendererHolder;
-				if (renderHolder != null)
+				CellRenderer renderer = (tag as RendererHolder)?.Renderer;
+				
+				Cell oldCell = renderer?.Cell;
+
+				if (oldCell != null)
 				{
-					Cell oldCell = renderHolder.Renderer.Cell;
 					((ICellController)oldCell).SendDisappearing();
 
 					if (Cell != oldCell)
+					{
 						SetRenderer(oldCell, null);
+					}
 				}
 			}
 
@@ -50,7 +54,7 @@ namespace Xamarin.Forms.Platform.Android
 
 			var holder = view.Tag as RendererHolder;
 			if (holder == null)
-				view.Tag = new RendererHolder { Renderer = this };
+				view.Tag = new RendererHolder(this);
 			else
 				holder.Renderer = this;
 
@@ -126,7 +130,22 @@ namespace Xamarin.Forms.Platform.Android
 
 		class RendererHolder : Object
 		{
-			public CellRenderer Renderer;
+			readonly WeakReference<CellRenderer> _rendererRef;
+
+			public RendererHolder(CellRenderer renderer)
+			{
+				_rendererRef = new WeakReference<CellRenderer>(renderer);
+			}
+
+			public CellRenderer Renderer
+			{
+				get
+				{
+					CellRenderer renderer;
+					return _rendererRef.TryGetTarget(out renderer) ? renderer : null;
+				}
+				set { _rendererRef.SetTarget(value); }
+			}
 		}
 	}
 }
\ No newline at end of file