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

AndroidStripILAfterAOT causing "InvalidProgramException: Invalid IL code in Xunit.Assert:True(bool)" #103782

Closed
tranb3r opened this issue Jan 19, 2024 · 31 comments · Fixed by #103975
Assignees
Milestone

Comments

@tranb3r
Copy link

tranb3r commented Jan 19, 2024

Android application type

.NET Android (net7.0-android, net8.0-android, etc.)

Affected platform version

VS 2022 17.8.5

Description

When setting AndroidStripILAfterAOT=true in a test app, some tests are failing the first time we run them, but then they'll pass if we run them again.
The error is 100% reproducible when starting the app again.
Here is the error: InvalidProgramException: Invalid IL code in Xunit.Assert:True(bool): method body is empty.
No issue in Debug. No issue when removing AndroidStripILAfterAOT=true.
This is similar to #104156, but no BroadcastReceiver is involved here.

Steps to Reproduce

  1. Open repro in visual studio.
  2. Run app in Release on android emulator.
  3. Run all tests: one of the test fails.
  4. Run all tests again: both tests succeed.

Did you find any workaround?

Remove AndroidStripILAfterAOT=true.

Relevant log output

No response

@lambdageek
Copy link
Member

fyi @vitek-karas

@grendello
Copy link
Contributor

@fanyang-mono & @vitek-karas could you take a look into this? Thanks! :)

@fanyang-mono
Copy link
Member

I will work on this.

@tranb3r
Copy link
Author

tranb3r commented Feb 6, 2024

Any update?

@fanyang-mono
Copy link
Member

Hi @tranb3r, sorry for the delay. I tried building and running the app that you linked in this issue. I tried with .NET8 sdk and .NET8 service 1 sdk. I was not able to reproduce with either of them. Here are my steps:

  • Get .NET8 sdk
  • Install maui workload
  • Add <AndroidStripILAfterAOT>true</AndroidStripILAfterAOT> in Issues/MauiAppXunitInvalidILCode/MauiAppXunitInvalidILCode/MauiAppXunitInvalidILCode.csproj
  • Open an Android arm64 emulator
  • Build and runt the app from Issues/MauiAppXunitInvalidILCode/MauiAppXunitInvalidILCode using dotnet build -t:Run -f net8.0-android -p:AndroidSdkDirectory=/Users/yangfan/android-sdk -p:EmbedAssembliesIntoApk=true

Did I miss anything?

@tranb3r
Copy link
Author

tranb3r commented Feb 10, 2024

@fanyang-mono
I run my tests on an x64 android emulator. Maybe this is the difference?

@fanyang-mono
Copy link
Member

@tranb3r From AndroidStripILAfterAOT feature's perspective, it shouldn't make a difference. Could you share the apk file where the failure happened?

@tranb3r
Copy link
Author

tranb3r commented Feb 20, 2024

@fanyang-mono I think you're building for Debug instead of Release.

Here is a simple command to build and run the apk:

  • x86_64: dotnet build -t:Run -f net8.0-android -c Release -r android-x64
  • arm64-v8a: dotnet build -t:Run -f net8.0-android -c Release -r android-arm64

Here are the corresponding apks:

I've reproduced the error both on x86_64 simulator and arm64-v8a device.

@fanyang-mono
Copy link
Member

@tranb3r I was able to reproduce with the apk file that you provided. Will look into it.

@tranb3r
Copy link
Author

tranb3r commented Mar 4, 2024

@fanyang-mono Hi, any progress? Thanks

@fanyang-mono
Copy link
Member

@tranb3r I was caught up by some other projects. Will find some time to look into this this week. Sorry about the delay.

@tranb3r
Copy link
Author

tranb3r commented Mar 12, 2024

@fanyang-mono Sorry to ask again. Did you have the time to work on this issue? Thx.

@fanyang-mono
Copy link
Member

@tranb3r I got some time to working on it now. I was able to reproduce it with the app you sent to me. Now I need to understand why that method got trimmed. I would need to build it locally. The repo that you linked originally is not accessible anymore. Did you moved the project to another location? If so, could you please send me a new link?

@tranb3r
Copy link
Author

tranb3r commented Mar 12, 2024

