Thanks Jon. Reply is inline. Jeremy
> -----Original Message----- > From: monodroid-boun...@lists.ximian.com [mailto:monodroid- > boun...@lists.ximian.com] On Behalf Of Jonathan Pryor > Sent: Monday, June 20, 2016 10:52 PM > To: Monodroid for Android > Subject: Re: [mono-android] JavaFinalize and .NET Finalizers > > 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.Pl > atform.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/tro > ubleshooting/#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. > I've tested this on a Tango device which is 4.4 ART (and I've definitely seen funny bugs with this device), as well as the API23 emulator image. > 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/a > rchitecture/#Java_Activation We put Activation constructors almost everywhere because they tend to fix crashes when coming back to an activity. Plus we allow almost all of our classes to be overridden by the user if they need to do so. Maybe in this case MvxRecyclerViewHolder doesn't need to use the constructor. It's hard to tell when it is needed. > > 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. As far as I can tell Dispose() is not being called on MvxRecyclerViewHolder. > > 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. All the values appear to be meaningful data. > > 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 Thanks for the tips. I'll enable debugging/logging and see what I can find. > > _______________________________________________ > Monodroid mailing list > Monodroid@lists.ximian.com > > UNSUBSCRIBE INFORMATION: > http://lists.ximian.com/mailman/listinfo/monodroid _______________________________________________ Monodroid mailing list Monodroid@lists.ximian.com UNSUBSCRIBE INFORMATION: http://lists.ximian.com/mailman/listinfo/monodroid
