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

[release/6.0-preview7] [debugger] Export mono_debugger_agent_unhandled_exception to avoid usage of Debugger.Mono_UnhandledException #56071

Merged

Conversation

thaystg
Copy link
Member

@thaystg thaystg commented Jul 21, 2021

Backport of #55963 to release/6.0-preview7

/cc @thaystg

Customer Impact

Xamarin android will be able to call this function and avoid calling Debugger.Mono_UnhandledException as this does not exist on net6.

Testing

Risk

Zero risk, it's just exporting and existing function.

…eam will be able to use mono_debugger_agent_unhandled_exception, and do not use Debugger.Mono_UnhandledException(as it does not exist on net6).
@ghost
Copy link

ghost commented Jul 21, 2021

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #55963 to release/6.0-preview7

/cc @thaystg

Customer Impact
Xamarin android will be able to call this function and avoid calling Debugger.Mono_UnhandledException as this does not exist on net6.

Testing
Risk
Zero risk, it's just exporting and existing function.

Author: thaystg
Assignees: -
Labels:

area-Debugger-mono

Milestone: -

@marek-safar marek-safar added the Servicing-consider Issue for next servicing release review label Jul 21, 2021
@mmitche mmitche added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jul 21, 2021
@thaystg thaystg requested a review from marek-safar July 21, 2021 19:05
@Anipik Anipik merged commit 743bd89 into release/6.0-preview7 Jul 21, 2021
jonpryor pushed a commit to dotnet/android that referenced this pull request Jul 23, 2021
[One .NET] Use Mono embedding API for exception debugger notification (#6106)

Context: dotnet/runtime#56071
Context: #4877
Context: #4927 (comment)
Context: #4927 (comment)

Context: xamarin/monodroid@3e9de5a
Context: xamarin/monodroid@b0f8597
Context: xamarin/monodroid@12a012e

What should happen when an exception is thrown and a debugger is
attached?

This is in fact a loaded question: there's what Xamarin.Android
(Legacy) *has* done, vs. what .NET 6 Android for .NET does, vs.
what "should" happen.

What "should" happen is easiest:

 1. We should behave like a "normal" Desktop .NET app when a debugger
    is attached, AND

 2. We shouldn't corrupt JVM state.

Unfortunately, (1)+(2) is currently not possible, in part because
Java doesn't have an equivalent to Windows' [two attempt][0] debugger
notification infrastructure.
See #4877 for details.

What Legacy Xamarin.Android does is also detailed in
#4877, and relies on the
`Debugger.Mono_UnhandledException()` method in order to alert an
attached debugger that there is an exception to show to the user.

However, `Debugger.Mono_UnhandledException()` never made it to the
`dotnet/runtime` repo.  It never existed there.

Thus, what .NET 6 Android for .NET *currently* does is…*nothing*.
If an exception is thrown and a debugger is attached, the debugger
is *not* notified.  Eventually you'll get an unhandled exception,
long after it was originally thrown; see commit c1a2ee7.

PR dotnet/runtime#56071 added a new
`mono_debugger_agent_unhandled_exception()` Mono embedding API which
is equivalent to `Debugger.Mono_UnhandledException()` for use with
.NET 6 + MonoVM.

Update `src/Mono.Android` and `src/monodroid` so that
`mono_debugger_agent_unhandled_exception()` is used to alert the
debugger that an exception has been thrown at a JNI boundary.
This should allow .NET 6 + Android to have equivalent exception
handling semantics as legacy Xamarin.Android.

[0]: https://docs.microsoft.com/en-us/windows/win32/debug/debugger-exception-handling
jonpryor pushed a commit to dotnet/android that referenced this pull request Jul 23, 2021
[One .NET] Use Mono embedding API for exception debugger notification (#6106)

Context: dotnet/runtime#56071
Context: #4877
Context: #4927 (comment)
Context: #4927 (comment)

Context: xamarin/monodroid@3e9de5a
Context: xamarin/monodroid@b0f8597
Context: xamarin/monodroid@12a012e

What should happen when an exception is thrown and a debugger is
attached?

This is in fact a loaded question: there's what Xamarin.Android
(Legacy) *has* done, vs. what .NET 6 Android for .NET does, vs.
what "should" happen.

What "should" happen is easiest:

 1. We should behave like a "normal" Desktop .NET app when a debugger
    is attached, AND

 2. We shouldn't corrupt JVM state.

Unfortunately, (1)+(2) is currently not possible, in part because
Java doesn't have an equivalent to Windows' [two attempt][0] debugger
notification infrastructure.
See #4877 for details.

What Legacy Xamarin.Android does is also detailed in
#4877, and relies on the
`Debugger.Mono_UnhandledException()` method in order to alert an
attached debugger that there is an exception to show to the user.

However, `Debugger.Mono_UnhandledException()` never made it to the
`dotnet/runtime` repo.  It never existed there.

Thus, what .NET 6 Android for .NET *currently* does is…*nothing*.
If an exception is thrown and a debugger is attached, the debugger
is *not* notified.  Eventually you'll get an unhandled exception,
long after it was originally thrown; see commit c1a2ee7.

PR dotnet/runtime#56071 added a new
`mono_debugger_agent_unhandled_exception()` Mono embedding API which
is equivalent to `Debugger.Mono_UnhandledException()` for use with
.NET 6 + MonoVM.

Update `src/Mono.Android` and `src/monodroid` so that
`mono_debugger_agent_unhandled_exception()` is used to alert the
debugger that an exception has been thrown at a JNI boundary.
This should allow .NET 6 + Android to have equivalent exception
handling semantics as legacy Xamarin.Android.

[0]: https://docs.microsoft.com/en-us/windows/win32/debug/debugger-exception-handling
@akoeplinger akoeplinger deleted the backport/pr-55963-to-release/6.0-preview7 branch July 26, 2021 10:15
@ghost ghost locked as resolved and limited conversation to collaborators Aug 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Debugger-mono Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants