On Jun 17, 2016, at 9:50 AM, Jeremy A. Kolb - ARA/NED <jk...@ara.com> wrote:
> I'm investigating https://github.com/MvvmCross/MvvmCross/issues/1343 for a
> possible memory leak related to our version of the RecyclerView widget
> MvxRecyclerView and have run against an interesting issue.
>
> When swapping out the itemsource in the adapter the RecyclerView instantiates
> ViewHolders but never seems to kill them no matter how many times I force a
> GC. If I add a "~MvxRecyclerViewHolder()" method to MvxRecyclerViewHolder I
> never get in there. However I do hit the JavaFinalize method and now I am
> completely confused.
>
> Why would I hit the JavaFinalizer and not the .NET one? There is never an
> explicit Dispose on the MvxRecyclerView so I can't see why the Java -> .NET
> connection would be severed.
>
> The only instance I've found where Xamarin uses JavaFinalize is at
> https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Platform.Android/Renderers/GenericAnimatorListener.cs#L35
> which doesn't explain much.
>
> Am I seeing a bug in Xamarin? In MvvmCross's RecyclerView? Or more likely:
> am I just confused about the interaction of two garbage collectors.
The interaction is deep voodoo. :-)
As usual with these things, enable GREF logging:
https://developer.xamarin.com/guides/android/troubleshooting/troubleshooting/#Global_Reference_Messages
https://developer.xamarin.com/releases/android/xamarin.android_5/xamarin.android_5.1/#GREF_Logging_Changes
$ adb shell setprop debug.mono.log gref,gc
The relevant GC bridge code is `take_global_ref_jni()`,
`take_weak_global_ref_jni()`, and `gc_cross_references()`:
https://github.com/xamarin/xamarin-android/blob/c3811e8/src/monodroid/jni/monodroid-glue.c#L1140-L1202
https://github.com/xamarin/xamarin-android/blob/c3811e8/src/monodroid/jni/monodroid-glue.c#L1483-L1522
This is run as part of Mono’s GC bridge code…which I’m kinda flaky on. The
*general* idea is as follows:
1. Mono’s GC looks for finalizable objects.
2. When a finalizable instance has no more managed references, it’s placed into
some finalizer queue
3. *Before* the finalizer queue is emptied, the GC bridge is queried to
determine if the instance is *actually* finalizable.
3.a. If the instance can be finalized, it will be during the next GC.
3.b. If the instance can’t be finalized, it will be kept alive.
The GC bridge, in this context, is `gc_cross_references()`, which is given a
set of Java.Lang.Object instances, and:
1. Toggles all the GREFs to Weak GREFs…for the *entire object graph* the
instance refers to. (Plus some mirroring of the object graph in Java-land…)
2. Kicks off a Java-side GC.
3. Checks to see if anything was collected. If it was collected, Mono’s GC is
told that the instance can be finalized. Otherwise, the instance is kept alive.
4. For all alive instances, toggle the Weak GREFs to GREFs.
Note: mid-term plan is to unify that code with the ~equivalent code here:
https://github.com/xamarin/java.interop/blob/master/src/java-interop/java-interop-gc-bridge-mono.c
That project keeps getting punted as more important work pops up…
Very important question: what Android version do you see this on? There are two
Weak reference implementations, one that uses “proper” JNI Weak Global
References, and one which uses java.lang.WeakReference (because API-8 was the
first to support JNI Weak Global references, and then Android 4.4 + ART broke
them…). The WeakReference implementation doesn’t get as much testing, so…it’s
possibly suspect unless you can discount it.
Returning to the original description…
`Java.Lang.Object.Handle` is a JNI Global Reference, which *should* keep the
instance alive. Period. Thus, it shouldn’t be possible for
`Object.JavaFinalize()` to be invoked while `Object.Handle` is not IntPtr.Zero.
I can think of *one* way it would happen, but `GenericAnimatorListener` doesn’t
trigger it: if the class had a `T(IntPtr, JniHandleOwnership)` constructor,
*and* the instance were `Dispose()`d, then I can see `JavaFinalize()` being
invoked:
class Silly : Java.Lang.Object {
public Silly () {}
public Silly (IntPtr h, JniHandleOwnership t) : base (h, t) {}
protected override void JavaFinalize () {
}
}
// …
using (var s = new Silly ()) {
}
Once `s.Dispose()` is invoked, nothing will keep the Java-side peer alive, so
later `Object.finalize()` may eventually be invoked by the Java GC, but since
there is no longer a registered peer for the JNI instance, we’ll invoke the
`Silly(IntPtr, JniHandleOwnership)` constructor to *create a new* peer, and
invoke `Silly.JavaFinalize()` on the newly created peer, *not* the original
instance:
// above `using` block + subsequent behavior is as if…
new Silly ().Dispose();
// time passes...
new Silly (value, JniHandleOwnership.DoNotTransfer).JavaFinalize();
That’s clearly not the case in GenericAnimatorListener — it doesn’t have the
right constructor.
That *may* be the case in MvxRecyclerViewHolder, as it *does* have the
activation constructor:
https://github.com/MvvmCross/MvvmCross-AndroidSupport/blob/e88c556/MvvmCross.Droid.Support.V7.RecyclerView/MvxRecyclerViewHolder.cs#L107-L109
public MvxRecyclerViewHolder(IntPtr handle, JniHandleOwnership transfer)
: base(handle, transfer)
{}
I would thus suggest you look into *why* `MvxRecyclerViewHolder ` has an
Activation constructor. I strongly suggest *avoiding* activation constructors
unless absolutely necessary:
https://developer.xamarin.com/guides/android/under_the_hood/architecture/#Java_Activation
The Activation constructor has a tendency to “hide” or “change” the behavior of
actual buggy behavior, so the fact that `MvxRecyclerViewHolder` provides an
activation constructor without overriding any virtual methods (other than
Dispose()) does not fill me with confidence.
If this is happening, you’d see the `MvxRecyclerViewHolder._bindingContext` is
null — along with all over members! You could check for that within your
`JavaFinalize()` override, and see if the instance has “meaningful” data
compared to what the instance had after construction.
Additionally, to track *managed* instance lifetimes, I suggest copious use of
RuntimeHelpers.GetHashCode(object):
https://msdn.microsoft.com/en-us/library/11tbk3h9(v=vs.110).aspx
- Jon
_______________________________________________
Monodroid mailing list
Monodroid@lists.ximian.com
UNSUBSCRIBE INFORMATION:
http://lists.ximian.com/mailman/listinfo/monodroid
