Custom AndroidMessageHandler.ServerCertificateCustomValidationCallback that returns false may cause application crash while debugging. #8608
Assignees
Labels
Area: Mono.Android
Issues with the Android API binding (Mono.Android.dll).
Comments
anton-yashin
added
Area: App Runtime
|
You may run sample code more than one time to crash application. Debugger is required. |
|
To be clear, the issue is that the exception isn't caught, correct? The exception is expected, but the |
|
With VS2022 17.8.3 debugger it's going to crash application, after second time running sample code. Without debugger seems works fine. There something strange happens. |
|
With a debugger, it looks like a crash from an uncaught exception. |
anton-yashin
changed the title
|
Please provide more information about the crash. We would need the following: Any messages VS shows in either its output/debug/etc panes and screenshots of popup boxes (if any) it produces. Full logcat of the application from the device/emulator. To record it, please follow the steps below:
Please attach |
grendello
added
need-info
|
Here requested addtional info |
microsoft-github-policy-service
bot
added
need-attention
|
Your sample uses an
It might be a bug on the Java side for this particular scenario, or something we don't handle well in our use of the certificate manager. @simonrozsival you have more experience and knowledge about this topic than I do, would you be able to look at this issue? Thanks! |
grendello
added
Area: Mono.Android
|
@grendello I'll have a look |
|
My observations:
3. After continuing execution, my breakpoint is triggered (correctly shows the caught exception)
4. After tapping the button again, or even just waiting for ~10 seconds, the app crashes
It seems to me that the exception thrown in the TrustManager (https://github.com/xamarin/xamarin-android/blob/main/src/Mono.Android/Xamarin.Android.Net/ServerCertificateCustomValidator.cs#L73) leaves the thread pool thread in an invalid state when debugging and when we try to run some other Task on the thread, the app crashes. If I understand how the Java interop layer works correctly, the exception is transformed from .NET exception to a matching Java exception when transitioning to Java (https://learn.microsoft.com/en-us/dotnet/api/javax.security.cert.certificateexception?view=xamarin-android-sdk-13). Is it possible that this transition from .NET to Java context breaks when debugging? |
|
@anton-yashin: does it crash in a Release configuration app or when not debugging the app? Cross-VM exception handling in Debug builds is potentially brittle; see also:
From #4548 (comment), if you have a managed method
Emphasis added. The abort you're seeing could be explaind by the JVM callstack being correupted when Your app still fails when not debugging, that would suggest something else is going on, but right now, I believe it's because of our altered exception handling behavior when the debugger is attached. |
|
@jonpryor: This only happens if the debugger is attached. In release and without debugger seems fine. |
Then as I feared, the only "fix" for now is to not have an unhandled first-chance C# exception, and your having Unfortunately, as per @simonrozsival's image, the exception is being thrown from This can't be easily fixed without additional runtime support. |
|
Thanks for your time. |
|
My team seems to be running into this. Are there any other related tickets open? We are just getting organized to look into this. Is there anything we could do to help move things along? |
Would it be possible to raise the Java exception directly through some JNI helper in |
jonpryor
pushed a commit
to dotnet/java-interop
that referenced
this issue
Dec 4, 2024
…1275) Fixes: #1258 Context: c8f3e51 Context: 176240d Context: dotnet/runtime#108211 Context: dotnet/android#9306 Context: dotnet/android#9309 Context: xamarin/monodroid@3e9de5a Context: dotnet/android@8bc7a3e The [Java Native Interface][0] allows native code to be associated with a [Java `native` method declaration][1], either by way of [`Java_`-prefixed native functions][2], or via function pointers provided to [`JNIEnv::RegisterNatives()`][3]. Both `Java_`-prefixed native functions and function pointers must refer to C-callable functions with appropriate [native method arguments][4]. A *Marshal Method* is a: 1. Method or delegate which is C-callable, 2. Accepting the appropriate Java Native Method arguments, 3. Is responsible for marshaling parameter and return types, and 3. *Delegates* the call to an appropriate managed method override. We have multiple different Marshal Method implementations running around, including: * XamarinAndroid1 and XAJavaInterop1 Marshal Methods, in which the Marshal Method is an `n_`-prefixed method in (roughly-ish) the same scope as the method that would be delegated to. * `jnimarshalmethod-gen`: see 176240d * LLVM Marshal Methods, which use LLVM-IR to emit `Java_`-prefixed native functions; see dotnet/android@8bc7a3e8. Which brings us to the current XAJavaInterop1 Marshal Methods implementation. Consider the [`java.util.function.IntConsumer`][5] interface: // Java public /* partial */ interface IntConsumer { void accept(int value); } With `generator --codegen-target=XAJavaInterop1` -- used by .NET for Android -- `IntConsumer` is bound as `IIntConsumer`: namespace Java.Util.Functions { // Metadata.xml XPath interface reference: path="/api/package[@name='java.util.function']/interface[@name='IntConsumer']" [Register ("java/util/function/IntConsumer", "", "Java.Util.Functions.IIntConsumerInvoker", ApiSince = 24)] public partial interface IIntConsumer : IJavaObject, IJavaPeerable { [Register ("accept", "(I)V", "GetAccept_IHandler:Java.Util.Functions.IIntConsumerInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", ApiSince = 24)] void Accept (int value); } [Register ("java/util/function/IntConsumer", DoNotGenerateAcw=true, ApiSince = 24)] internal partial class IIntConsumerInvoker : global::Java.Lang.Object, IIntConsumer { static Delegate? cb_accept_Accept_I_V; static Delegate GetAccept_IHandler () { if (cb_accept_Accept_I_V == null) cb_accept_Accept_I_V = JNINativeWrapper.CreateDelegate (new _JniMarshal_PPI_V (n_Accept_I)); return cb_accept_Accept_I_V; } static void n_Accept_I (IntPtr jnienv, IntPtr native__this, int value) { var __this = global::Java.Lang.Object.GetObject<Java.Util.Functions.IIntConsumer> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!; __this.Accept (value); } } } The Marshal Method is `IIntConsumerInvoker.n_Accept_I()`. We also have a *Connector Method*. A Connector Method is a `static` method matching the signature of `Func<Delegate>`. The name of the connector method is mentioned in the 3rd `connector` parameter of `RegisterAttribute` on the interface method. During [Java Type Registration][6], all Connector methods for a type are looked up and invoked, and the `Delegate` instances returned from all those connector method invocations are provided to `JNIEnv::RegisterNatives()`. There are static and runtime issues with connector method and marshal method implementations until now: 1. Java Native Methods, and thus Marshal Methods, *must* conform to the C ABI. C does not support exceptions. C# *does*. What happens when `__this.Accept(value)` throws? 2. The answer to (1) is in the connector method, via the `JNINativeWrapper.CreateDelegate()` invocation. [`JNINativeWrapper.CreateDelegate()`][7] uses System.Reflection.Emit to *wrap* the Marshal Method with a try/catch block. At runtime, the intermixing of (1) and (2) will result in registering a method similar to the following with `JNIEnv::RegisterNatives()`: static void n_Accept_I (IntPtr jnienv, IntPtr native__this, int value) { JNIEnv.WaitForBridgeProcessing (); try { var __this = ava.Lang.Object.GetObject<IIntConsumer> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!; __this.Accept (value); } catch (Exception e) when (!Debugger.IsAttached) { AndroidEnvironment.UnhandledException (e); } } which presents a further two problems: 1. System.Reflection.Emit is used, which possibly slows down type registration and won't work with NativeAOT. 2. The `catch` block only executes when you're *not* debugging! Which means that if you're debugging the app, and an exception is thrown, you are now potentially unwinding the stack frame through a JNI boundary, which can *corrupt JVM state*, possibly resulting in an [app abort or crash][8]. ([***Why?!***][9]) This has been how things work since the beginning. .NET 9 introduces some features that allow us to rethink all this: * [`DebuggerDisableUserUnhandledExceptionsAttribute`][10] * [`Debugger.BreakForUserUnhandledException(Exception)`][11] > If a .NET Debugger is attached that supports the > [BreakForUserUnhandledException(Exception)][11] API, the debugger > won't break on user-unhandled exceptions when the exception is > caught by a method with this attribute, unless > [BreakForUserUnhandledException(Exception)][11] is called. Embrace .NET 9, remove the possible need for System.Reflection.Emit, and fully prevent possible JVM corruption by updating connector methods and marshal methods to instead be: namespace Java.Util.Functions { internal partial class IIntConsumerInvoker { static Delegate? cb_accept_Accept_I_V; static Delegate GetAccept_IHandler () { return cb_accept_Accept_I_V ??= new _JniMarshal_PPI_V (n_Accept_I); } [DebuggerDisableUserUnhandledExceptions] static void n_Accept_I (IntPtr jnienv, IntPtr native__this, int value) { if (!JniEnvironment.BeginMarshalMethod (jnienv, out var __envp, out var __r)) return; try { var __this = Java.Lang.Object.GetObject<IIntConsumer> (jnienv, native__this, JniHandleOwnership.DoNotTransfer)!; __this.Accept (value); } catch (global::System.Exception __e) { __r?.OnUserUnhandledException (ref __envp, __e); } finally { JniEnvironment.EndMarshalMethod (ref __envp); } } } } This removes the call to `JNINativeWrapper.CreateDelegate()` and it's implicit use of System.Reflection.Emit, properly wraps *everything* in a `try`/`catch` block so that exceptions are properly caught and marshaled back to Java if necessary, and integrates properly with expected "first chance exception" semantics. The *downside* is that this requires the "new debugger backend" to work, which at the time of this writing is only used by VSCode. As this code will only be used for .NET 10+ (2025-Nov), this is fine. [0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/jniTOC.html [1]: http://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#compiling_loading_and_linking_native_methods [2]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#resolving_native_method_names [3]: http://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#RegisterNatives [4]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#native_method_arguments [5]: https://developer.android.com/reference/java/util/function/IntConsumer [6]: https://github.com/dotnet/android/wiki/Blueprint#java-type-registration [7]: https://github.com/dotnet/android/blob/65906e0b7b2f471fcfbd07e7e01b68169c25d9da/src/Mono.Android/Android.Runtime/JNINativeWrapper.cs#L29-L105 [8]: dotnet/android#8608 (comment) [9]: dotnet/android#4877 [10]: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.debuggerdisableuserunhandledexceptionsattribute?view=net-9.0 [11]: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.debugger.breakforuserunhandledexception?view=net-9.0#system-diagnostics-debugger-breakforuserunhandledexception(system-exception)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Android application type
.NET Android (net7.0-android, net8.0-android, etc.)
Affected platform version
VS2022 18.3 with .net 34.0.43/8.0.100
Description
When you using AndroidMessageHandler with ServerCertificateCustomValidationCallback that returns false while debugging you may get application crash.
Steps to Reproduce
Sample code:
See sample project in attachment: UnhandledCertificateException.zip
Did you find any workaround?
No response
Relevant log output
No response
The text was updated successfully, but these errors were encountered: