Bug 31177 - JavaCast<RecyclerView.LayoutParams>() causes leak of RecyclerView.LayoutParams instances
Summary: JavaCast<RecyclerView.LayoutParams>() causes leak of RecyclerView.LayoutParam...
Status: CONFIRMED
Alias: None
Product: Android
Classification: Xamarin
Component: General (show other bugs)
Version: 5.1
Hardware: PC Mac OS
: Normal normal
Target Milestone: ---
Assignee: Jonathan Pryor
URL:
Depends on:
Blocks:
 
Reported: 2015-06-16 19:33 UTC by Brendan Zagaeski (Xamarin Support)
Modified: 2015-06-17 16:06 UTC (History)
3 users (show)

See Also:
Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Test case (33.70 KB, application/zip)
2015-06-16 19:33 UTC, Brendan Zagaeski (Xamarin Support)
Details
Profiler logs and version information (490.94 KB, application/zip)
2015-06-16 19:34 UTC, Brendan Zagaeski (Xamarin Support)
Details
Counter-example using ListView (31.95 KB, application/zip)
2015-06-16 19:35 UTC, Brendan Zagaeski (Xamarin Support)
Details
adb logcat GREF log (2.10 MB, text/plain)
2015-06-17 16:06 UTC, Brendan Zagaeski (Xamarin Support)
Details

Description Brendan Zagaeski (Xamarin Support) 2015-06-16 19:33:11 UTC
Created attachment 11634 [details]
Test case

JavaCast<RecyclerView.LayoutParams>() causes leak of RecyclerView.LayoutParams instances


This might be an unavoidable consequence of how `RecyclerView` is implemented and bound vs. how `ListView` is implemented and bound, but I am filing a bug anyway just on the chance that it is something that should be fixed on the Xamarin side.




## Regression status: probably NOT a regression

### BAD
Xamarin.Android 5.1.3.1 (d419c93)
Xamarin.Android.Support.v7.RecyclerView 21.0.3.0

### BAD
Xamarin.Android 5.1.3.1 (d419c93)
Xamarin.Android.Support.v7.RecyclerView 22.1.1.1

### BAD
Xamarin.Android 4.20.2.1 (86274ad)
Xamarin.Android.Support.v7.RecyclerView 21.0.3.0




## Steps to reproduce

1. Launch the Xamarin Android Player (Nexus 4, KitKat) emulator.


2. Enable heapshot profiling:
> $ adb shell setprop 'debug.mono.profile' 'log:heapshot'


3. Launch the attached test case in the "Debug" configuration on the emulator.


4. Tap the "Hello World, Click Me!" button 10 times.


5. Download the "profile.mlpd" file from the device:
> $ adb pull /data/data/com.companyname.androidapp1/files/.__override__/profile.mlpd


6. Filter the profile data for item counts that increase or decrease over the 10 button clicks:
> $ mprof-report --alloc-sort=count profile.mlpd | grep 'count: [+-][^0]\|Heap shot [0-9]'




## Results with a RecyclerView

The `Android.Support.V7.Widget.RecyclerView/LayoutParams` type leaks:

> Heap shot 0 at 1.689 secs: size: 178304, object count: 3110, class count: 160, roots: 0
> Heap shot 1 at 2.184 secs: size: 178040, object count: 3091, class count: 163, roots: 0
>       1392         87       16 System.WeakReference (bytes: -400, count: -25)
>       1032         12       86 unresolved class 0xb9229d58 (bytes: +24, count: +1)
>        280         10       28 System.Type[] (bytes: +24, count: +1)
> Heap shot 2 at 2.632 secs: size: 178072, object count: 3092, class count: 163, roots: 0
> Heap shot 3 at 3.073 secs: size: 178104, object count: 3093, class count: 163, roots: 0
> Heap shot 4 at 3.504 secs: size: 178136, object count: 3094, class count: 163, roots: 0
> Heap shot 5 at 3.926 secs: size: 178168, object count: 3095, class count: 163, roots: 0
> Heap shot 6 at 4.358 secs: size: 178200, object count: 3096, class count: 163, roots: 0
>        224          7       32 Android.Support.V7.Widget.RecyclerView/LayoutParams (bytes: +32, count: +1)
> Heap shot 7 at 4.824 secs: size: 178232, object count: 3097, class count: 163, roots: 0
>        256          8       32 Android.Support.V7.Widget.RecyclerView/LayoutParams (bytes: +32, count: +1)
> Heap shot 8 at 5.223 secs: size: 178264, object count: 3098, class count: 163, roots: 0
>        288          9       32 Android.Support.V7.Widget.RecyclerView/LayoutParams (bytes: +32, count: +1)
> Heap shot 9 at 5.599 secs: size: 178296, object count: 3099, class count: 163, roots: 0
>        320         10       32 Android.Support.V7.Widget.RecyclerView/LayoutParams (bytes: +32, count: +1)




## "Expected results" (after replacing the RecyclerView with a ListView)

There are no leaks:

> Heap shot 0 at 1.579 secs: size: 132144, object count: 2294, class count: 156, roots: 0
> Heap shot 1 at 2.018 secs: size: 131760, object count: 2270, class count: 156, roots: 0
>       1328         83       16 System.WeakReference (bytes: -384, count: -24)
> Heap shot 2 at 2.458 secs: size: 131760, object count: 2270, class count: 156, roots: 0
> Heap shot 3 at 2.903 secs: size: 131760, object count: 2270, class count: 156, roots: 0
> Heap shot 4 at 3.330 secs: size: 131760, object count: 2270, class count: 156, roots: 0
> Heap shot 5 at 3.729 secs: size: 131760, object count: 2270, class count: 156, roots: 0
> Heap shot 6 at 4.137 secs: size: 131760, object count: 2270, class count: 156, roots: 0
> Heap shot 7 at 4.543 secs: size: 131760, object count: 2270, class count: 156, roots: 0
> Heap shot 8 at 4.943 secs: size: 131760, object count: 2270, class count: 156, roots: 0
> Heap shot 9 at 5.345 secs: size: 131760, object count: 2270, class count: 156, roots: 0




## Workaround

Manually dispose the `LayoutParams` after they are no longer needed. For example, add the following line after the call to `JavaCast<RecyclerView.LayoutParams>()` in `MainActivity.cs`:

> x.Dispose();


### Results

The heapshots no longer show the `RecyclerView/LayoutParams` type.




## Version information (brief)



### Devices

Xamarin Android Player 0.3.7 (1), Nexus 4 (KitKat)



### OS X 10.10.3

=== Xamarin.Android ===

Version: 5.1.3.1 (Enterprise Edition)
Android SDK: /Users/macuser/Library/Developer/Xamarin/android-sdk-macosx
	Supported Android versions:
		2.3    (API level 10)
		4.0.3  (API level 15)
		4.1    (API level 16)
		4.2    (API level 17)
		4.3    (API level 18)
		4.4    (API level 19)
		4.4.87 (API level 20)
		5.0    (API level 21)
Java SDK: /usr
java version "1.8.0"
Java(TM) SE Runtime Environment (build 1.8.0-b132)
Java HotSpot(TM) 64-Bit Server VM (build 25.0-b70, mixed mode)
Comment 1 Brendan Zagaeski (Xamarin Support) 2015-06-16 19:34:04 UTC
Created attachment 11635 [details]
Profiler logs and version information
Comment 2 Brendan Zagaeski (Xamarin Support) 2015-06-16 19:35:44 UTC
Created attachment 11636 [details]
Counter-example using ListView

This is the ListView counter-example that does _not_ show a memory leak.
Comment 3 Jonathan Pryor 2015-06-17 09:54:42 UTC
Is this an actual *leak*, in that the instance is never collected and GREFs constantly increase, or is this just the creation of extra garbage which the GC will eventually collect?

Annotate your code to "somewhere" reliably do:

    System.GC.Collect();
    System.GC.WaitForPendingFinalizers();
    System.GC.Collect();

Does that allow the instances to be collected? Or do they stick around? (Note: this requires that any generator temporaries actually be collectable, so some care must be taken.)

If the instances are "garbage" and are collected by the GC, then this is By Design™ and not a bug. If the instances are not collected and actually leak, that's a problem.
Comment 4 Brendan Zagaeski (Xamarin Support) 2015-06-17 13:35:10 UTC
Yup, the test case in comment 0 includes 1 call each to `GC.Collect()` and `GC.WaitForPendingFinalizers()`. These are executed one time for each button click.

To be careful, I followed comment 3 more precisely and added an extra call to `GC.Collect()` after the call to `GC.WaitForPendingFinalizers()`.

After that change the `RecyclerView.LayoutParams` instances still survive through both garbage collections, so there is at least something "different" happening for the `RecyclerView.LayoutParams` type.


### Excerpt from `mprof-report` output after I added an extra `GC.Collect()` after `GC.WaitForPendingFinalizers()`

> Heap shot 14 at 4.349 secs: size: 189776, object count: 2959, class count: 147, roots: 0
>       1360         85       16 System.WeakReference (bytes: +16, count: +1)
>        312          8       39 Android.Support.V7.Widget.RecyclerView.LayoutParams (bytes: +40, count: +1)
> Heap shot 15 at 4.357 secs: size: 189760, object count: 2958, class count: 147, roots: 0
>       1344         84       16 System.WeakReference (bytes: -16, count: -1)
> Heap shot 16 at 4.788 secs: size: 189816, object count: 2960, class count: 147, roots: 0
>       1360         85       16 System.WeakReference (bytes: +16, count: +1)
>        352          9       39 Android.Support.V7.Widget.RecyclerView.LayoutParams (bytes: +40, count: +1)
> Heap shot 17 at 4.796 secs: size: 189800, object count: 2959, class count: 147, roots: 0
>       1344         84       16 System.WeakReference (bytes: -16, count: -1)
> Heap shot 18 at 5.365 secs: size: 189856, object count: 2961, class count: 147, roots: 0
>       1360         85       16 System.WeakReference (bytes: +16, count: +1)
>        392         10       39 Android.Support.V7.Widget.RecyclerView.LayoutParams (bytes: +40, count: +1)
> Heap shot 19 at 5.373 secs: size: 189840, object count: 2960, class count: 147, roots: 0
>       1344         84       16 System.WeakReference (bytes: -16, count: -1)
Comment 6 Jonathan Pryor 2015-06-17 14:59:10 UTC
Interestingly, your mprof-report output doesn't agree with GREF logging:

    $ adb shell setprop debug.mono.log gref
    $ xbuild  /t:_Run *.csproj
    $ adb logcat
...
> 06-17 14:57:24.377  4150  4150 I monodroid-gref: -w- grefc 97 gwrefc 0 handle 0x737/W from thread 'finalizer'(4150)
...
> 06-17 14:57:33.561  4150  4150 I monodroid-gref: -w- grefc 97 gwrefc 0 handle 0x100737/W from thread 'finalizer'(4150)
...
> 06-17 14:57:48.973  4150  4150 I monodroid-gref: -w- grefc 97 gwrefc 0 handle 0x200737/W from thread 'finalizer'(4150)

TL;DR: The GREF count holds steady at ~97 GREFs. This doesn't look like a leak to me, at least GREF-wise.

Perhaps we should grab the Profiler team to determine why the heap shot size keeps increasing?
Comment 7 Brendan Zagaeski (Xamarin Support) 2015-06-17 16:06:30 UTC
Created attachment 11649 [details]
adb logcat GREF log

Intriguing. On my end I haven't yet been able to reproduce the result where the GREFs hold steady.


I tested on:

- Xamarin Android Player 0.3.7, Nexus 4 (KitKat)

- Google ARM API 19 emulator with Google Play Services

- LG Optimus L9, Android 4.1.2 (API 16) device


### Representative gref results from LG Optimus L9, Android 4.1.2 (API 16) device

(After 10 button clicks, using 2 calls to `GC.Collect()` as discussed in comment 3.)

> 06-17 15:55:07.544 I/monodroid-gref(11831): -w- grefc 102 gwrefc 0 handle 0x1d20014b/W from thread 'finalizer'(11831)
...
> 06-17 15:55:08.334 I/monodroid-gref(11831): -w- grefc 103 gwrefc 0 handle 0x1d20014f/W from thread 'finalizer'(11831)
...
> 06-17 15:55:08.982 I/monodroid-gref(11831): -w- grefc 104 gwrefc 0 handle 0x1d200153/W from thread 'finalizer'(11831)
...
> 06-17 15:55:09.677 I/monodroid-gref(11831): -w- grefc 105 gwrefc 0 handle 0x1d200157/W from thread 'finalizer'(11831)
...
> 06-17 15:55:10.310 I/monodroid-gref(11831): -w- grefc 106 gwrefc 0 handle 0x1d20015b/W from thread 'finalizer'(11831)
...
> 06-17 15:55:11.029 I/monodroid-gref(11831): -w- grefc 107 gwrefc 0 handle 0x1d20015f/W from thread 'finalizer'(11831)
...
> 06-17 15:55:11.630 I/monodroid-gref(11831): -w- grefc 108 gwrefc 0 handle 0x1d200163/W from thread 'finalizer'(11831)
...
> 06-17 15:55:12.146 I/monodroid-gref(11831): -w- grefc 109 gwrefc 0 handle 0x1d200167/W from thread 'finalizer'(11831)
...
> 06-17 15:55:12.669 I/monodroid-gref(11831): -w- grefc 110 gwrefc 0 handle 0x1d20016b/W from thread 'finalizer'(11831)
...
> 06-17 15:55:13.294 I/monodroid-gref(11831): -w- grefc 111 gwrefc 0 handle 0x1d20016f/W from thread 'finalizer'(11831)

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