Skip to content

Conversation

@akoeplinger
Copy link
Member

@akoeplinger akoeplinger commented Apr 21, 2022

Fixes: #6626

Comment on lines +4 to +6
<assembly fullname="System.Private.CoreLib">
<type fullname="System.Runtime.ExceptionServices.FirstChanceExceptionEventArgs">
<method signature="System.Void .ctor(System.Exception)" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akoeplinger: why doesn't the linker already preserve this method?

At a glance, it's because the only managed code that creates an instance of FirstChanceExceptionEventArgs is in NativeAOT: https://github.com/dotnet/runtime/blob/2389815a1bc810b5b3c39586b570cbaae2f0f3a5/src/coreclr/nativeaot/System.Private.CoreLib/src/System/AppContext.CoreRT.cs#L14

Oddly, the only native code I see which uses FirstChanceExceptionEventArgs is CoreCLR, e.g.

I can't easily find any Mono dotnet/runtime code which mentions FirstChanceExceptionEventArgs.

Thus it looks like we need the PreserveList because it's native code which is creating an instance of the type, but it would be nice to know which mono code is creating this instance. Or does Mono also use excep.cpp?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it's that object.c code from mono, nothing in the BCL references the ctor.

@jonpryor
Copy link
Contributor

Given that dotnet/runtime#68235 has been merged and backported, do we really need this fix?

@akoeplinger
Copy link
Member Author

@jonpryor the backport won't be merged in time for the 6.0 release that you're shipping with, so this is to fix the issue until that backport reaches you.

@jonpryor
Copy link
Contributor

[Microsoft.Android.Sdk.ILLink] Preserve FirstChanceExceptionEventArgs

Fixes: https://github.com/xamarin/xamarin-android/issues/6626

Context: https://github.com/dotnet/runtime/pull/68235

If you create a .NET 6 app whichs ubscribes to the
[`AppDomain.FirstChanceException`][0] event:

	partial class MainActivity {
	    protected override void OnCreate (Bundle? savedInstanceState)
	    {
	        AppDomain.CurrentDomain.FirstChanceException += (o, e) => {
	            Console.WriteLine ("First chance exception!");
	       };
	    }
	}

and has a way to cause an exception to be thrown:

	partial class MainActivity {
	    public override void OnBackPressed() => throw new Exception ("wee?");
	}

and is built in Release configuration with Trimming enabled,

then the app will crash when the exception is thrown (in the above
case, by launching the app and then pressing the Back button):

	E companyname.fo: * Assertion at /__w/1/s/src/mono/mono/metadata/object.c:4179, condition `ctor' not met

The cause of the crash is that the linker removes the
[`FirstChanceExceptionEventArgs(Exception)`][1] constructor.

This was fixed in dotnet/runtime#68235 by using appropriate
`[DynamicDependency]` attributes.  However, this runtime fix is
unlikely to be released before our next RC.

Add a workaround by updating
`src/Microsoft.Android.Sdk.ILLink/PreserveLists/System.Private.CoreLib.xml`
so that the linker won't remove the
`FirstChanceExceptionEventArgs(Exception)` constructor.

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.appdomain.firstchanceexception?view=net-6.0
[1]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.exceptionservices.firstchanceexceptioneventargs.-ctor?view=net-6.0

@jonpryor jonpryor merged commit 3d533b4 into dotnet:main Apr 21, 2022
jonathanpeppers pushed a commit that referenced this pull request Apr 21, 2022
)

Fixes: #6626

Context: dotnet/runtime#68235

If you create a .NET 6 app whichs ubscribes to the
[`AppDomain.FirstChanceException`][0] event:

	partial class MainActivity {
	    protected override void OnCreate (Bundle? savedInstanceState)
	    {
	        AppDomain.CurrentDomain.FirstChanceException += (o, e) => {
	            Console.WriteLine ("First chance exception!");
	       };
	    }
	}

and has a way to cause an exception to be thrown:

	partial class MainActivity {
	    public override void OnBackPressed() => throw new Exception ("wee?");
	}

and is built in Release configuration with Trimming enabled,

then the app will crash when the exception is thrown (in the above
case, by launching the app and then pressing the Back button):

	E companyname.fo: * Assertion at /__w/1/s/src/mono/mono/metadata/object.c:4179, condition `ctor' not met

The cause of the crash is that the linker removes the
[`FirstChanceExceptionEventArgs(Exception)`][1] constructor.

This was fixed in dotnet/runtime#68235 by using appropriate
`[DynamicDependency]` attributes.  However, this runtime fix is
unlikely to be released before our next RC.

Add a workaround by updating
`src/Microsoft.Android.Sdk.ILLink/PreserveLists/System.Private.CoreLib.xml`
so that the linker won't remove the
`FirstChanceExceptionEventArgs(Exception)` constructor.

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.appdomain.firstchanceexception?view=net-6.0
[1]: https://docs.microsoft.com/en-us/dotnet/api/system.runtime.exceptionservices.firstchanceexceptioneventargs.-ctor?view=net-6.0
@akoeplinger akoeplinger deleted the workaround-firstchanceexception branch April 22, 2022 18:28
akoeplinger added a commit that referenced this pull request Jun 22, 2022
jonpryor pushed a commit that referenced this pull request Jul 19, 2022
jonpryor pushed a commit that referenced this pull request Jul 20, 2022
…Args (#6953)" (#7118)

Context: dotnet/runtime@a6202a8
Context: dotnet/runtime@ed41478

Commit 3d533b4 was introduced to workaround an issue fixed in
dotnet/runtime@a6202a8c.  This fix is now available in the stable
.NET 6.0 series via dotnet/runtime@ed41478a, and thus our workaround
is no longer necessary.

Revert commit 3d533b4.
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using the default TrimMode=link breaks FirstChanceException handler support and crashes app

3 participants