Fix out-of-memory crashes on iOS when creating maps (#467)

* Pool map views on iOS 10 to avoid memory issues
Clean up disposal implementation for map renderer, delegate

* Enable 39489 test on iOS

* Add missing XF Maps references

* Add missing maps reference to Windows 8.1 project

* Actually enable the test for iOS

* Fix placement of Isolate override

* Don't create a new control for ImageRenderer if NewElement is null

* Reverting fixes to ViewRenderer Dispose; have to fix that in a separate PR

8 files changed, 152 insertions, 11 deletions

diff --git a/Xamarin.Forms.ControlGallery.Windows/Xamarin.Forms.ControlGallery.Windows.csproj b/Xamarin.Forms.ControlGallery.Windows/Xamarin.Forms.ControlGallery.Windows.csproj
index 4f67107d..69901f1b 100644
--- a/Xamarin.Forms.ControlGallery.Windows/Xamarin.Forms.ControlGallery.Windows.csproj
+++ b/Xamarin.Forms.ControlGallery.Windows/Xamarin.Forms.ControlGallery.Windows.csproj
@@ -127,6 +127,10 @@
       <Project>{e5c4698d-fb57-4eec-98c0-89e620f6920a}</Project>
       <Name>Xamarin.Forms.Maps.WinRT.Tablet</Name>
     </ProjectReference>
+    <ProjectReference Include="..\Xamarin.Forms.Maps\Xamarin.Forms.Maps.csproj">
+      <Project>{7d13bac2-c6a4-416a-b07e-c169b199e52b}</Project>
+      <Name>Xamarin.Forms.Maps</Name>
+    </ProjectReference>
     <ProjectReference Include="..\Xamarin.Forms.Platform.WinRT.Tablet\Xamarin.Forms.Platform.WinRT.Tablet.csproj">
       <Project>{d3f9265a-30ac-43e8-a3eb-59bb76d2d0bf}</Project>
       <Name>Xamarin.Forms.Platform.WinRT.Tablet</Name>
diff --git a/Xamarin.Forms.ControlGallery.WindowsPhone/Xamarin.Forms.ControlGallery.WindowsPhone.csproj b/Xamarin.Forms.ControlGallery.WindowsPhone/Xamarin.Forms.ControlGallery.WindowsPhone.csproj
index 349b0b4c..478b1f4e 100644
--- a/Xamarin.Forms.ControlGallery.WindowsPhone/Xamarin.Forms.ControlGallery.WindowsPhone.csproj
+++ b/Xamarin.Forms.ControlGallery.WindowsPhone/Xamarin.Forms.ControlGallery.WindowsPhone.csproj
@@ -100,6 +100,10 @@
       <Project>{2633af57-f2cb-442a-ac19-f97bd8a06571}</Project>
       <Name>Xamarin.Forms.Maps.WinRT.Phone</Name>
     </ProjectReference>
+    <ProjectReference Include="..\Xamarin.Forms.Maps\Xamarin.Forms.Maps.csproj">
+      <Project>{7d13bac2-c6a4-416a-b07e-c169b199e52b}</Project>
+      <Name>Xamarin.Forms.Maps</Name>
+    </ProjectReference>
     <ProjectReference Include="..\Xamarin.Forms.Platform.WinRT.Phone\Xamarin.Forms.Platform.WinRT.Phone.csproj">
       <Project>{3361d52c-2e74-433e-8285-9c9a5c485977}</Project>
       <Name>Xamarin.Forms.Platform.WinRT.Phone</Name>
diff --git a/Xamarin.Forms.ControlGallery.WindowsUniversal/Xamarin.Forms.ControlGallery.WindowsUniversal.csproj b/Xamarin.Forms.ControlGallery.WindowsUniversal/Xamarin.Forms.ControlGallery.WindowsUniversal.csproj
index 7f0162f1..9907d361 100644
--- a/Xamarin.Forms.ControlGallery.WindowsUniversal/Xamarin.Forms.ControlGallery.WindowsUniversal.csproj
+++ b/Xamarin.Forms.ControlGallery.WindowsUniversal/Xamarin.Forms.ControlGallery.WindowsUniversal.csproj
@@ -110,6 +110,10 @@
       <Project>{04d89a60-78ef-4a32-ae17-87e47e0233a5}</Project>
       <Name>Xamarin.Forms.Maps.UWP</Name>
     </ProjectReference>
+    <ProjectReference Include="..\Xamarin.Forms.Maps\Xamarin.Forms.Maps.csproj">
+      <Project>{7d13bac2-c6a4-416a-b07e-c169b199e52b}</Project>
+      <Name>Xamarin.Forms.Maps</Name>
+    </ProjectReference>
     <ProjectReference Include="..\Xamarin.Forms.Platform.UAP\Xamarin.Forms.Platform.UAP.csproj">
       <Project>{00d8d049-ffaa-4759-8fc9-1eca30777f72}</Project>
       <Name>Xamarin.Forms.Platform.UAP</Name>
diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla39489.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla39489.cs
index 3fe44681..0e04fcd5 100644
--- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla39489.cs
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla39489.cs
@@ -2,6 +2,7 @@ using Xamarin.Forms.CustomAttributes;
 using Xamarin.Forms.Internals;
 using System.Threading;
 using System;
+using System.Diagnostics;
 using System.Threading.Tasks;
 using Xamarin.Forms.Maps;
 
@@ -13,7 +14,7 @@ using NUnit.Framework;
 namespace Xamarin.Forms.Controls.Issues
 {
 	[Preserve(AllMembers = true)]
-	[Issue(IssueTracker.Bugzilla, 39489, "Memory leak when using NavigationPage with Maps", PlatformAffected.Android)]
+	[Issue(IssueTracker.Bugzilla, 39489, "Memory leak when using NavigationPage with Maps", PlatformAffected.Android | PlatformAffected.iOS)]
 	public class Bugzilla39489 : TestNavigationPage
 	{
 		protected override void Init()
@@ -22,7 +23,6 @@ namespace Xamarin.Forms.Controls.Issues
 		}
 
 #if UITEST
-#if !__IOS__ // Temporarily disabling this test on iOS
 
 		protected override bool Isolate => true;
 
@@ -44,19 +44,40 @@ namespace Xamarin.Forms.Controls.Issues
 			}
 		}
 #endif
