-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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] Mark AOT modules unusable if no AOT version is found #106026
[mono] Mark AOT modules unusable if no AOT version is found #106026
Conversation
Tagging subscribers to 'arch-android': @vitek-karas, @simonrozsival, @steveisok, @akoeplinger |
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
one minor suggestion
Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
The |
Fixes: #9081 Context: dotnet/runtime#104397 Context: dotnet/runtime#106026 Context: #9081 (comment) Context: c227042 ("Compatibility is fun") Consider a P/Invoke method declaration: [DllImport("libSkiaSharp")] static extern void gr_backendrendertarget_delete(IntPtr rendertarget); Historically, when attempting to resolve this method, Mono would try loading the following native libraries: * `libSkiaSharp` (original name) * `libSkiaSharp.so` (add `.so` suffix) * `liblibSkiaSharp` (add `lib` prefix) * `liblibSkiaSharp.so` (`lib` prefix + `.so` suffix) .NET for Android would further permute these names, *removing* the `lib` prefix, for attempted compatibility in case there is a P/Invoke into `"SkiaSharp"`. The unfortunate occasional result would be an *ambiguity*: when told to resolve "SkiaSharp", what should we return? The information for `libSkiaSharp.so`, or for the *AOT'd image* of the assembly `SkiaSharp.dll`, by way of `libaot-SkiaSharp.dll.so`? %struct.DSOCacheEntry { i64 u0x12e73d483788709d, ; from name: SkiaSharp.so i64 u0x3cb282562b838c95, ; uint64_t real_name_hash i1 false, ; bool ignore ptr @.DSOCacheEntry.23_name, ; name: libaot-SkiaSharp.dll.so ptr null; void* handle }, ; 71 %struct.DSOCacheEntry { i64 u0x12e73d483788709d, ; from name: SkiaSharp.so i64 u0x43db119dcc3147fa, ; uint64_t real_name_hash i1 false, ; bool ignore ptr @.DSOCacheEntry.7_name, ; name: libSkiaSharp.so ptr null; void* handle }, ; 72 If we return the wrong thing, then the app may crash or otherwise behave incorrectly. Fix this by: * Splitting up the DSO cache into AOT-related `.so` files and everything else. * Updating `PinvokeOverride::load_library_symbol()` so that the AOT files are *not* consulted when resolving P/Invoke libraries. * Updating `MonodroidDl::monodroid_dlopen()` -- which is called by MonoVM via `mono_dl_fallback_register()` -- so that the AOT files *are* consulted *first* when resolving AOT images. When dotnet/runtime#104397 is fixed, it will make the AOT side of the split more efficient as we won't have to permute the shared library name as many times as now.
Description
On Android, we might hit a collision where we instead of
<libName>.dll.so
try to load<libName>.so
. By detecting this issue early we can mark the library as AOT unusable. We do this by checking if we were able to get the AOT file version and if not, we mark the.so
file as unusable.Full issue description #104397.
This is a fix from #104397 (comment). cc: @lambdageek
Verification
Tried locally build runtime with Xamarin Android app previously crashing due to SkiaSharp load. The crash wasn't present with this PR and the app started up correctly.
Previous behavior:
This PR: