New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Mono.Android] preserve public methods for GetJniHandleConverterForType #6923
Conversation
Fixes: xamarin#6895 In some cases an app with a single `HttpClient` request can crash with: android.runtime.JavaProxyThrowable: System.ArgumentNullException: ArgumentNull_Generic Arg_ParamName_Name, method at System.Delegate.CreateDelegate(Type , Object , MethodInfo , Boolean , Boolean ) at System.Delegate.CreateDelegate(Type , MethodInfo , Boolean ) at System.Delegate.CreateDelegate(Type , MethodInfo ) at Java.Interop.JavaConvert.GetJniHandleConverterForType(Type ) at Java.Interop.JavaConvert.GetJniHandleConverter(Type ) at Java.Interop.JavaConvert.FromJniHandle[IList`1](IntPtr , JniHandleOwnership , Boolean& ) at Java.Interop.JavaConvert.FromJniHandle[IList`1](IntPtr , JniHandleOwnership ) at Android.Runtime.JavaDictionary`2[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Collections.Generic.IList`1[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].Get(String ) at Android.Runtime.JavaDictionary`2[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Collections.Generic.IList`1[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].get_Item(String ) at Xamarin.Android.Net.AndroidMessageHandler.CopyHeaders(HttpURLConnection , HttpResponseMessage ) at Xamarin.Android.Net.AndroidMessageHandler.DoProcessRequest(HttpRequestMessage , URL , HttpURLConnection , CancellationToken , RequestRedirectionState ) at Xamarin.Android.Net.AndroidMessageHandler.SendAsync(HttpRequestMessage , CancellationToken ) at System.Net.Http.HttpClient.GetStringAsyncCore(HttpRequestMessage , CancellationToken ) at XamarinAndroid.MainActivity.OnCreate(Bundle savedInstanceState) at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object ) at Android.App.SyncContext.<>c__DisplayClass2_0.<Post>b__0() at Java.Lang.Thread.RunnableImplementor.Run() at Java.Lang.IRunnableInvoker.n_Run(IntPtr , IntPtr ) at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(_JniMarshal_PP_V , IntPtr , IntPtr ) at mono.java.lang.RunnableImplementor.n_run(Native Method) at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:30) at android.os.Handler.handleCallback(Handler.java:938) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loopOnce(Looper.java:201) at android.os.Looper.loop(Looper.java:288) at android.app.ActivityThread.main(ActivityThread.java:7870) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003) In this case, `JavaDictionary<,>.FromJniHandle()` was linked away, and we access it via reflection: https://github.com/xamarin/xamarin-android/blob/9f3ee589d9a8cf9eb0d9d0dbe6c6e6acc8fc7f96/src/Mono.Android/Java.Interop/JavaConvert.cs#L87-L92 This works in Xamarin.Android due to: https://github.com/xamarin/xamarin-android/blob/0ab3db079d43d47ad1bc522e28e65204542fea60/src/Xamarin.Android.Build.Tasks/Linker/PreserveLists/Mono.Android.xml#L28 We actually get a linker warning for this: src\Mono.Android\Java.Interop\JavaConvert.cs(89,4): warning IL2070: Java.Interop.JavaConvert.GetJniHandleConverterForType(Type): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethod(String,BindingFlags)'. The parameter 't' of method 'Java.Interop.JavaConvert.GetJniHandleConverterForType(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. Adding `[DynamicallyAccessedMembers]` preserves these methods, and the warning goes away. This seems like a safe option, as we shouldn't need to keep updating a list of types like we did in Xamarin.Android. I tried updating `InstallAndRunTests.CustomLinkDescriptionPreserve()`, but some usage of types is preserving `FromJniHandle()`. I couldn't get the test to fail... Long term, we should get the linker warnings to zero and make our build fail if new warnings are ever introduced. Tracking this in: xamarin#5652
@jonathanpeppers : what I'd like is an elaboration on:
How does |
@jonpryor yes, it's my understanding that in .NET 6 the linker has some static analysis that can tell which |
It sort of is (at least to me). But if you want to read deeper about it, check out:
Which has the basic designs for how these things work. |
…pe (#6923) Fixes: #6895 Context: #5652 In some cases an app with a single `HttpClient` request can crash with: android.runtime.JavaProxyThrowable: System.ArgumentNullException: ArgumentNull_Generic Arg_ParamName_Name, method at System.Delegate.CreateDelegate(Type , Object , MethodInfo , Boolean , Boolean ) at System.Delegate.CreateDelegate(Type , MethodInfo , Boolean ) at System.Delegate.CreateDelegate(Type , MethodInfo ) at Java.Interop.JavaConvert.GetJniHandleConverterForType(Type ) at Java.Interop.JavaConvert.GetJniHandleConverter(Type ) at Java.Interop.JavaConvert.FromJniHandle[IList`1](IntPtr , JniHandleOwnership , Boolean& ) at Java.Interop.JavaConvert.FromJniHandle[IList`1](IntPtr , JniHandleOwnership ) at Android.Runtime.JavaDictionary`2[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Collections.Generic.IList`1[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].Get(String ) at Android.Runtime.JavaDictionary`2[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Collections.Generic.IList`1[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].get_Item(String ) at Xamarin.Android.Net.AndroidMessageHandler.CopyHeaders(HttpURLConnection , HttpResponseMessage ) at Xamarin.Android.Net.AndroidMessageHandler.DoProcessRequest(HttpRequestMessage , URL , HttpURLConnection , CancellationToken , RequestRedirectionState ) at Xamarin.Android.Net.AndroidMessageHandler.SendAsync(HttpRequestMessage , CancellationToken ) at System.Net.Http.HttpClient.GetStringAsyncCore(HttpRequestMessage , CancellationToken ) at XamarinAndroid.MainActivity.OnCreate(Bundle savedInstanceState) at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object ) at Android.App.SyncContext.<>c__DisplayClass2_0.<Post>b__0() at Java.Lang.Thread.RunnableImplementor.Run() at Java.Lang.IRunnableInvoker.n_Run(IntPtr , IntPtr ) at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(_JniMarshal_PP_V , IntPtr , IntPtr ) at mono.java.lang.RunnableImplementor.n_run(Native Method) at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:30) at android.os.Handler.handleCallback(Handler.java:938) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loopOnce(Looper.java:201) at android.os.Looper.loop(Looper.java:288) at android.app.ActivityThread.main(ActivityThread.java:7870) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003) In this case, `JavaDictionary<,>.FromJniHandle()` was linked away, as [we access it via reflection][0]: static Func<IntPtr, JniHandleOwnership, object> GetJniHandleConverterForType (Type t) { MethodInfo m = t.GetMethod ("FromJniHandle", BindingFlags.Static | BindingFlags.Public)!; return (Func<IntPtr, JniHandleOwnership, object>) Delegate.CreateDelegate ( typeof (Func<IntPtr, JniHandleOwnership, object>), m); } This [works in Classic Xamarin.Android][1] because we preserve *all* `Android.Runtime` types, instead of letting the linker sort it out; see commit 15269f6. We actually get a linker warning for this: src\Mono.Android\Java.Interop\JavaConvert.cs(89,4): warning IL2070: Java.Interop.JavaConvert.GetJniHandleConverterForType(Type): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethod(String,BindingFlags)'. The parameter 't' of method 'Java.Interop.JavaConvert.GetJniHandleConverterForType(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. Adding `[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]` to the `Type t` parameter in `GetJniHandleConverterForType()` preserves these methods, and the warning goes away. This seems like a safe option, as we shouldn't need to keep updating a list of types like we did in Xamarin.Android. I tried updating `InstallAndRunTests.CustomLinkDescriptionPreserve()`, but some usage of types is preserving `FromJniHandle()`; I couldn't get the test to fail... Long term, we should get the linker warnings to zero and make our build fail if new warnings are ever introduced. This is tracked in #5652. [0]: https://github.com/xamarin/xamarin-android/blob/9f3ee589d9a8cf9eb0d9d0dbe6c6e6acc8fc7f96/src/Mono.Android/Java.Interop/JavaConvert.cs#L87-L92 [1]: https://github.com/xamarin/xamarin-android/blob/0ab3db079d43d47ad1bc522e28e65204542fea60/src/Xamarin.Android.Build.Tasks/Linker/PreserveLists/Mono.Android.xml#L26
…pe (#6923) Fixes: #6895 Context: #5652 In some cases an app with a single `HttpClient` request can crash with: android.runtime.JavaProxyThrowable: System.ArgumentNullException: ArgumentNull_Generic Arg_ParamName_Name, method at System.Delegate.CreateDelegate(Type , Object , MethodInfo , Boolean , Boolean ) at System.Delegate.CreateDelegate(Type , MethodInfo , Boolean ) at System.Delegate.CreateDelegate(Type , MethodInfo ) at Java.Interop.JavaConvert.GetJniHandleConverterForType(Type ) at Java.Interop.JavaConvert.GetJniHandleConverter(Type ) at Java.Interop.JavaConvert.FromJniHandle[IList`1](IntPtr , JniHandleOwnership , Boolean& ) at Java.Interop.JavaConvert.FromJniHandle[IList`1](IntPtr , JniHandleOwnership ) at Android.Runtime.JavaDictionary`2[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Collections.Generic.IList`1[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].Get(String ) at Android.Runtime.JavaDictionary`2[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Collections.Generic.IList`1[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].get_Item(String ) at Xamarin.Android.Net.AndroidMessageHandler.CopyHeaders(HttpURLConnection , HttpResponseMessage ) at Xamarin.Android.Net.AndroidMessageHandler.DoProcessRequest(HttpRequestMessage , URL , HttpURLConnection , CancellationToken , RequestRedirectionState ) at Xamarin.Android.Net.AndroidMessageHandler.SendAsync(HttpRequestMessage , CancellationToken ) at System.Net.Http.HttpClient.GetStringAsyncCore(HttpRequestMessage , CancellationToken ) at XamarinAndroid.MainActivity.OnCreate(Bundle savedInstanceState) at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object ) at Android.App.SyncContext.<>c__DisplayClass2_0.<Post>b__0() at Java.Lang.Thread.RunnableImplementor.Run() at Java.Lang.IRunnableInvoker.n_Run(IntPtr , IntPtr ) at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(_JniMarshal_PP_V , IntPtr , IntPtr ) at mono.java.lang.RunnableImplementor.n_run(Native Method) at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:30) at android.os.Handler.handleCallback(Handler.java:938) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loopOnce(Looper.java:201) at android.os.Looper.loop(Looper.java:288) at android.app.ActivityThread.main(ActivityThread.java:7870) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003) In this case, `JavaDictionary<,>.FromJniHandle()` was linked away, as [we access it via reflection][0]: static Func<IntPtr, JniHandleOwnership, object> GetJniHandleConverterForType (Type t) { MethodInfo m = t.GetMethod ("FromJniHandle", BindingFlags.Static | BindingFlags.Public)!; return (Func<IntPtr, JniHandleOwnership, object>) Delegate.CreateDelegate ( typeof (Func<IntPtr, JniHandleOwnership, object>), m); } This [works in Classic Xamarin.Android][1] because we preserve *all* `Android.Runtime` types, instead of letting the linker sort it out; see commit 15269f6. We actually get a linker warning for this: src\Mono.Android\Java.Interop\JavaConvert.cs(89,4): warning IL2070: Java.Interop.JavaConvert.GetJniHandleConverterForType(Type): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethod(String,BindingFlags)'. The parameter 't' of method 'Java.Interop.JavaConvert.GetJniHandleConverterForType(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. Adding `[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]` to the `Type t` parameter in `GetJniHandleConverterForType()` preserves these methods, and the warning goes away. This seems like a safe option, as we shouldn't need to keep updating a list of types like we did in Xamarin.Android. I tried updating `InstallAndRunTests.CustomLinkDescriptionPreserve()`, but some usage of types is preserving `FromJniHandle()`; I couldn't get the test to fail... Long term, we should get the linker warnings to zero and make our build fail if new warnings are ever introduced. This is tracked in #5652. [0]: https://github.com/xamarin/xamarin-android/blob/9f3ee589d9a8cf9eb0d9d0dbe6c6e6acc8fc7f96/src/Mono.Android/Java.Interop/JavaConvert.cs#L87-L92 [1]: https://github.com/xamarin/xamarin-android/blob/0ab3db079d43d47ad1bc522e28e65204542fea60/src/Xamarin.Android.Build.Tasks/Linker/PreserveLists/Mono.Android.xml#L26
Fixes: #6895
In some cases an app with a single
HttpClient
request can crash with:In this case,
JavaDictionary<,>.FromJniHandle()
was linked away, andwe access it via reflection:
xamarin-android/src/Mono.Android/Java.Interop/JavaConvert.cs
Lines 87 to 92 in 9f3ee58
This works in Xamarin.Android due to:
xamarin-android/src/Xamarin.Android.Build.Tasks/Linker/PreserveLists/Mono.Android.xml
Lines 25 to 28 in 0ab3db0
We actually get a linker warning for this:
Adding
[DynamicallyAccessedMembers]
preserves these methods, and thewarning goes away. This seems like a safe option, as we shouldn't need
to keep updating a list of types like we did in Xamarin.Android.
I tried updating
InstallAndRunTests.CustomLinkDescriptionPreserve()
,but some usage of types is preserving
FromJniHandle()
. I couldn'tget the test to fail...
Long term, we should get the linker warnings to zero and make our
build fail if new warnings are ever introduced. Tracking this in:
#5652