[mono] Fix the race condition issue of aot_module loading#103975
[mono] Fix the race condition issue of aot_module loading#103975fanyang-mono merged 4 commits intodotnet:mainfrom
Conversation
fanyang-mono
requested review from
BrzVlad,
kotlarmilos,
lambdageek,
lateralusX and
steveisok
as code owners
ghost
added
the
src/mono/mono/mini/aot-runtime.c
Outdated
| while (!(image->aot_module) && (count < 10)) { | ||
| g_usleep (100); | ||
| count++; |
There was a problem hiding this comment.
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)
lambdageek
left a comment
lambdageek
left a comment
There was a problem hiding this comment.
Need to also set the module to AOT_MODULE_NOT_FOUND here:
runtime/src/mono/mono/mini/aot-runtime.c
Lines 2086 to 2096 in c5e8f83
|
/azp run runtime-ioslike |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-android |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-android |
|
/azp run runtime-ioslike |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
steveisok
left a comment
steveisok
left a comment
There was a problem hiding this comment.
The android and ios/tvos failures are known. LGTM!
|
Is this issue going to be merged back on .net 8.0? I'm having an issue when IL stripping assemblies on Android |
|
@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. |
4 participants
Fixes #103782