Protect Android Bindings

[kjeremy]

Occasionally we can get in a state where an object has been garbage collected by the Android GC but not Mono. This is most common with MvxRecyclerView where we can't deterministically destroy the bindings.

To protect against this we need to detect invalid Java Handles and skip setting binding values. Part of this change introduces a weak event subscription model that also takes this into account. We now weak subscribe to Android Views/Widgets in an attempt to reduce the refcount on the Mono side. The latter change is more of a precautionary measure but is part of the reason we're seeing: #1405

Fixes: #1402

This is the first part of a two part fix for:
#1405
#1444

[Cheesebaron]

I just looked through this. It seems to achieve the same thing I did in one of my PR's but ran into the issue that EventHandler cannot be cast to EventHandler, which lead to some issues with detecting whether the Event was present on the View, through reflection.

This seems like a nice way of tackling this and makes Android bindings weak, yay!

I'll give this a spin soon 👍

[kjeremy]

I spent a LOT of time trying to figure out why C# can't tell the difference between Func(TSource source, EventHandler handler) and Func(TSource source, EventHandler<TEventArgs> handler) at the caller site.

I'm not sure if the other platforms should also weak subscribe to their event handlers as well (separate change). What makes Android special is that you can get into this weird undead (but not zombie) object state.

[Cheesebaron]

Something is up with SelectedItem binding for MvxRadioGroup.

It doesn't work until I change the FillTargetFactories registration from MvxRadioGroup to RadioGroup. After that change it triggers. However, a new problem appears:

10-16 22:38:23.755 I/MonoDroid( 3106): UNHANDLED EXCEPTION:
10-16 22:38:23.755 I/MonoDroid( 3106): System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.InvalidCastException: Specified cast is not valid.
10-16 22:38:23.755 I/MonoDroid( 3106):   at MvvmCross.Binding.Droid.Target.MvxRadioGroupSelectedItemBinding.RadioGroupCheckedChanged (System.Object sender, Android.Widget.RadioGroup+CheckedChangeEventArgs args) [0x00001] in D:\git\MvvmCross-kj\MvvmCross\Binding\Droid\Target\MvxRadioGroupSelectedItemBinding.cs:45 
10-16 22:38:23.755 I/MonoDroid( 3106):   at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
10-16 22:38:23.755 I/MonoDroid( 3106):   at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00038] in /Users/builder/data/lanes/3819/96c7ba6c/source/mono/mcs/class/corlib/System.Reflection/MonoMethod.cs:305 
10-16 22:38:23.755 I/MonoDroid( 3106):    --- End of inner exception stack trace ---
10-16 22:38:23.755 I/MonoDroid( 3106):   at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00050] in /Users/builder/data/lanes/3819/96c7ba6c/source/mono/mcs/class/corlib/System.Reflection/MonoMethod.cs:313 
10-16 22:38:23.755 I/MonoDroid( 3106):   at System.Reflection.MethodBase.Invoke (System.Object obj, System.Object[] parameters) [0x00000] in /Users/builder/data/lanes/3819/96c7ba6c/source/mono/mcs/class/referencesource/mscorlib/system/reflection/methodbase.cs:229 
10-16 22:38:23.755 I/MonoDroid( 3106):   at MvvmCross.Platform.WeakSubscription.MvxWeakEventSubscription`2[TSource,TEventArgs].OnSourceEvent (System.Object sender, TEventArgs e) [0x00011] in D:\git\MvvmCross-kj\MvvmCross\Platform\Platform\WeakSubscription\MvxWeakEventSubscription.cs:77 
10-16 22:38:23.755 I/MonoDroid( 3106):   at Android.Widget.RadioGroup+IOnCheckedChangeListenerImplementor.OnCheckedChanged (Android.Widget.RadioGroup group, System.Int32 checkedId) [0x0000d] in /Users/builder/data/lanes/3819/96c7ba6c/source/monodroid/src/Mono.Android/platforms/android-24/src/generated/Android.Widget.RadioGroup.cs:266 
10-16 22:38:23.755 I/MonoDroid( 3106):   at Android.Widget.RadioGroup+IOnCheckedChangeListenerInvoker.n_OnCheckedChanged_Landroid_widget_RadioGroup_I (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_group, System.Int32 checkedId) [0x00011] in /Users/builder/data/lanes/3819/96c7ba6c/source/monodroid/src/Mono.Android/platforms/android-24/src/generated/Android.Widget.RadioGroup.cs:217 
10-16 22:38:23.755 I/MonoDroid( 3106):   at (wrapper dynamic-method) System.Object:051aaf09-1fcb-4a93-83b0-f6b728639adb (intptr,intptr,intptr,int)

EDIT:

Just changed the casts to RadioGroup in the MvxRadioGroupSelectedItemBinding and it magically works again? What the heck is going on? I am using MvxRadioGroup in my axml file... Why does the cast not work?

EDIT EDIT:

Apart from this strangeness with MvxRadioGroup, I think this is good to go when merge conflict has been resolved. I've tested with the APIExample where I added explicit GC on all property changed, which does not seem to break bindings.

[kjeremy]

I bet you that MvxRadioGroup was swapped with MvxAppCompatRadioGroup

[Cheesebaron]

Aw crap! You might be right! I guess it is good then. These changes to use WeakSubscribe instead should ofc also be present in AndroidSupport.

[kjeremy]

There is a PR open for that (MvvmCross/MvvmCross-AndroidSupport#300). Also: I'm surprised that binding worked for RadioGroup. I thought it would need something specific that MvxRadioGroup provides.

[Cheesebaron]

SelectedItem binding will only work on MvxRadioGroup because of the way MvxAdapter wraps stuff in MvxListItemView. However, for the binding target, I don't see a reason why we couldn't just apply it on RadioGroup since it doesn't depend on anything in MvxRadioGroup directly.

[kjeremy]

Cool. Let's take the binding discussion over to #1466. This probably isn't the only widget where we hit that problem.

[kjeremy]

@Cheesebaron hey is this good? Did you look at the support lib version?

[Cheesebaron]

I think this is good. Only thing was the MvxRadioGroup. I guess well take that in the issue you linked. So fix the merge conflict and we'll merge this.

[kjeremy]

Rebased onto develop.

[kjeremy]

Yes this is fixed. You are still seeing the issue in the latest?

…

On Dec 5, 2016 5:40 AM, "Artiom K" ***@***.***> wrote: @kjeremy <https://github.com/kjeremy> could you clarify, is the #1405 <#1405> fixed by this PR? I can see that the issue was closed but we still get this errors. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1446 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEIBRCP6UjaWFoRfdh0SlMs4SA75V_nuks5rE-o5gaJpZM4KMNAM> .