Commits on May 8, 2020

1. [typemaps] Handle Java to managed type maps properly …

   Fixes: dotnet#4596
   Fixes: dotnet#4660
   Context: xamarin/monodroid@192b1f1
   Context: https://github.com/xamarin/java.interop/blob/fc18c54b2ccf14f444fadac0a1e7e81f837cc470/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/TypeNameMapGenerator.cs#L149-L186
   Context: https://github.com/xamarin/java.interop/blob/010161d1ef6625b762429334f02e7b15e1f7caae/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/TypeNameMapGenerator.cs#L155
   
   ~~ Debug Mode ~~
   
   In some environments, `BundleTest.TestBundleIntegerArrayList2()` fails
   when  using the commercial shared runtime:
   
   	Test 'Xamarin.Android.RuntimeTests.BundleTest.TestBundleIntegerArrayList2' failed: System.MemberAccessException : Cannot create an instance of Android.Runtime.JavaList`1[T] because Type.ContainsGenericParameters is true.
   	  at System.Reflection.RuntimeConstructorInfo.DoInvoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
   	  at System.Reflection.RuntimeConstructorInfo.Invoke (System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
   	  at System.Reflection.ConstructorInfo.Invoke (System.Object[] parameters)
   	  at Java.Interop.TypeManager.CreateProxy (System.Type type, System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer)
   	  at Java.Interop.TypeManager.CreateInstance (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type targetType)
   	  at Java.Lang.Object.GetObject (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type type)
   	  at Java.Lang.Object._GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer)
   	  at Java.Lang.Object.GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer)
   	  at Android.OS.Bundle.Get (System.String key)
   	  at Xamarin.Android.RuntimeTests.BundleTest.TestBundleIntegerArrayList2 ()
   	  at (wrapper managed-to-native) System.Reflection.RuntimeMethodInfo.InternalInvoke(System.Reflection.RuntimeMethodInfo,object,object[],System.Exception&)
   	  at System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
   
   The problem appears to be rooted in ce2bc68.  In a pre-ce2bc689 world,
   there were two separate sets of "typemap" files that the application
   could use:
   
     * Java-to-managed mappings, in `typemap.jm`
     * Managed-to-Java mappings, in `typemap.mj`.
   
   These files did *not* need to have the same number of entries!
   
   	% unzip /Library/Frameworks/Xamarin.Android.framework/Versions/10.2.0.100/lib/xamarin.android/xbuild/Xamarin/Android/Mono.Android.DebugRuntime-debug.apk typemap.jm
   	% unzip /Library/Frameworks/Xamarin.Android.framework/Versions/10.2.0.100/lib/xamarin.android/xbuild/Xamarin/Android/Mono.Android.DebugRuntime-debug.apk typemap.mj
   	% strings typemap.jm | wc -l
   	   10318
   	% strings typemap.mj | wc -l
   	   11956
   
   The reason why they have a different number of entries is *aliasing*:
   there can be *multiple* bindings for a given Java type.  The
   managed-to-Java mappings can contain *all* of them; the
   Java-to-managed mapping *must* pick Only One.
   
   For example, [`java.util.ArrayList`][0] contains (at least?) *three* bindings:
   
     * [`Android.Runtime.JavaList`][1]
     * [`Android.Runtime.JavaList<T>`][2]
     * `Java.Util.ArrayList` (generated at build time)
   
   In a pre-ce2bc689 world, this was "fine": we'd binary search each file
   *separately* to find type mappings.
   
   This changed in ce2bc68, as the typemap information was merged into a
   *single* array in order to de-duplicate information.  This reduced
   `.apk` size.
   
   An unanticipated result of this is that the Java-to-managed mappings
   can now contain *duplicate* keys, "as if" the original `typemap.jm`
   had the contents:
   
     java/util/ArrayList
     Android.Runtime.JavaList, Mono.Android
     java/util/ArrayList
     Android.Runtime.JavaList`1, Mono.Android
     java/util/ArrayList
     Java.Util.ArrayLlist, Mono.Android
   
   Whereas pre-ce2bc689 there would have only been the first entry.
   
   "Sometimes" this duplication is "fine": the typemap search
   "Just happens" to return the first entry, or the 3rd entry, and apps
   continue to work as intended.
   
   "Sometimes" it isn't: the typemap binary search finds the 2nd entry,
   which is a generic type.  This results in attempting to instantiate a
   generic type *without appropriate type parameters*, which results in
   the aforementioned `MemberAccessException`.
   
   Update typemap generation code in `Xamarin.Android.Build.Tasks` so that
   all the duplicate Java type names will point to the same managed type
   name.  Additionally, make sure we select the managed type in the same
   fashion the old typemap generator in `Java.Interop` worked - prefer base
   types to the derived ones if the type is an interface or an abstract
   class.  The effect of this change is that no matter which entry
   `binary_search` ends up selecting it will always return the same managed
   type name for all the aliased Java types.
   
   ~~ Release mode ~~
   
   Another issue introduced in ce2bc68 is that we no longer ignore
   interface and generic types when generating the Java-to-managed maps.
   This adversely affects the `Release` mode in which it is possible that
   an attempt to instantiate an open generic type (or an interface) will
   happen, leading to error similar to:
   
   	FATAL EXCEPTION: main
   	  Process: com.companyname.androidsingleviewapp1, PID: 24068
   	  android.runtime.JavaProxyThrowable: System.MemberAccessException: Cannot create an instance of Com.Contoso.Javalibrary1.Class0`1[T] because Type.ContainsGenericParameters is true.
   	    at System.Reflection.RuntimeConstructorInfo.DoInvoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
   	    at System.Reflection.RuntimeConstructorInfo.Invoke (System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
   	    at System.Reflection.ConstructorInfo.Invoke (System.Object[] parameters)
   	    at Java.Interop.TypeManager.CreateProxy (System.Type type, System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer)
   	    at Java.Interop.TypeManager.CreateInstance (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type targetType)
   	    at Java.Lang.Object.GetObject (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type type)
   	    at Java.Lang.Object._GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer)
   	    at Java.Lang.Object.GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer)
   	    at Com.Contoso.Javalibrary1.Class1.Create ()
   	    at AndroidSingleViewApp1.MainActivity.OnCreate (Android.OS.Bundle savedInstanceState)
   	    at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_ (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_savedInstanceState)
   	    at (wrapper dynamic-method) Android.Runtime.DynamicMethodNameCounter.1(intptr,intptr,intptr)
   	         at crc647d7dcab8da8ee782.MainActivity.n_onCreate(Native Method)
   	         at crc647d7dcab8da8ee782.MainActivity.onCreate(MainActivity.java:29)
   	         at android.app.Activity.performCreate(Activity.java:7144)
   	         at android.app.Activity.performCreate(Activity.java:7135)
   	         at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1271)
   	         at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2931)
   	         at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3086)
   	         at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:78)
   	         at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:108)
   	         at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:68)
   	         at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1816)
   	         at android.os.Handler.dispatchMessage(Handler.java:106)
   	         at android.os.Looper.loop(Looper.java:193)
   	         at android.app.ActivityThread.main(ActivityThread.java:6718)
   	         at java.lang.reflect.Method.invoke(Native Method)
   	         at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
   	         at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)
   
   Update typemap generation code to also ignore generic types and
   interfaces when generating the Java to Managed map in `Release` mode. We
   must not simply skip outputting entries for such types because we need
   to maintain element count parity between java-to-managed and
   managed-to-java lookup tables.  Therefore, the way we skip such types is
   to output a type token ID of `0` instead of the actual one.  This will
   cause the runtime code to fail to find a mapping, thus allowing the
   managed code to deal with such a type properly.
   
   ~~ Test Updates ~~
   
   Update `TestBundleIntegerArrayList2()` so that it asserts
   the runtime *type* of `obj` returned, asserting that it is in fact a
   `JavaList` and not e.g. `Java.Util.ArrayList` or something.
   (The linker could otherwise "change" things, and this assertion will
   ensure that `JavaList` won't be linked away…)
   
   Add a unit test for issue dotnet#4660 (by @brendanzagaeski)
   
   It's very cute. :-)
   
   We sort the Java-to-managed type name table, so it can be binary
   searched to find the Managed type.
   
   Additionally, the "preferred" managed type is the *first* type in
   `monodis --typedef` output; the first type that we encounter when
   going through all types in an assembly.
   
   Brendan's cute way of provoking the bug is to add a new *generic* type
   which will come *before* the "normally preferred" (and correct!) type.
   Thus, even if/when binary search returns the first entry, that'll
   still be the *wrong entry*, because that'll be a generic type!
   
   Very cute.
   
   [0]: https://developer.android.com/reference/java/util/ArrayList
   [1]: https://github.com/xamarin/xamarin-android/blob/61f09bf2ef0c8f09550e2d77eb97f417432b315b/src/Mono.Android/Android.Runtime/JavaList.cs#L11-L13
   [2]: https://github.com/xamarin/xamarin-android/blob/61f09bf2ef0c8f09550e2d77eb97f417432b315b/src/Mono.Android/Android.Runtime/JavaList.cs#L667-L668

   

   grendello committed May 8, 2020
   Configuration menu

   • View commit details
   • Copy full SHA for f56d333
   • Browse repository at this point

   Copy the full SHA

   f56d333 View commit details

   Browse the repository at this point in the history