Skip to content
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

Raise the AppDomain.UnhandledException event for exceptions outside the runtime. #102730

Open
rolfbjarne opened this issue May 27, 2024 · 21 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime.InteropServices
Milestone

Comments

@rolfbjarne
Copy link
Member

rolfbjarne commented May 27, 2024

Background and motivation

Extension of prior exception handling utilities from #101560.

In scenarios where .NET is not the process owner (for example, iOS or Android), it can occur that an exception is about to take down the process but that it didn't originate in the .NET environment. Having a way to trigger .NET's unhandled exception infrastructure for applications would allow for improved application UX.

This event is often used by customers when logging app crashes, so the fact that we don't always raise it becomes a problem (dotnet/macios#15252).

API Proposal

The API would return after running any supplied handlers. It is expected that the process would terminate shortly after this returns. Any handler set via ExceptionHandling.SetUnhandledExceptionHandler() would not be called since this is coming from outside of the .NET environment and therefore it isn't possible for .NET to ensure the exception can be ignored from the other system.

namespace System.Runtime.ExceptionServices;

public static class ExceptionHandling
{
+    public static void ReportUnhandledException(Exception exception);
}

API Usage

[UnmanagedCallersOnly]
public static void ExternalCaller(IntPtr externalExceptionData)
{
    // Marshal the externalExceptionData to a .NET Exception that represents
    // an unhandled exception outside of the .NET environment.
    Exception e = ...
    
    ExceptionHandling. ReportUnhandledException(e);
}

static void MyExceptionHandler(object sender, UnhandledExceptionEventArgs args)
{
    Debug.Assert(args.IsTerminating);
    Exception e = (Exception)args.ExceptionObject;
    Console.WriteLine("MyHandler caught : {0}", e.Message);
}

AppDomain.UnhandledException += MyExceptionHandler;

Alternative Designs

Original proposal

Background and motivation

The ObjectiveCMarshal.Initialize method takes a callback that's called if an exception is supposed to be thrown when returning from managed code to native code.

This works fine, but if we determine that the exception is truly unhandled, there doesn't seem to be a way for us to invoke the AppDomain.UnhandledException event before terminating the process.

This event is often used by customers when logging app crashes, so the fact that we don't always raise it becomes a problem (dotnet/macios#15252). We have other events (our own) we raise, but if we could raise the event everybody else uses that would be preferrable.

Note that we'd need the same for Mono. Looks like we can use mono_unhandled_exception for Mono.

API Proposal

namespace System.Runtime.InteropServices.ObjectiveC;

public static class ObjectiveCMarshal 
{
	// This raises the AppDomain.UnhandledException event
    public void OnUnhandledException (Exception exception);
}

API Usage

The method would be called when we detect that there are no Objective-C exception handlers nor any managed exception handlers on the stack.

Currently this happens:

  • The ObjectiveCMarshal unhandled exception propagation handler is called.
  • We convert the managed exception into an Objective-C exception and throw the Objective-C exception.
  • Objective-C doesn't find any Objective-C exception handlers, and calls our Objective-C unhandled exception callback
  • In our Objective-C unhandled exception callback, we abort the process. This is where we'd like to raise the AppDomain.UnhandledException event, before aborting. Note that we still have access to the original managed exception at this point.

It would also be nice if we could tell the debugger about these unhandled exception as well, but I don't have any idea how that would look.

Alternative Designs

No response

Risks

No response

@rolfbjarne rolfbjarne added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label May 27, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 27, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label May 27, 2024
@rolfbjarne
Copy link
Member Author

CC @AaronRobinsonMSFT

@teo-tsirpanis teo-tsirpanis added area-System.Runtime.InteropServices and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 27, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

@AaronRobinsonMSFT
Copy link
Member

@rolfbjarne Would #101560 help with this issue? If not, is this a .NET 9 ask?

@rolfbjarne
Copy link
Member Author

@rolfbjarne Would #101560 help with this issue?

No, I don't think so, that's a proposal to support ignoring unhandled exceptions, while our exceptions can't be ignored, the process will terminate soon after no matter what the handler does.

The fatal error handler would eventually be called, but getting the unhandled managed exception (to log it for instance) after the error handled has been called due to a SIGABRT wouldn't be easy, so it's not really a good alternative.

If not, is this a .NET 9 ask?

Yes, please!

@janvorli
Copy link
Member

Maybe that in the Objective-C unhandled exception callback, we can unwind to the first managed frame and rethrow the managed exception from there. That way, the exception that was not handled by the Objective-C stuff would continue flowing into the managed caller of that code and possibly get even handled in the managed code or hit the standard unhandled exception path that would also raise the AppDomain.UnhandledException event.
There are some unknowns to me though that can complicate things. For example, I am not sure if objective-C frames can be unwound using the libunwind.

