Bug 31171 - NavigationPage.PopAsync() cause memory leak by still holding on popped Page
Summary: NavigationPage.PopAsync() cause memory leak by still holding on popped Page
Status: CONFIRMED
Alias: None
Product: Forms
Classification: Xamarin
Component: Forms (show other bugs)
Version: 1.4.3
Hardware: PC Windows
: High normal
Target Milestone: ---
Assignee: Bugzilla
URL:
: 30522 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-06-16 17:16 UTC by Sergey Komisarchik
Modified: 2017-11-18 17:24 UTC (History)
10 users (show)

See Also:
Tags: ac, navigation, memory
Is this bug a regression?: ---
Last known good build:


Attachments
Test case (559.31 KB, application/octet-stream)
2015-06-16 17:16 UTC, Sergey Komisarchik
Details

Description Sergey Komisarchik 2015-06-16 17:16:12 UTC
Simple test case:
1) get start with Page1
2) navigate to Page2,
3) get back to Page1 with PopAsync() or back button.

Here is a minimal complete example:

public class App : Application
{
    public App()
    {
        this.MainPage = new NavigationPage(new Page1());
    }
}

public class Page1 : ContentPage
{
    public Page1()
    {
        var stack = new StackLayout() { VerticalOptions = LayoutOptions.Center };

        stack.Children.Add(new Label() { VerticalOptions = LayoutOptions.Center, XAlign = TextAlignment.Center, Text = "Page 1" });
        stack.Children.Add(new Button() { Text = "try fix", Command = new Command(this.FixMemoryLeak) });

        this.Content = stack;
    }

    private WeakReference page2Tracker;

    protected override async void OnAppearing()
    {
        base.OnAppearing();

        if (this.page2Tracker == null)
        {
            var page2 = new Page2();

            this.page2Tracker = new WeakReference(page2);

            await Task.Delay(1000);
            await this.Navigation.PushAsync(page2);

            this.StartTrackPage2();
        }
    }

    private void FixMemoryLeak()
    {
        var navPage = (NavigationPage)Application.Current.MainPage;
        navPage.GetType().GetTypeInfo().GetDeclaredProperty("CurrentNavigationTask").SetValue(navPage, null);
    }

    private async void StartTrackPage2()
    {
        while (true)
        {
            ((Label)((StackLayout)this.Content).Children[0]).Text = String.Format("Page1. But Page2 IsAlive = {0}", this.page2Tracker.IsAlive);
            await Task.Delay(1000);
            GC.Collect();
        }
    }
}

public class Page2 : ContentPage
{
    public Page2()
    {
        this.Content = new Label() { VerticalOptions = LayoutOptions.Center, HorizontalOptions = LayoutOptions.Center, Text = "Page 2" };
    }

    protected override async void OnAppearing()
    {
        base.OnAppearing();
            
        await Task.Delay(1000);
        await this.Navigation.PopAsync();
    }
}
Comment 1 Sergey Komisarchik 2015-06-16 17:16:34 UTC
Created attachment 11630 [details]
Test case
Comment 2 Sergey Komisarchik 2015-06-16 17:19:57 UTC
Found with profiler that Task<Page> holds popped Page from the CurrentNavigationTask property of NavigationPage. 

If I null it out then Page is freeing for Android and WP8 SL at least, but not for WinRT.Phone
Comment 3 Jason Smith [MSFT] 2015-06-16 20:27:38 UTC
Thank you for reporting this, we were hoping to get a fix out before anyone noticed :P
Comment 4 Hakan 2016-11-01 15:53:52 UTC
I am experiencing the same issue. Shouldn't be the priority of this bug be high? It is pretty important and fundamental I think. Is anyone working on fixing this?
Comment 5 Paul DiPietro [MSFT] 2016-11-09 22:23:18 UTC
*** Bug 30522 has been marked as a duplicate of this bug. ***
Comment 6 adrianknight89 2016-12-04 06:33:11 UTC
I think this can be fixed with something like:

void SetCurrentNavigationTask(Task task)
{
	CurrentNavigationTask = task;
	CurrentNavigationTask.ContinueWith(t => { CurrentNavigationTask = null; });
}

Looks like there is an existing development branch already.
Comment 7 Samantha Houts [MSFT] 2016-12-08 18:12:57 UTC
https://github.com/xamarin/Xamarin.Forms/pull/625
Comment 8 Paul DiPietro [MSFT] 2017-06-16 18:47:59 UTC
Issue has stalled; setting back to confirmed.
Comment 9 adrianknight89 2017-11-18 17:24:06 UTC
@Paul, any update on this?

Note You need to log in before you can comment on or make changes to this bug.