-#endif
+	}
+
+	public class Bz39489Map : Map
+	{
+		static int s_count;
+
+		public Bz39489Map()
+		{
+			Interlocked.Increment(ref s_count);
+			Debug.WriteLine($"++++++++ Bz39489Map : Constructor, count is {s_count}");
+		}
+
+		~Bz39489Map()
+		{
+			Interlocked.Decrement(ref s_count);
+			Debug.WriteLine($"-------- Bz39489Map: Destructor, count is {s_count}");
+		}
 	}
 
 	[Preserve(AllMembers = true)]
 	public class Bz39489Content : ContentPage
 	{
+		static int s_count;
+
 		public Bz39489Content()
 		{
+			Interlocked.Increment(ref s_count);
+			Debug.WriteLine($">>>>> Bz39489Content Bz39489Content 54: Constructor, count is {s_count}");
+
 			var button = new Button { Text = "New Page" };
 
 			var gcbutton = new Button { Text = "GC" };
 
-			var map = new Map();
+			var map = new Bz39489Map();
 
 			button.Clicked += Button_Clicked;
 			gcbutton.Clicked += GCbutton_Clicked;
@@ -76,5 +97,11 @@ namespace Xamarin.Forms.Controls.Issues
 		{
 			Navigation.PushAsync(new Bz39489Content());
 		}
+
+		~Bz39489Content()
+		{
+			Interlocked.Decrement(ref s_count);
+			Debug.WriteLine($">>>>> Bz39489Content ~Bz39489Content 82: Destructor, count is {s_count}");
+		}
 	}
 }
\ No newline at end of file
diff --git a/Xamarin.Forms.Maps.iOS/FormsMaps.cs b/Xamarin.Forms.Maps.iOS/FormsMaps.cs
index ca8dca67..19c0701e 100644
--- a/Xamarin.Forms.Maps.iOS/FormsMaps.cs
+++ b/Xamarin.Forms.Maps.iOS/FormsMaps.cs
@@ -7,6 +7,7 @@ namespace Xamarin
 	{
 		static bool s_isInitialized;
 		static bool? s_isiOs8OrNewer;
+		static bool? s_isiOs10OrNewer;
 
 		internal static bool IsiOs8OrNewer
 		{
@@ -18,6 +19,16 @@ namespace Xamarin
 			}
 		}
 
+		internal static bool IsiOs10OrNewer
+		{
+			get
+			{
+				if (!s_isiOs10OrNewer.HasValue)
+					s_isiOs10OrNewer = UIDevice.CurrentDevice.CheckSystemVersion(10, 0);
+				return s_isiOs10OrNewer.Value;
+			}
+		}
+
 		public static void Init()
 		{
 			if (s_isInitialized)
diff --git a/Xamarin.Forms.Maps.iOS/MapPool.cs b/Xamarin.Forms.Maps.iOS/MapPool.cs
new file mode 100644
index 00000000..dc8a9c47
--- /dev/null
+++ b/Xamarin.Forms.Maps.iOS/MapPool.cs
@@ -0,0 +1,25 @@
+using System.Collections.Concurrent;
+using MapKit;
+
+namespace Xamarin.Forms.Maps.iOS
+{
+	// A static pool of MKMapView instances we can reuse 
+	internal class MapPool
+	{
+		static MapPool s_instance;
+		public static MapPool Instance => s_instance ?? (s_instance = new MapPool());
+
+		internal readonly ConcurrentQueue<MKMapView> Maps = new ConcurrentQueue<MKMapView>();
+
+		public static void Add(MKMapView mapView)
+		{
+			Instance.Maps.Enqueue(mapView);
+		}
+
+		public static MKMapView Get()
+		{
+			MKMapView mapView;
+			return Instance.Maps.TryDequeue(out mapView) ? mapView : null;
+		}
+	}
+}
\ No newline at end of file
diff --git a/Xamarin.Forms.Maps.iOS/MapRenderer.cs b/Xamarin.Forms.Maps.iOS/MapRenderer.cs
index af1f0808..cc9f2533 100644
--- a/Xamarin.Forms.Maps.iOS/MapRenderer.cs
+++ b/Xamarin.Forms.Maps.iOS/MapRenderer.cs
@@ -17,9 +17,10 @@ namespace Xamarin.Forms.Maps.iOS
 	internal class MapDelegate : MKMapViewDelegate
 	{
 		// keep references alive, removing this will cause a segfault
-		static readonly List<object> List = new List<object>();
-		readonly Map _map;
+		readonly List<object> List = new List<object>();
+		Map _map;
 		UIView _lastTouchedView;
+		bool _disposed;
 
 		internal MapDelegate(Map map)
 		{
@@ -103,31 +104,80 @@ namespace Xamarin.Forms.Maps.iOS
 
 			targetPin.SendTap();
 		}
+
+		protected override void Dispose(bool disposing)
+		{
+			if (_disposed)
+			{
+				return;
+			}
+
+			_disposed = true;
+
+			if (disposing)
+			{
+				_map = null;
+				_lastTouchedView = null;
+			}
+
+			base.Dispose(disposing);
+		}
 	}
 
 	public class MapRenderer : ViewRenderer
 	{
 	    CLLocationManager _locationManager;
 		bool _shouldUpdateRegion;
+		bool _disposed; 
+
+		const string MoveMessageName = "MapMoveToRegion";
 
 		public override SizeRequest GetDesiredSize(double widthConstraint, double heightConstraint)
 		{
 			return Control.GetSizeRequest(widthConstraint, heightConstraint);
 		}
 
+		// iOS 10 has some issues with releasing memory from map views; each one we create allocates
+		// a bunch of memory we can never get back. Until that's fixed, we'll just reuse MKMapViews
+		// as much as possible to prevent creating new ones and losing more memory
+
+		// For the time being, we don't want ViewRenderer handling disposal of the MKMapView
+		// if we're on iOS 10; during Dispose we'll be putting the MKMapView in a pool instead
+		protected override bool ManageNativeControlLifetime => !FormsMaps.IsiOs10OrNewer;
+
 		protected override void Dispose(bool disposing)
 		{
+			if (_disposed)
+			{
+				return;
+			}
+
+			_disposed = true;
+
 			if (disposing)
 			{
 				if (Element != null)
 				{
 					var mapModel = (Map)Element;
-					MessagingCenter.Unsubscribe<Map, MapSpan>(this, "MapMoveToRegion");
+					MessagingCenter.Unsubscribe<Map, MapSpan>(this, MoveMessageName);
 					((ObservableCollection<Pin>)mapModel.Pins).CollectionChanged -= OnCollectionChanged;
 				}
 
 				var mkMapView = (MKMapView)Control;
 				mkMapView.RegionChanged -= MkMapViewOnRegionChanged;
+				mkMapView.GetViewForAnnotation = null;
+				mkMapView.Delegate.Dispose();
+				mkMapView.Delegate = null;
+				mkMapView.RemoveFromSuperview();
+
+				if (FormsMaps.IsiOs10OrNewer)
+				{
+					// This renderer is done with the MKMapView; we can put it in the pool
+					// for other rendererers to use in the future
+					MapPool.Add(mkMapView);
+				}
+
+				// For iOS versions < 10, the MKMapView will be disposed in ViewRenderer's Dispose method
 
 				if (_locationManager != null)
 				{
@@ -146,7 +196,7 @@ namespace Xamarin.Forms.Maps.iOS
 			if (e.OldElement != null)
 			{
 				var mapModel = (Map)e.OldElement;
-				MessagingCenter.Unsubscribe<Map, MapSpan>(this, "MapMoveToRegion");
+				MessagingCenter.Unsubscribe<Map, MapSpan>(this, MoveMessageName);
 				((ObservableCollection<Pin>)mapModel.Pins).CollectionChanged -= OnCollectionChanged;
 			}
 
@@ -156,14 +206,30 @@ namespace Xamarin.Forms.Maps.iOS
 
 				if (Control == null)
 				{
-					SetNativeControl(new MKMapView(RectangleF.Empty));
+					MKMapView mapView = null;
+
+					if (FormsMaps.IsiOs10OrNewer)
+					{
+						// See if we've got an MKMapView available in the pool; if so, use it
+						mapView = MapPool.Get();
+					}
+
+					if (mapView == null)
+					{
+						// If this is iOS 9 or lower, or if there weren't any MKMapViews in the pool,
+						// create a new one
+						mapView = new MKMapView(RectangleF.Empty);
+					}
+
+					SetNativeControl(mapView);
+
 					var mkMapView = (MKMapView)Control;
 					var mapDelegate = new MapDelegate(mapModel);
 					mkMapView.GetViewForAnnotation = mapDelegate.GetViewForAnnotation;
 					mkMapView.RegionChanged += MkMapViewOnRegionChanged;
 				}
 
-				MessagingCenter.Subscribe<Map, MapSpan>(this, "MapMoveToRegion", (s, a) => MoveToRegion(a), mapModel);
+				MessagingCenter.Subscribe<Map, MapSpan>(this, MoveMessageName, (s, a) => MoveToRegion(a), mapModel);
 				if (mapModel.LastMoveToRegion != null)
 					MoveToRegion(mapModel.LastMoveToRegion, false);
 
@@ -202,7 +268,6 @@ namespace Xamarin.Forms.Maps.iOS
 				MoveToRegion(((Map)Element).LastMoveToRegion, false);
 				_shouldUpdateRegion = false;
 			}
-
 		}
 
 		void AddPins(IList pins)
diff --git a/Xamarin.Forms.Maps.iOS/Xamarin.Forms.Maps.iOS.csproj b/Xamarin.Forms.Maps.iOS/Xamarin.Forms.Maps.iOS.csproj
index 4ce638ea..bcfc498e 100644
--- a/Xamarin.Forms.Maps.iOS/Xamarin.Forms.Maps.iOS.csproj
+++ b/Xamarin.Forms.Maps.iOS/Xamarin.Forms.Maps.iOS.csproj
@@ -61,6 +61,7 @@
     <Compile Include="..\Xamarin.Forms.Core\Properties\GlobalAssemblyInfo.cs">
       <Link>Properties\GlobalAssemblyInfo.cs</Link>
     </Compile>
+    <Compile Include="MapPool.cs" />
     <Compile Include="MapRenderer.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="FormsMaps.cs" />