From ba7b66b83a83328f56e0676efcff5d3c553ac6bf Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Thu, 12 Jan 2017 14:43:57 -0700 Subject: 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 --- .../Xamarin.Forms.ControlGallery.Windows.csproj | 4 ++ ...amarin.Forms.ControlGallery.WindowsPhone.csproj | 4 ++ ...in.Forms.ControlGallery.WindowsUniversal.csproj | 4 ++ .../Bugzilla39489.cs | 35 ++++++++-- Xamarin.Forms.Maps.iOS/FormsMaps.cs | 11 +++ Xamarin.Forms.Maps.iOS/MapPool.cs | 25 +++++++ Xamarin.Forms.Maps.iOS/MapRenderer.cs | 79 ++++++++++++++++++++-- .../Xamarin.Forms.Maps.iOS.csproj | 1 + 8 files changed, 152 insertions(+), 11 deletions(-) create mode 100644 Xamarin.Forms.Maps.iOS/MapPool.cs diff --git a/Xamarin.Forms.ControlGallery.Windows/Xamarin.Forms.ControlGallery.Windows.csproj b/Xamarin.Forms.ControlGallery.Windows/Xamarin.Forms.ControlGallery.Windows.csproj index 4f67107..69901f1 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 @@ {e5c4698d-fb57-4eec-98c0-89e620f6920a} Xamarin.Forms.Maps.WinRT.Tablet + + {7d13bac2-c6a4-416a-b07e-c169b199e52b} + Xamarin.Forms.Maps + {d3f9265a-30ac-43e8-a3eb-59bb76d2d0bf} Xamarin.Forms.Platform.WinRT.Tablet diff --git a/Xamarin.Forms.ControlGallery.WindowsPhone/Xamarin.Forms.ControlGallery.WindowsPhone.csproj b/Xamarin.Forms.ControlGallery.WindowsPhone/Xamarin.Forms.ControlGallery.WindowsPhone.csproj index 349b0b4..478b1f4 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 @@ {2633af57-f2cb-442a-ac19-f97bd8a06571} Xamarin.Forms.Maps.WinRT.Phone + + {7d13bac2-c6a4-416a-b07e-c169b199e52b} + Xamarin.Forms.Maps + {3361d52c-2e74-433e-8285-9c9a5c485977} Xamarin.Forms.Platform.WinRT.Phone diff --git a/Xamarin.Forms.ControlGallery.WindowsUniversal/Xamarin.Forms.ControlGallery.WindowsUniversal.csproj b/Xamarin.Forms.ControlGallery.WindowsUniversal/Xamarin.Forms.ControlGallery.WindowsUniversal.csproj index 7f0162f..9907d36 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 @@ {04d89a60-78ef-4a32-ae17-87e47e0233a5} Xamarin.Forms.Maps.UWP + + {7d13bac2-c6a4-416a-b07e-c169b199e52b} + Xamarin.Forms.Maps + {00d8d049-ffaa-4759-8fc9-1eca30777f72} Xamarin.Forms.Platform.UAP 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 3fe4468..0e04fcd 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 ca8dca6..19c0701 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 0000000..dc8a9c4 --- /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 Maps = new ConcurrentQueue(); + + 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 af1f080..cc9f253 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 List = new List(); - readonly Map _map; + readonly List List = new List(); + 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(this, "MapMoveToRegion"); + MessagingCenter.Unsubscribe(this, MoveMessageName); ((ObservableCollection)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(this, "MapMoveToRegion"); + MessagingCenter.Unsubscribe(this, MoveMessageName); ((ObservableCollection)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(this, "MapMoveToRegion", (s, a) => MoveToRegion(a), mapModel); + MessagingCenter.Subscribe(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 4ce638e..bcfc498 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 @@ Properties\GlobalAssemblyInfo.cs + -- cgit v0.11.2