Bug 43583 - [Android] ListViewRenders leaking
Summary: [Android] ListViewRenders leaking
Status: CONFIRMED
Alias: None
Product: Forms
Classification: Xamarin
Component: Android (show other bugs)
Version: 2.3.4
Hardware: PC Windows
: Highest major
Target Milestone: ---
Assignee: E.Z. Hart [MSFT]
URL:
Depends on:
Blocks:
 
Reported: 2016-08-22 08:25 UTC by Dima
Modified: 2017-10-05 18:17 UTC (History)
9 users (show)

See Also:
Tags: xamexttriage AppCompat android navigation ac
Is this bug a regression?: ---
Last known good build:


Attachments

Description Dima 2016-08-22 08:25:22 UTC
Noticed that in AppCompat when using PopToRootAsync() renderers of previous pages are not disposed.
Page renderers are disposed when Xamarin.Forms page PopAsync() is called but in case of PopToRootAsync it doesn't happens.

Using Xamarin.Forms.Platform.Android.FormsAppCompatActivity

Environment:
Visual Studio 2015 update 2
Xamarin 4.1
Xamarin.Android 6.1
Comment 1 Rui Marinho 2016-08-22 17:46:32 UTC
Hi, we have made some work in this field recently, and on 2.3 versions, can you please test with the latest pre, or please send us a reproduction so we can't see why it isn't fixed yet.

Thanks 

Warm Regards

Xamarin Forms Team
Comment 2 Paul Diston 2016-09-28 14:09:24 UTC
Hi, I believe I am seeing a similar issue, if not the same, on version 2.3.3.152-pre2.

So I have a navigation stack of 2 pages, when I call PopToRootAsync against my NavigationPage.Navigation, the top most page's destructor is called however the destructor of the page below this one is not called.
Comment 3 Paul Diston 2016-09-28 14:09:42 UTC
Hi, I believe I am seeing a similar issue, if not the same, on version 2.3.3.152-pre2.

So I have a navigation stack of 2 pages, when I call PopToRootAsync against my NavigationPage.Navigation, the top most page's destructor is called however the destructor of the page below this one is not called.
Comment 4 Jon Goldberger [MSFT] 2017-06-08 19:35:43 UTC
Setting status as confirmed as I could reproduce this with a test case, a link to which will be provided in a private comment below.

Using Xamarin profiler to profile the app, it looks like the Xamarin.Forms.Platform.Android.PageRenderer is getting leaked, and also Xamarin.Forms.Platform.Android.PinchgestureHandler, PanGestureHandler and TapGestureHandler, but those may be held by the PageRenderer and thus are leaking as well. 

The Xamarin Profiler showed that the number of allocated PageRenderers increased by one with each page push, and the Pinch and Pan GestureHandlers went up by 65 allocations with each page push. 

These were the only definite leaks I found, I imagine other objects that are referenced by the PageRenderer are also being leaked, but I did not comb through every object in the Profiler snapshots to find every leaking object. I will include a link to get the .mlpd file that can be loaded into Xamarin Profiler to see what I was seeing. I took the snapshots at app launch after the UI was displayed and then after steps 2 and 3 (repeatedly) as noted in the steps to reproduce. 


## Steps to reproduce:

1.       Open app, tap any row on the listview
2.       On Page1, tap “Tap This” near the top (note the memory usage in the console output "********* Memory: xxxxx")
3.       Tap any row on page 2
4.       Repeat steps 2 and 3 several times.

Expected result: Total memory usage will not increase steadily

Actual result: Total memory usage continues to increase dramatically. 


## Note

The app never crashed for me with an out of memory error, but that is likely because I was using an emulator with more RAM than the device the customer was testing on. However when profiling, as noted above,
Comment 5 Jon Goldberger [MSFT] 2017-06-08 19:36:45 UTC
Additional Note: I changed the versoin to the current stable version of Forms as my tests used the latest stable version 2.3.4.247
Comment 7 Jon Goldberger [MSFT] 2017-06-08 20:07:34 UTC
Additional comments from the reporting customer:

## "I have been experimenting with an alternative navigation approach which does not require NavigationPage (It’s a single page host pattern that animates in and out new page content as ContentViews), and as expected from your findings the problem persists (I retrofitted the demo solution I sent you with my prototype to test the theory)." 

[The retrofitted demo solution is not included in the test project linked above and I was not provided this]


## "I ran a couple more experiments this afternoon.
 
1.       Ensuring correct use of async / await – in particular making sure all navigation calls were awaited as far up as the event handler. Made no difference.
2.       Move to commands, all ui tap events moved to commands (I suspect the structure under the hood isn’t much difference but thought it was something quick to try) again it made no difference."
Comment 8 Paul DiPietro [MSFT] 2017-06-12 18:05:26 UTC
The reproduction shows the increasing memory values against the nightly. Also checked on a Galaxy S5 which was able to crash, but took a dozen or so repeats of the steps to occur, for reference.
Comment 9 Jon Goldberger [MSFT] 2017-06-12 20:24:08 UTC
I don't think this is technically an issue with PopToRootAsync, at least in the test project I provided.

In an effort to workaround this bug, instead of pushing Page2 onto the stack and then using PopToRootAsync when selecting an item from Page2 and then pushing a new Page1, instead I pushed Page2 as a modal page, then when an item from Page2 is selected I pop the modal page and change the context for Page1 in OnAppearing override. So in this scenario, PopToRootAsync is never used yet the leak still occurs. 

The revised test project that does not use PopToRootAsync is linked in private comment below.
Comment 11 Jon Goldberger [MSFT] 2017-06-13 20:17:25 UTC
Customer who sent the test project said they also tried the following to try to workaround the issue:

"Moved all tap event handler registration from Xaml to page constructors.
Added an interface with a void Popped() method which each page implements and removed the event handlers from the tap events.
Added a simple AppNavigation class that pushes, pops and pops to root, when popping calls Popped on the pages being popped (to clear the event handlers).
This didn’t help, so I also added a GC.Collect just in case it was a case of slow collection (something that plagues android unfortunately)."

Also I tried changing the Labels that were using a TapGestureRecgonizer to Buttons instead. No difference.
Comment 12 Jon Goldberger [MSFT] 2017-06-14 20:20:19 UTC
More things tried to see where issue may be:

Removed the DataCache class and removed all images from UI. No Difference.

Tried without FormsAppCompatActivity, Using FormsApplicationActivity instead. No difference

Removed BoxView and changed Grid to StackLayouts - no difference.

Tried without ListView.CachingStrategy set to RecycleElement. No difference. 

All of the above tried without using PopToRootAsync.
Comment 14 Jon Goldberger [MSFT] 2017-06-14 20:26:45 UTC
Reporting customer also made the following attempts to workaround this leak:

"I tried an entirely different approach (let me know if you want the code and I’ll send it), removing navigation from the equation completely.
 
I converted all pages to ContentViews.
Had a simple single ContentPage to host the entire app.
I then just changed the content of the host page to the appropriate content view.
 
In this scenario the leak still persisted.
 
My code example is a little more complex than this as it animates the transitions and “tracks” a navigation stack, but the principal is the same. Switching the content following a tap event rather than push / pop does not fix the leak."
Comment 15 Jon Goldberger [MSFT] 2017-06-15 20:13:13 UTC
Customer provided a new test project that does not use Forms navigation at all. Instead they only create one page and then just swap out ContentViews. (Project linked in private comment below). 

Profiling this test project it seems to be the ListViewRenderer that is leaking.

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