@rolfbjarne
Copy link
Member Author

Maybe that in the Objective-C unhandled exception callback, we can unwind to the first managed frame and rethrow the managed exception from there.

There isn't necessarily any managed frames up the stack (in fact probably won't be any).

Also note that there may be both managed and Objective-C exception handled up the stack... because Apple does something like this sometimes:

void someFunction ()
{
    @try {
        callUserCode ();
    } @catch (NSException *ex) {
        terminateDueToUnhandledObjectiveCException (ex);
    }
}

Stack trace: https://gist.github.com/rolfbjarne/5ab15ce73bc6476d4cee83cff387dd30

frame 0 is our unhandled Objective-C exception callback.
frame 6 is (probably) something like the someFunction from above.
frame 20 has an Objective-C catch clause.
frames 26-23 are managed frames.

In any case, it doesn't sound like a good idea to try to recover and continue executing from this stack trace:

    frame #2: 0x000000018005fbe4 libobjc.A.dylib`_objc_terminate() + 112
    frame #3: 0x000000018028e150 libc++abi.dylib`std::__terminate(void (*)()) + 12
    frame #4: 0x000000018028e100 libc++abi.dylib`std::terminate() + 52
    frame #5: 0x00000001800842d4 libobjc.A.dylib`objc_terminate + 12

That way, the exception that was not handled by the Objective-C stuff would continue flowing into the managed caller of that code and possibly get even handled in the managed code or hit the standard unhandled exception path that would also raise the AppDomain.UnhandledException event.

We end up with a kind of chicken-and-egg problem here: exceptions are only truly unhandled if there are neither Objective-C exception handlers nor managed exception handlers up the stack.

  1. We convert Objective-C exceptions into managed exceptions after calling Objective-C selectors that throw Objective-C exceptions.
  2. We convert managed exceptions into Objective-C exceptions when returning to native code and a managed exception occurred.

This means that (almost) all unhandled managed exceptions will actually be converted into Objective-C exceptions at some point.

We reach the end when the there are no Objective-C exception handlers on the stack, and the unhandled Objective-C exception callback is called. At this point we can inspect the Objective-C exception, and determine whether it originated from a managed exception (and this is where we want to call AppDomain.UnhandledException).

@janvorli
Copy link
Member

Ok, I can see I have not considered the cases when objective-C thread is reverse-pinvoking managed code or when there is an objective-C host. What I was talking about was a case when managed code app would call objective-C method.
I have also not expected that objective C would call its unhandled exception handler down the call chain from the std::terminate in what I assume is a termination handler. I am not sure if it would be safe to execute any managed code (the AppDomain.UnhandledException) on that thread from that state at all.

rolfbjarne added a commit to dotnet/macios that referenced this issue May 29, 2024
…xception. (#20656)

Call mono_unhandled_exception to raise AppDomain.UnhandledException when
managed exceptions are unhandled.

Partial fix for #15252 (for MonoVM, still pending for CoreCLR, which
needs dotnet/runtime#102730 fixed first).
@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Jun 5, 2024

@VSadov Do you have any thoughts on adding an additional API to #101560? The issue here is attempting to trigger the runtime's UnhandledException infrastructure on demand. It seems like it fits nicely with the aforementioned API. The API itself is likely uninteresting, but the semantics definitely are.

namespace System.Runtime.ExceptionServices
{
    public static class ExceptionHandling
    {
        /// <summary>
        /// Triggers the runtime's UnhandledException infrastructure.
        /// </summary>
        /// <param name="exception">Exception to trigger</exception>
        /// <remarks>
        /// QUESTION: What happens on return?
        /// </remarks>
        public static void TriggerUnhandledExceptionHandlers(Exception exception);
    }
}

/cc @jkotas

@stephentoub stephentoub added this to the Future milestone Jul 19, 2024
@jeffschwMSFT jeffschwMSFT removed the untriaged New issue has not been triaged by the area owner label Jul 24, 2024
@rolfbjarne
Copy link
Member Author

@AaronRobinsonMSFT @jkotas any updates on this?

Is there still time to get this in .NET 9?

@jkotas
Copy link
Member

jkotas commented Sep 13, 2024

.NET 9 is done. We should address this scenario as part of #101560 in .NET 10.

@jkotas jkotas modified the milestones: Future, 10.0.0 Sep 13, 2024
@vitek-karas
Copy link
Member

@jonpryor could you please add the description of the scenarios for Android interop?

@AaronRobinsonMSFT
Copy link
Member

@rolfbjarne I'm going to take over this API proposal and update it based on what I defined in #102730 (comment). I'll move the original proposal into a hidden section.

@AaronRobinsonMSFT
Copy link
Member

@jonpryor I've updated this proposal with a general purpose API that should be usable from Android.

@AaronRobinsonMSFT AaronRobinsonMSFT changed the title Raise the AppDomain.UnhandledException event from Objective-C bridge Raise the AppDomain.UnhandledException event for exceptions outside the runtime. Mar 27, 2025
@AaronRobinsonMSFT AaronRobinsonMSFT changed the title Raise the AppDomain.UnhandledException event for exceptions outside the runtime. Raise the AppDomain.UnhandledException event for exceptions outside the runtime. Mar 27, 2025
@jkotas
Copy link
Member

jkotas commented Mar 28, 2025

Mark the API with [System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute]?

@jkotas
Copy link
Member

jkotas commented Mar 28, 2025

Any handler set via ExceptionHandling.SetUnhandledExceptionHandler() would not be called

The API should not be called TriggerUnhandledExceptionHandlers then.

Some naming ideas: FailWithUnhandledException, TriggerUnhandledException, ReportUnhandledException.

@jonpryor
Copy link
Member

@vitek-karas asked:

could you please add the description of the scenarios for Android interop?

What is the developer expectation for an unhandled exception?

class App {
    public static void Main () => throw new System.Exception("wee!!!");
}

An answer: the unhandled exception "experience" occurs, one of the items of which is that the AppDomain.UnhandledException event is raised.

Now, what should happen if an unhandled exception happens from a Java thread?

partial class MyActivity : Activity {
    // OnCreate() will be invoked from the UI thread, which is a Java thread
    protected override void OnCreate(Bundle? savedInstanceState) =>
        throw new Exception ("wee?"); 
}

On .NET for Android with MonoVM, a similar "unhandled exception experience" occurs:

  1. MyActivity.OnCreate() throws the exception.

  2. The "marshal method" that sits between the MyActivity.onCreate() method of the Java peer and the C# MyActivity.OnCreate() method override catches the Exception, wraps it into a JavaProxyThrowable, and raises the JavaProxyThrowable on the Java side.

    See also: dotnet/java-interop@356485e
    See also: JNIEnv::Throw()

  3. The JavaProxyThrowable is raised on the Java side, which looks for a catch block, and won't find one.

  4. Java (eventually) looks for a Thread.UncaughtExceptionHandler which was registered via Thread.setDefaultUncaughtExceptionHandler(). If it finds one, Java calls Thread.UncaughtExceptionHandler.uncaughtException().

  5. .NET for Android registers a XamarinUncaughtExceptionHandler at startup:

  6. XamarinUncaughtExceptionHandler.uncaughtException() eventually hits JNIEnv.PropagateUncaughtException(), which "unwraps" the exception, obtaining the original Exception from the JavaProxyThrowable, and then -- on MonoVM -- calls mono_unhandled_exception(), which eventually causes thye AppDomain.UnhandledException event to be raised.

  7. Execution should then return to Java, which will allow Java to do…whatever it wants to do.

@jonpryor
Copy link
Member

@AaronRobinsonMSFT: I like your idea of an ExceptionHandling.TriggerUnhandledExceptionHandlers() that would "just" raise AppDomain.UnhandledException and related behaviors (notify any attached debuggers, whatever).

@jkotas: given my above description on Android semantics, I do not think it would be appropriate to mark the new API with [System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute], as execution should eventually return to Java (step 7), and my unresearched read on "DoesNotReturnAttribute" implies that execution would not return to Java in that case.

@AaronRobinsonMSFT
Copy link
Member

Any handler set via ExceptionHandling.SetUnhandledExceptionHandler() would not be called

The API should not be called TriggerUnhandledExceptionHandlers then.

Good point.

Some naming ideas: FailWithUnhandledException, TriggerUnhandledException, ReportUnhandledException.

I like ReportUnhandledException.

@jkotas: given #102730 (comment), I do not think it would be appropriate to mark the new API with [System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute], as execution should eventually return to Java (step 7), and my unresearched read on "DoesNotReturnAttribute" implies that execution would not return to Java in that case.

I agree with this. Applying the DoesNotReturnAttribute seems odd to me since the expectation is this will always return to the caller. Do we have prior art for applying this attribute but it returning with expectations on the caller?

@jkotas
Copy link
Member

jkotas commented Mar 28, 2025

I have missed the detail that the API is expected to return.

related behaviors (notify any attached debuggers, whatever

What are the related behaviors exactly? As far as I can tell, Mono's mono_unhandled_exception raises AppDomain.UnhandledException event, prints the exception to console if no event handler is registered (it is odd that it is done only when no handler registered - I do not think we want to do it like that), and sets exit code to 1 (we have separate API to set exit code):

  • mono_unhandled_exception (MonoObject *exc)
    {
    MONO_EXTERNAL_ONLY_GC_UNSAFE_VOID (mono_unhandled_exception_internal (exc));
    }
  • mono_unhandled_exception_internal (MonoObject *exc_raw)
    {
    ERROR_DECL (error);
    HANDLE_FUNCTION_ENTER ();
    MONO_HANDLE_DCL (MonoObject, exc);
    mono_unhandled_exception_checked (exc, error);
    mono_error_assert_ok (error);
    HANDLE_FUNCTION_RETURN ();
    }
  • void
    mono_unhandled_exception_checked (MonoObjectHandle exc, MonoError *error)
    {
    MONO_REQ_GC_UNSAFE_MODE;
    MonoDomain *current_domain = mono_domain_get ();
    MonoClass *klass = mono_handle_class (exc);
    /*
    * AppDomainUnloadedException don't behave like unhandled exceptions unless thrown from
    * a thread started in unmanaged world.
    * https://msdn.microsoft.com/en-us/library/system.appdomainunloadedexception(v=vs.110).aspx#Anchor_6
    */
    gboolean no_event = (klass == mono_defaults.threadabortexception_class);
    if (no_event)
    return;
    MONO_STATIC_POINTER_INIT (MonoClassField, field)
    static gboolean inited;
    if (!inited) {
    field = mono_class_get_field_from_name_full (mono_defaults.appcontext_class, "UnhandledException", NULL);
    inited = TRUE;
    }
    MONO_STATIC_POINTER_INIT_END (MonoClassField, field)
    if (!field)
    goto leave;
    MonoObject *delegate = NULL;
    MonoObjectHandle delegate_handle;
    MonoVTable *vt = mono_class_vtable_checked (mono_defaults.appcontext_class, error);
    goto_if_nok (error, leave);
    // TODO: use handles directly
    mono_field_static_get_value_checked (vt, field, &delegate, MONO_HANDLE_NEW (MonoString, NULL), error);
    goto_if_nok (error, leave);
    delegate_handle = MONO_HANDLE_NEW (MonoObject, delegate);
    if (MONO_HANDLE_IS_NULL (delegate_handle)) {
    mono_print_unhandled_exception_internal (MONO_HANDLE_RAW (exc)); // TODO: use handles
    } else {
    gpointer args [2];
    args [0] = current_domain->domain;
    args [1] = MONO_HANDLE_RAW (create_unhandled_exception_eventargs (exc, error));
    mono_error_assert_ok (error);
    mono_runtime_delegate_try_invoke_handle (delegate_handle, args, error);
    }
    leave:
    /* set exitcode if we will abort the process */
    mono_environment_exitcode_set (1);
    }

If the API just raises the event and nothing else, we may want to call it RaiseUnhandledExceptionEvent. From .NET Framework design guidelines:

DO use the term “raise” for events rather than “fire” or “trigger.”

When referring to events in documentation, use the phrase “an event was raised” instead of “an event was fired” or “an event was triggered.”

Also, there is prior art like RaisePropertyChanging, RaiseDeserializationEvent,

@rolfbjarne
Copy link
Member Author

Mark the API with [System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute]?

For iOS / Objective-C it doesn't really matter, because we call abort right after raising the unhandled exception event anyways:

https://github.com/dotnet/macios/blob/be2ec4d442e0b80edf380b6a99b39b6c843f5864/runtime/runtime.m#L1020-L1022

@rolfbjarne
Copy link
Member Author

Any handler set via ExceptionHandling.SetUnhandledExceptionHandler() would not be called

IMHO the naming here is unfortunate... just looking at the names, it's not clear at all why calling ExceptionHandling.ReportUnhandledException() wouldn't call handlers set with ExceptionHandling.SetUnhandledExceptionHandler().

There's no documentation for ExceptionHandling.SetUnhandledExceptionHandler (https://learn.microsoft.com/en-us/dotnet/api/system.runtime.exceptionservices.exceptionhandling.setunhandledexceptionhandler?view=net-10.0) - which is understandable since this is not released yet.

However, the xml docs don't make anything clearer either:

/// <summary>
/// Sets a handler for unhandled exceptions.
/// </summary>
/// <exception cref="ArgumentNullException">If handler is null</exception>
/// <exception cref="InvalidOperationException">If a handler is already set</exception>
public static void SetUnhandledExceptionHandler(Func<Exception, bool> handler)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime.InteropServices
Projects
Status: No status
Development

No branches or pull requests

9 participants