diff options
8 files changed, 140 insertions, 34 deletions
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 new file mode 100644 index 00000000..266b0fa6 --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla39489.cs @@ -0,0 +1,75 @@ +using Xamarin.Forms.CustomAttributes; +using Xamarin.Forms.Internals; +using System.Threading; +using System; +using System.Threading.Tasks; +using Xamarin.Forms.Maps; + +#if UITEST +using Xamarin.UITest; +using NUnit.Framework; +#endif + +namespace Xamarin.Forms.Controls +{ + [Preserve(AllMembers = true)] + [Issue(IssueTracker.Bugzilla, 39489, "Memory leak when using NavigationPage with Maps", PlatformAffected.Android)] + public class Bugzilla39489 : TestNavigationPage + { + protected override void Init() + { + PushAsync(new Bz39489Content()); + } + + #if UITEST + [Test] + public async void Bugzilla39458Test() + { + // Original bug report (https://bugzilla.xamarin.com/show_bug.cgi?id=39489) had a crash (OOM) after 25-30 + // page loads. Obviously it's going to depend heavily on the device and amount of available memory, but + // if this starts failing before 50 we'll know we've sprung another serious leak + int iterations = 50; + + for (int n = 0; n < iterations; n++) + { + RunningApp.WaitForElement(q => q.Marked("New Page")); + RunningApp.Tap(q => q.Marked("New Page")); + RunningApp.WaitForElement(q => q.Marked("New Page")); + await Task.Delay(1000); + RunningApp.Back(); + } + } + #endif + } + + [Preserve(AllMembers = true)] + public class Bz39489Content : ContentPage + { + public Bz39489Content() + { + var button = new Button { Text = "New Page" }; + + var gcbutton = new Button { Text = "GC" }; + + var map = new Map(); + + button.Clicked += Button_Clicked; + gcbutton.Clicked += GCbutton_Clicked; + + Content = new StackLayout { Children = { button, gcbutton, map } }; + } + + void GCbutton_Clicked(object sender, EventArgs e) + { + System.Diagnostics.Debug.WriteLine(">>>>>>>> Running Garbage Collection"); + GC.Collect(); + GC.WaitForPendingFinalizers(); + System.Diagnostics.Debug.WriteLine($">>>>>>>> GC.GetTotalMemory = {GC.GetTotalMemory(true):n0}"); + } + + void Button_Clicked(object sender, EventArgs e) + { + Navigation.PushAsync(new Bz39489Content()); + } + } +}
\ 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 364b3c44..7a47c10f 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 @@ -437,6 +437,7 @@ <Compile Include="$(MSBuildThisFileDirectory)Issue55555.cs" /> <Compile Include="$(MSBuildThisFileDirectory)Bugzilla41029.cs" /> <Compile Include="$(MSBuildThisFileDirectory)Bugzilla39908.cs" /> + <Compile Include="$(MSBuildThisFileDirectory)Bugzilla39489.cs" /> </ItemGroup> <ItemGroup> <EmbeddedResource Include="$(MSBuildThisFileDirectory)Bugzilla22229.xaml"> diff --git a/Xamarin.Forms.Maps.Android/MapRenderer.cs b/Xamarin.Forms.Maps.Android/MapRenderer.cs index dc1b46cc..f83b6fad 100644 --- a/Xamarin.Forms.Maps.Android/MapRenderer.cs +++ b/Xamarin.Forms.Maps.Android/MapRenderer.cs @@ -59,25 +59,30 @@ namespace Xamarin.Forms.Maps.Android protected override void Dispose(bool disposing) { - if (disposing && !_disposed) + if (_disposed) { - _disposed = true; + return; + } - Map mapModel = Element; - if (mapModel != null) + _disposed = true; + + if (disposing) + { + if (Element != null) { MessagingCenter.Unsubscribe<Map, MapSpan>(this, MoveMessageName); - ((ObservableCollection<Pin>)mapModel.Pins).CollectionChanged -= OnCollectionChanged; + ((ObservableCollection<Pin>)Element.Pins).CollectionChanged -= OnCollectionChanged; } - GoogleMap gmap = NativeMap; - if (gmap == null) - { - return; - } - gmap.MyLocationEnabled = false; - gmap.InfoWindowClick -= MapOnMarkerClick; - gmap.Dispose(); + if (NativeMap != null) + { + NativeMap.MyLocationEnabled = false; + NativeMap.SetOnCameraChangeListener(null); + NativeMap.InfoWindowClick -= MapOnMarkerClick; + NativeMap.Dispose(); + } + + Control?.OnDestroy(); } base.Dispose(disposing); @@ -265,6 +270,7 @@ namespace Xamarin.Forms.Maps.Android catch (IllegalStateException exc) { System.Diagnostics.Debug.WriteLine("MoveToRegion exception: " + exc); + Log.Warning("Xamarin.Forms MapRenderer", $"MoveToRegion exception: {exc}"); } } diff --git a/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs index 2d175860..4c33df85 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/ButtonRenderer.cs @@ -71,6 +71,13 @@ namespace Xamarin.Forms.Platform.Android.AppCompat if (disposing) { + if (Control != null) + { + Control.SetOnClickListener(null); + Control.RemoveOnAttachStateChangeListener(this); + Control.Tag = null; + _textColorSwitcher = null; + } } base.Dispose(disposing); diff --git a/Xamarin.Forms.Platform.Android/AppCompat/FragmentContainer.cs b/Xamarin.Forms.Platform.Android/AppCompat/FragmentContainer.cs index 7c7b5250..62e43809 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/FragmentContainer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/FragmentContainer.cs @@ -1,9 +1,9 @@ using System; using Android.OS; using Android.Runtime; -using Android.Support.V4.App; using Android.Views; using AView = Android.Views.View; +using Fragment = Android.Support.V4.App.Fragment; namespace Xamarin.Forms.Platform.Android.AppCompat { @@ -75,7 +75,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat return null; } - + public override void OnDestroyView() { if (Page != null) @@ -90,18 +90,16 @@ namespace Xamarin.Forms.Platform.Android.AppCompat _visualElementRenderer.Dispose(); } - if (_pageContainer != null && _pageContainer.Handle != IntPtr.Zero) - { - _pageContainer.RemoveFromParent(); - _pageContainer.Dispose(); - } + // We do *not* eagerly dispose of the _pageContainer here; doing so causes a memory leak + // if animated fragment transitions are enabled (it removes some info that the animation's + // onAnimationEnd handler requires to properly clean things up) + // Instead, we let the garbage collector pick it up later, when we can be sure it's safe Page?.ClearValue(Android.Platform.RendererProperty); } _onCreateCallback = null; _visualElementRenderer = null; - _pageContainer = null; base.OnDestroyView(); } diff --git a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs index b2b9108a..61d0b322 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/NavigationPageRenderer.cs @@ -41,6 +41,9 @@ namespace Xamarin.Forms.Platform.Android.AppCompat ToolbarTracker _toolbarTracker; bool _toolbarVisible; + // The following is based on https://android.googlesource.com/platform/frameworks/support/+/refs/heads/master/v4/java/android/support/v4/app/FragmentManager.java#849 + const int TransitionDuration = 220; + public NavigationPageRenderer() { AutoPackage = false; @@ -71,7 +74,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat } } - FragmentManager FragmentManager => _fragmentManager ?? (_fragmentManager = ((FormsAppCompatActivity)Context).SupportFragmentManager); + FragmentManager FragmentManager => _fragmentManager ?? (_fragmentManager = ((FormsAppCompatActivity)Context).SupportFragmentManager); IPageController PageController => Element as IPageController; @@ -132,14 +135,14 @@ namespace Xamarin.Forms.Platform.Android.AppCompat if (Element != null) { - foreach(Element element in PageController.InternalChildren) + foreach (Element element in PageController.InternalChildren) { var child = element as VisualElement; if (child == null) { continue; } - + IVisualElementRenderer renderer = Android.Platform.GetRenderer(child); renderer?.Dispose(); } @@ -320,7 +323,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat else transaction.SetTransition((int)FragmentTransit.FragmentClose); } - + internal int GetNavBarHeight() { if (!ToolbarVisible) @@ -501,7 +504,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat ToolbarNavigationClickListener = new ClickListener(Element) }; - if (_drawerListener != null) + if (_drawerListener != null) { _drawerLayout.RemoveDrawerListener(_drawerListener); } @@ -569,6 +572,12 @@ namespace Xamarin.Forms.Platform.Android.AppCompat var tcs = new TaskCompletionSource<bool>(); Fragment fragment = FragmentContainer.CreateInstance(view); FragmentManager fm = FragmentManager; + +#if DEBUG + // Enables logging of moveToState operations to logcat + FragmentManager.EnableDebugLogging(true); +#endif + List<Fragment> fragments = _fragmentStack; Current = view; @@ -599,7 +608,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat transaction.Remove(currentToRemove); popPage = popToRoot; } - + Fragment toShow = fragments.Last(); // Execute pending transactions so that we can be sure the fragment list is accurate. fm.ExecutePendingTransactions(); @@ -621,7 +630,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat // The fragment transitions don't really SUPPORT telling you when they end // There are some hacks you can do, but they actually are worse than just doing this: - + if (animated) { if (!removed) @@ -633,12 +642,15 @@ namespace Xamarin.Forms.Platform.Android.AppCompat else if (_drawerToggle != null && ((INavigationPageController)Element).StackDepth == 2) AnimateArrowOut(); - Device.StartTimer(TimeSpan.FromMilliseconds(200), () => + Device.StartTimer(TimeSpan.FromMilliseconds(TransitionDuration), () => { tcs.TrySetResult(true); - fragment.UserVisibleHint = true; + fragment.UserVisibleHint = !removed; if (removed) + { UpdateToolbar(); + } + return false; }); } @@ -647,8 +659,9 @@ namespace Xamarin.Forms.Platform.Android.AppCompat Device.StartTimer(TimeSpan.FromMilliseconds(1), () => { tcs.TrySetResult(true); - fragment.UserVisibleHint = true; + fragment.UserVisibleHint = !removed; UpdateToolbar(); + return false; }); } @@ -656,7 +669,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat Context.HideKeyboard(this); ((Platform)Element.Platform).NavAnimationInProgress = false; - // 200ms is how long the animations are, and they are "reversible" in the sense that starting another one slightly before it's done is fine + // TransitionDuration is how long the built-in animations are, and they are "reversible" in the sense that starting another one slightly before it's done is fine return tcs.Task; } @@ -821,4 +834,4 @@ namespace Xamarin.Forms.Platform.Android.AppCompat } } } -}
\ No newline at end of file +} diff --git a/Xamarin.Forms.Platform.Android/ViewRenderer.cs b/Xamarin.Forms.Platform.Android/ViewRenderer.cs index f285d6d9..057fbcee 100644 --- a/Xamarin.Forms.Platform.Android/ViewRenderer.cs +++ b/Xamarin.Forms.Platform.Android/ViewRenderer.cs @@ -76,7 +76,8 @@ namespace Xamarin.Forms.Platform.Android { if (Control != null && ManageNativeControlLifetime) { - Control.RemoveFromParent(); + Control.OnFocusChangeListener = null; + RemoveView(Control); Control.Dispose(); Control = null; } diff --git a/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs b/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs index 404dd23e..745d2cdd 100644 --- a/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs +++ b/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs @@ -229,6 +229,9 @@ namespace Xamarin.Forms.Platform.Android if (disposing) { + SetOnClickListener(null); + SetOnTouchListener(null); + if (Tracker != null) { Tracker.Dispose(); @@ -273,6 +276,8 @@ namespace Xamarin.Forms.Platform.Android if (Platform.GetRenderer(Element) == this) Platform.SetRenderer(Element, null); + (Element as IElementController).EffectControlProvider = null; + Element = null; } } |