The repo has not been moved.
Here is the link to the project: https://github.com/tranb3r/Issues/tree/main/MauiAppXunitInvalidILCode

@tranb3r
Copy link
Author

tranb3r commented Mar 15, 2024

@fanyang-mono Did you manage to access the repo and rebuild the test app?

@fanyang-mono
Copy link
Member

@fanyang-mono Did you manage to access the repo and rebuild the test app?

Yes, working on it.

@tranb3r
Copy link
Author

tranb3r commented Mar 26, 2024

Hi @fanyang-mono ,
Any progress with this issue? Is there anything I can do to help?

@tranb3r
Copy link
Author

tranb3r commented Apr 8, 2024

Hi @fanyang-mono
Sorry to ask again for an update... but now I'm begining to worry that this issue is never going to be fixed.

@tranb3r
Copy link
Author

tranb3r commented Apr 22, 2024

Hi @jonathanpeppers
Apologies for the tag, but I really need a fix for this issue. Maybe you can help resolve it?

@tranb3r
Copy link
Author

tranb3r commented May 22, 2024

This is still happening with net9-pre4

@charlesroddie
Copy link

AndroidStripILAfterAOT is an important stopgap before Android gets NativeAOT support. It looks like it won't get NativeAOT support in dotnet 9 and if this issue isn't fixed either, then it will be the only platform where IL is deployed to end-user systems.

@fanyang-mono
Copy link
Member

Sorry for the delay. I had to focus on other projects. I am able to focus on this issue now.

I did some investigation and found that

  1. Xunit.Assert:True(bool) was AOT compiled
  2. When AndroidStripILAfterAOT was enabled, the method body of the IL code of Xunit.Assert:True(bool) was removed. Because, it is expected to always use the AOT'ed version of it.
  3. When I enabled logging for Android by adb shell setprop debug.mono.log default,assembly,mono_log_level=debug,mono_log_mask=aot, I can see AOT: FOUND method Xunit.Assert:True (bool) in the log once. That means Mono runtime is able to find the AOT version of Xunit.Assert:True (bool).
  4. When running the tests on the second time, the error went away.

This seems to me that when running the test for the first time or invoking Xunit.Assert:True (bool) for the first time, mono runtime wanted to JIT it for some reason. I need to investigate more to figure out why.

@fanyang-mono
Copy link
Member

@tranb3r Do you have an iOS app which the same kind of testing, meaning a few tests within one assembly? If so, are they running fine?

@tranb3r
Copy link
Author

tranb3r commented Jun 19, 2024

@tranb3r Do you have an iOS app which the same kind of testing, meaning a few tests within one assembly? If so, are they running fine?

@fanyang-mono
I've added the net8.0-ios target to the repro.
I do not have any issue with it : tests are running fine.
But, is there actually an equivalent for AndroidStripILAfterAOT on iOS ?

@fanyang-mono
Copy link
Member

There isn't an equivalent for AndroidStripILAfterAOT on iOS, because it is not needed. For iOS, it uses full aot mode of mono, which means that mono AOT compile every methods. And all the IL code for all the methods are removed by default.

@lambdageek
Copy link
Member

Fun news - I believe I can get the same failure on iOS too https://github.com/lambdageek/maui-aot-race

The underlying issue is how mono_aot_get_method races with load_aot_module - one thread could attempt to read MonoImage:aot_module while the other thread is still loading the aot module.

@jonathanpeppers jonathanpeppers transferred this issue from dotnet/android Jun 20, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jun 20, 2024
Copy link
Contributor

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

@tipa
Copy link

tipa commented Jun 28, 2024

@fanyang-mono will this fix also resolve #104156?

@lambdageek
Copy link
Member

@tipa yea that looks related.


Our next steps here are to wait for this fix to flow to the Android and ios SDKs and to get some performance benchmarks. If there is negligible performance impact on non-racy code we will try to do a net8.0 backport.

@fanyang-mono
Copy link
Member

The fix to this issue is on track to be shipped as part of .NET9 Preview7. Additionally, I've opened an PR to backport this fix to .NET8.

@tranb3r
Copy link
Author

tranb3r commented Aug 14, 2024

I've just tested net9-pre7. The fix works on my repro project. Thx.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants