Skip to content

[Mono.Android] fix crash on startup with EnableLLVM #7188

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 3, 2022
Merged

[Mono.Android] fix crash on startup with EnableLLVM #7188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions build-tools/automation/azure-pipelines.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ stages:
testName: Mono.Android_Tests-Aot
project: tests/Mono.Android-Tests/Mono.Android-Tests.csproj
testResultsFiles: TestResult-Mono.Android_Tests-$(ApkTestConfiguration)-Aot.xml
extraBuildArgs: /p:AotAssemblies=True /p:EnableLlvm=True
extraBuildArgs: /p:AotAssemblies=True /p:EnableLLVM=True
artifactSource: bin/Test$(ApkTestConfiguration)/Mono.Android_Tests-Signed.apk
artifactFolder: AotLlvm

Expand Down Expand Up @@ -691,13 +691,15 @@ stages:

- template: yaml-templates/apk-instrumentation.yaml
parameters:
# TODO: disable LLVM test, see: https://github.com/dotnet/runtime/issues/68914
# TODO: disable LLVM test, see:
# https://github.com/dotnet/runtime/issues/68914
# https://github.com/dotnet/runtime/issues/73304
condition: false
Comment on lines -694 to 697
Copy link
Member Author

@jonathanpeppers jonathanpeppers Aug 3, 2022

Choose a reason for hiding this comment

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

I kept these disabled because of: dotnet/runtime#73304

I think we should merge this with the tests disabled, because it does get things further.

configuration: $(XA.Build.Configuration)
testName: Mono.Android.NET_Tests-AotLlvm
project: tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/Mono.Android.NET-Tests.csproj
testResultsFiles: TestResult-Mono.Android.NET_Tests-$(XA.Build.Configuration)AotLlvm.xml
extraBuildArgs: -p:TestsFlavor=AotLlvm -p:EnableLlvm=true
extraBuildArgs: -p:TestsFlavor=AotLlvm -p:EnableLLVM=true -p:AndroidEnableProfiledAot=false
Copy link
Member

@fanyang-mono fanyang-mono Aug 3, 2022

Choose a reason for hiding this comment

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

Is it necessary to disable profiled AOT?

Copy link
Member Author

@jonathanpeppers jonathanpeppers Aug 3, 2022

Choose a reason for hiding this comment

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

This one was: dotnet/runtime#73003

Using Profiled AOT and LLVM at the same time can hit:

[ERROR] FATAL UNHANDLED EXCEPTION: System.PlatformNotSupportedException: Arg_PlatformNotSupported
at System.Text.ASCIIUtility.GetIndexOfFirstNonAsciiByte_Intrinsified(Byte* , UIntPtr )
at System.Text.Unicode.Utf8Utility.GetPointerToFirstInvalidByte(Byte* , Int32 , Int32& , Int32& )
at System.Text.UTF8Encoding.GetCharCount(Byte* , Int32 )
at System.String.CreateStringFromEncoding(Byte* , Int32 , Encoding )
at System.Text.Encoding.GetString(Byte* , Int32 )
at System.String.CreateStringForSByteConstructor(Byte* , Int32 )
at System.String.Ctor(SByte* value)
at Java.Interop.TypeManager.GetClassName(IntPtr )
at Android.Runtime.JNIEnv.RegisterJniNatives(IntPtr , Int32 , IntPtr , IntPtr , Int32 )

artifactSource: bin/Test$(XA.Build.Configuration)/$(DotNetTargetFramework)-android/Mono.Android.NET_Tests-Signed.aab
artifactFolder: $(DotNetTargetFramework)-AotLlvm
useDotNet: true
Expand Down
9 changes: 8 additions & 1 deletion src/Mono.Android/Android.Runtime/JNIEnv.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,17 @@ internal static unsafe void Initialize (JnienvInitializeArgs* args)
}

#if !MONOANDROID1_0
SynchronizationContext.SetSynchronizationContext (Android.App.Application.SynchronizationContext);
SetSynchronizationContext ();
#endif
}

#if !MONOANDROID1_0
// NOTE: prevents Android.App.Application static ctor from running
[MethodImpl (MethodImplOptions.NoInlining)]
static void SetSynchronizationContext () =>
SynchronizationContext.SetSynchronizationContext (Android.App.Application.SynchronizationContext);
#endif

internal static void Exit ()
{
/* Manually dispose surfaced objects and close the current JniEnvironment to
Expand Down
4 changes: 2 additions & 2 deletions tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/Mono.Android.NET-Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@
<AndroidUseNegotiateAuthentication>true</AndroidUseNegotiateAuthentication>
<!--
TODO: Fix excluded tests
For AOT, InetAccess excluded due to: https://github.com/dotnet/runtime/issues/56315
For $(EnableLLVM), InetAccess excluded due to: https://github.com/dotnet/runtime/issues/56315
-->
<ExcludeCategories>DotNetIgnore</ExcludeCategories>
<ExcludeCategories Condition=" '$(RunAOTCompilation)' == 'true' ">$(ExcludeCategories):InetAccess</ExcludeCategories>
<ExcludeCategories Condition=" '$(EnableLLVM)' == 'true' ">$(ExcludeCategories):InetAccess</ExcludeCategories>
Copy link
Member Author

@jonathanpeppers jonathanpeppers Aug 3, 2022

Choose a reason for hiding this comment

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

Since 0084162, this Condition wasn't actually doing anything. We weren't passing in RunAOTCompilation=true because that is the default value for Release builds.

InetAccess seems to crash the tests when EnableLLVM=true, however... So let's keep the condition for that case.

Copy link
Member

@fanyang-mono fanyang-mono Aug 3, 2022

Choose a reason for hiding this comment

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

Can you please also open an issue for the InetAccess test crashes with LLVM enabled?

<ExcludeCategories Condition=" '$(UseInterpreter)' == 'true' ">$(ExcludeCategories):IgnoreInterpreter</ExcludeCategories>
</PropertyGroup>

Expand Down