Skip to content

Fix crash on aot unloading#106502

Merged
Repiteo merged 1 commit into
godotengine:masterfrom
Lange-Studios:fix-aot-crash
May 19, 2025
Merged

Fix crash on aot unloading#106502
Repiteo merged 1 commit into
godotengine:masterfrom
Lange-Studios:fix-aot-crash

Conversation

@TCROC
Copy link
Copy Markdown
Contributor

@TCROC TCROC commented May 16, 2025

We are currently debugging why AOT build are crashing on ios devices. In doing so, we tried to reproduce it on our Linux devices. I'm unable to reproduce the ios crash on linux BUT I did discover a new crash on Linux. I expect this issue applies to all platforms when exiting the application as well. Anyways, the segfault is due to attempting to unload the .net aot dll on program exit. .NET does not support this for aot builds. Here's the issue confirming as such on the .net side:

dotnet/runtime#79348

The fix is simple. Don't unload aot dll's and just let them die with the process on program exit :)

@TCROC TCROC requested a review from a team as a code owner May 16, 2025 19:22
@TCROC
Copy link
Copy Markdown
Contributor Author

TCROC commented May 16, 2025

I don't think that error is anything to do with this pr. Someone correct me if I'm wrong tho and I'll fix it

@raulsntos
Copy link
Copy Markdown
Member

This is correct, NativeAOT does not support unloading. I'm not sure why we're reusing the hostfxr_dll_handle, this looks confusing to me. We can probably just avoid assigning it, and then we don't need the is_aot boolean. Because I would've assumed hostfxr_dll_handle to be null when using NativeAOT.

I don't think that error is anything to do with this pr.

Yes, the CI errors are unrelated to this PR. I see someone already restarted the CI check.

@raulsntos raulsntos added this to the 4.5 milestone May 16, 2025
@TCROC
Copy link
Copy Markdown
Contributor Author

TCROC commented May 16, 2025

Sounds good! I'll go update it :)

@TCROC
Copy link
Copy Markdown
Contributor Author

TCROC commented May 16, 2025

@raulsntos Fixed per suggestion! :) At least I think I did. Is that the idiomatic way to discard the result of a variable that would have normally been assigned to? Or is there a different way that is normally taken in the godot repo?

Copy link
Copy Markdown
Member

@raulsntos raulsntos left a comment

Choose a reason for hiding this comment

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

Thanks. I think it's fine, but we could also remove the argument in the try_load_native_aot_library function since this is the only place where we call it.

@Repiteo Repiteo merged commit d57050c into godotengine:master May 19, 2025
20 checks passed
@Repiteo
Copy link
Copy Markdown
Contributor

Repiteo commented May 19, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants