Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[typemaps] Handle Java to managed type maps properly (#4656)
Fixes: #4596 Fixes: #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.dll` 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 `EmbeddedAssemblies::binary_search()` ends up selecting it will always return the same managed type name for all aliased Java types. ~~ Release config apps ~~ 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 `BundleTest.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 the Release config app bug in Issue #4660; thanks to @brendanzagaeski. It works by provoking the binary search bug by "ensuring" that a *generic* type comes *before* the "normally preferred" *non*-generic type. Thus, even if/when binary search returns the first entry, that would still be the *wrong entry*, because that'll be a generic type! [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
- Loading branch information