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

[mono] Fix the race condition issue of aot_module loading #103975

Merged
merged 4 commits into from
Jun 28, 2024

Conversation

fanyang-mono
Copy link
Member

Fixes #103782

Comment on lines 4940 to 4942
while (!(image->aot_module) && (count < 10)) {
g_usleep (100);
count++;
Copy link
Member

@lambdageek lambdageek Jun 25, 2024

Choose a reason for hiding this comment

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

This isn't 100% correct (if the current thread is holding the loader lock, while another thread is trying to load some assemblies while loading hte AOT module, the other thread won't make progress). But it should handle 80% of the cases (where we just had a simple race, rather than a potential deadlock).

We decided this is ok.

The correct solution would be to make load_aot_module safe to call from multiple threads at the same time using a lock-free "create and publish" strategy. But that will be a much harder rewrite because load_aot_module has global effects (on the JIT info tables, for example)

Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

Need to also set the module to AOT_MODULE_NOT_FOUND here:

if (!sofile) {
// Maybe do these on more platforms ?
#ifndef HOST_WASM
if (mono_aot_only && !mono_use_interpreter && table_info_get_rows (&assembly->image->tables [MONO_TABLE_METHOD])) {
aot_name = g_strdup_printf ("%s%s", assembly->image->name, MONO_SOLIB_EXT);
g_error ("Failed to load AOT module '%s' ('%s') in aot-only mode.\n", aot_name, assembly->image->name);
g_free (aot_name);
}
#endif
return;
}

@fanyang-mono
Copy link
Member Author

/azp run runtime-ioslike

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@fanyang-mono
Copy link
Member Author

/azp run runtime-android

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@fanyang-mono
Copy link
Member Author

/azp run runtime-android

@fanyang-mono
Copy link
Member Author

/azp run runtime-ioslike

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@steveisok steveisok left a comment

Choose a reason for hiding this comment

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

The android and ios/tvos failures are known. LGTM!

@gabriel-kozma
Copy link

Is this issue going to be merged back on .net 8.0? I'm having an issue when IL stripping assemblies on Android

@fanyang-mono
Copy link
Member Author

fanyang-mono commented Jul 15, 2024

@gabriel-kozma Yes, I've opened an PR to backport this fix to .NET8. However, this fix is on track to be shipped as part of .NET9 Preview7 as well.

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

Successfully merging this pull request may close these issues.

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