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

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

Closed
lauxjpn opened this issue Jan 8, 2022 · 4 comments · Fixed by #6953
Closed
Assignees
Labels
Area: App+Library Build Issues when building Library projects or Application projects.
Milestone

Comments

@lauxjpn
Copy link
Contributor

lauxjpn commented Jan 8, 2022

Android application type

Android for .NET (net6.0-android, etc.)

Affected platform version

.NET 6.0.100

Description

When registering an AppDomain.CurrentDomain.FirstChanceException handler, the handler works fine with non-trimmed builds.

With the default TrimMode=link option when using PublishTrimmed=true, the app crashes at runtime (presumable when the handler gets called for the first time).

I also tried the following, but the crash still happens at runtime:

<Target Name="ConfigureTrimming"
        BeforeTargets="PrepareForILLink">
    <ItemGroup>
        <ManagedAssemblyToLink Condition="'%(Filename)' == 'System.Runtime'">
            <IsTrimmable>false</IsTrimmable>
        </ManagedAssemblyToLink>
    </ItemGroup>
</Target>

I wanted to use a FirstChanceException handler to break into the code when an exception because of another trimmed method occurs, so that I could take a look at the call stack and the parameters of a System.Delegate.CreateDelegate(Type type, Type target, String method) call, so that I could investigate which target method got trimmed in #6612 (comment).

Unfortunately, since the TrimMode=link also trims away the FirstChanceException handler support, I can't use it for that purpose.

Steps to Reproduce

See description.

Did you find any workaround?

No response

Relevant log output

No response

@lauxjpn lauxjpn added Area: App+Library Build Issues when building Library projects or Application projects. needs-triage Issues that need to be assigned. labels Jan 8, 2022
@jonathanpeppers jonathanpeppers removed the needs-triage Issues that need to be assigned. label Apr 13, 2022
@jonathanpeppers jonathanpeppers added this to the .NET 6 milestone Apr 13, 2022
@jonathanpeppers
Copy link
Member

This example in a dotnet new android app works for me:

[Activity(Label = "@string/app_name", MainLauncher = true)]
public class MainActivity : Activity
{
    protected override void OnCreate(Bundle? savedInstanceState)
    {
        base.OnCreate(savedInstanceState);

        // Set our view from the "main" layout resource
        SetContentView(Resource.Layout.activity_main);

        AppDomain.CurrentDomain.FirstChanceException += (sender, e) =>
        {
            Console.WriteLine("First change exception!");
        };
    }

    public override void OnBackPressed() => Task.Factory.StartNew(() => throw new Exception("foo"));
}

However, if I do this, then FirstChanceException doesn't fire:

public override void OnBackPressed() => throw new Exception("foo");

@jonathanpeppers
Copy link
Member

I think this behavior is correct for Debug builds ^^

The problem is Release builds (that are trimmed) crash with:

04-18 11:47:56.344 31371 31456 E companyname.fo: * Assertion at /__w/1/s/src/mono/mono/metadata/object.c:4179, condition `ctor' not met

@steveisok do you have any ideas on which type I should check to see what is linked away?

@jonathanpeppers
Copy link
Member

@steveisok @akoeplinger here is a smaller example:

        AppDomain.CurrentDomain.FirstChanceException += (sender, e) =>
        {
            Console.WriteLine("First chance exception!");
        };

        try
        {
            throw new Exception("foo");
        }
        catch
        {
            Console.WriteLine("Exception handled!");
        }

I tested with adb shell setprop debug.mono.log default,timing,assembly,mono_log_level=debug,mono_log_mask=all

In a trimmed build (Release), this crashes without much info was to what is wrong:

04-19 15:01:49.456 24437 24437 D Mono    : Running class .cctor for System.AppDomain from 'System.Private.CoreLib.dll'
...
04-19 15:02:44.254 24437 24437 E companyname.fo: * Assertion at /__w/1/s/src/mono/mono/metadata/object.c:4179, condition `ctor' not met

adb.txt

So I was looking at:

https://github.com/dotnet/runtime/blob/6c7ce85e149eaaf4cc452a7008754d5c799db938/src/mono/mono/metadata/object.c#L94

What would preserve the ctor of this type?

image

image

I think ILSpy is showing this type has no ctor.

akoeplinger added a commit to akoeplinger/runtime that referenced this issue Apr 19, 2022
The runtime is creating the exception so we need to preserve it.
See dotnet/android#6626
@akoeplinger
Copy link
Member

akoeplinger commented Apr 19, 2022

Thanks, I opened dotnet/runtime#68235 to fix this. In the meantime you can workaround the issue by adding this to a custom linker configuration:

<assembly fullname="System.Private.CoreLib">
    <type fullname="System.Runtime.ExceptionServices.FirstChanceExceptionEventArgs">
        <method signature="System.Void .ctor(System.Exception)" />
    </type>
</assembly>

github-actions bot pushed a commit to dotnet/runtime that referenced this issue Apr 20, 2022
The runtime is creating the exception so we need to preserve it.
See dotnet/android#6626
akoeplinger added a commit to dotnet/runtime that referenced this issue Apr 20, 2022
The runtime is creating the exception so we need to preserve it.
See dotnet/android#6626

While looking at this I noticed that `FirstChanceExceptionEventArgs` is in ExceptionNotification.cs for some reason, fixed the filename to match the type name.

Use DynamicDependency for preserving `FirstChanceExceptionEventArgs` and `UnhandledExceptionEventArgs`
directhex pushed a commit to directhex/runtime that referenced this issue Apr 21, 2022
The runtime is creating the exception so we need to preserve it.
See dotnet/android#6626

While looking at this I noticed that `FirstChanceExceptionEventArgs` is in ExceptionNotification.cs for some reason, fixed the filename to match the type name.

Use DynamicDependency for preserving `FirstChanceExceptionEventArgs` and `UnhandledExceptionEventArgs`
jonpryor pushed a commit that referenced this issue 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
jonathanpeppers pushed a commit that referenced this issue 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
rolfbjarne pushed a commit to xamarin/xamarin-macios that referenced this issue Apr 25, 2022
* Workaround FirstChanceExceptionEventArgs being trimmed

See dotnet/android#6626

* Don't use ILLink descriptor on Microsoft.macOS.dll

It is using CoreCLR which doesn't run into the issue.
carlossanlop pushed a commit to dotnet/runtime that referenced this issue May 5, 2022
)

* [mono] Preserve FirstChanceExceptionEventArgs ctor

The runtime is creating the exception so we need to preserve it.
See dotnet/android#6626

* PR feedback: use DynamicDependencyAttribute instead

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
@ghost ghost locked as resolved and limited conversation to collaborators Jun 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: App+Library Build Issues when building Library projects or Application projects.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants