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

Marshalled managed exceptions should be considered unhandled exceptions by the debugger #15037

Open
Tracked by #7176
rolfbjarne opened this issue May 16, 2022 · 7 comments
Labels
enhancement The issue or pull request is an enhancement
Milestone

Comments

@rolfbjarne
Copy link
Member

rolfbjarne commented May 16, 2022

The debugger should consider (at least by default) marshalled managed exceptions as unhandled exceptions, and break.

This has become more important with .NET, where we've enabled exception marshalling by default.

A managed exception may now do the following:

  • For iOS and tvOS: get converted into an Objective-C exception, which may end up aborting the process (without the debugger breaking).
  • For Mac Catalyst and macOS: get converted into an Objective-C exception, which AppKit swallows and goes on its merry way (without the debugger breaking).

Neither are good scenarios, so we should look into what needs to happen to make the debugger break when an exception is marshalled (it's possible runtime changes will be needed).

Ref: dotnet/maui#7176

@rolfbjarne
Copy link
Member Author

I wonder if we can just call mono_debugger_agent_unhandled_exception like Android does:

https://github.com/xamarin/xamarin-android/blob/1d24421106f5a07b7c150c4390e7d722fe4e618d/src/native/monodroid/monodroid-glue.cc#L837

@StephaneDelcroix
Copy link
Member

hitting the same issue when executing invalid IL. no exception reported, but a crash logged in Console.app

@rolfbjarne
Copy link
Member Author

Calling mono_debugger_agent_unhandled_exception helps, now exceptions in event handlers in Mac Catalyst aren't just silently ignored:

Screenshot 2024-12-09 at 16 25 09

However, it seems the app stops too late to show the correct location, the stack has already been unwound a bit (native stacktrace). I believe this is because the debugger and/or runtime doesn't consider that exceptions that happen as a result of calling mono_runtime_invoke aren't handled.

@thaystg is this something you can give advice about? It seems that doing something like this:

static void DoSomething () { throw new Exception ("exception!"); }
MonoMethod *doSomethingMethod = ...;
void *arg_ptrs [1];
MonoObject *exception = NULL;
MonoObject *ret;

ret = mono_runtime_invoke (doSomethingMethod, NULL, arg_ptrs, &exception);

won't make the debugger in VSCode stop when the exception is thrown.

@rolfbjarne
Copy link
Member Author

Seems like this is a variation of dotnet/android#4927 (comment)

rolfbjarne added a commit that referenced this issue Dec 9, 2024
…ion is marshalled.

This means the debugger will be notified of all marshalled exceptions, and
consider them unhandled (and break the debugged app into the debugger).

There's no corresponding change for CoreCLR, because we have no configuration
where we can attach a debugger to a CoreCLR-based app (VSCode doesn't support
macOS, and NativeAOT doesn't support debugging).

Partial fix for #15037.
Fixes dotnet/maui#7176.
Ref: dotnet/android#6106
@jonpryor
Copy link
Member

Note: Debugger.BreakForUserUnhandledException() only works when using the "ICorDebug" debugger backend, not mono/debugger-libs.

@rolfbjarne
Copy link
Member Author

@jonpryor I don't think that API will get us (nor you in fact) the behavior we want.

We want the debugger to break when the exception is thrown:

  • if it's not handled,
  • or only handled in our managed<->native/java trampolines.

That attribute will prevent the debugger from breaking when the exception is thrown, and then we call some API later on to let the debugger know "oh, sorry, please treat this exception as unhandled after all". However, at that point, the stack has already been unwound (somewhat at least), which means the debugging experience is subpar.

The documentation for DebuggerDisableUserUnhandledExceptionsAttribute says:

"the debugger won't break on user-unhandled exceptions when the exception is caught by a method with this attribute" - we want the opposite of this, something like: "the debugger will break on user-unhandled exceptions even if the exception is caught by a method with this attribute" (or more accurately "the debugger will not consider any exception handlers in this method when determining whether an exception is handled or not")

Additionally, we don't have any managed method to attach any attributes or run custom code in, because use the embedding API - mono_runtime_invoke - so we'd need a solution that works for that as well.

rolfbjarne added a commit that referenced this issue Dec 13, 2024
…ion is marshalled. (#21778)

This means the debugger will be notified of all marshalled exceptions, and
consider them unhandled (and break the debugged app into the debugger).

There's no corresponding change for CoreCLR, because we have no configuration
where we can attach a debugger to a CoreCLR-based app (VSCode doesn't support
macOS, and NativeAOT doesn't support debugging).

Partial fix for #15037.
Fixes dotnet/maui#7176.
Ref: dotnet/android#6106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue or pull request is an enhancement
Projects
None yet
Development

No branches or pull requests

3 participants