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

.NET 9: Android application crashes in release mode with Skiasharp #104397

Open
daltzctr opened this issue Jul 3, 2024 · 5 comments
Open

.NET 9: Android application crashes in release mode with Skiasharp #104397

daltzctr opened this issue Jul 3, 2024 · 5 comments

Comments

@daltzctr
Copy link

daltzctr commented Jul 3, 2024

Description

I was directed to submit an issue here by the dotnet android development team. I upgraded my MAUI application to .NET 9 Preview 5 and have occurred release mode crashes whenever initializing a view that contains Skiasharp.

Reproduction Steps

  1. Clone https://github.com/daltzctr/maui-dotnet9-crash (ensure you are using .NET 9)
  2. Run in release mode
  3. Observe crash

Expected behavior

It does not crash

Actual behavior

It crashes

Regression?

Yes. This worked in .NET 8

Known Workarounds

<RunAOTCompilation>False</RunAOTCompilation>

Configuration

.NET SDK:
 Version:           9.0.100-preview.5.24307.3
 Commit:            35b2c21ea6
 Workload version:  9.0.100-manifests.949230c4
 MSBuild version:   17.11.0-preview-24279-02+b963c24ef

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22631
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\9.0.100-preview.5.24307.3\

.NET workloads installed:
Configured to use loose manifests when installing new manifests.
 [android]
   Installation Source: SDK 9.0.100-preview.5, VS 17.10.35004.147
   Manifest Version:    34.99.0-preview.5.308/9.0.100-preview.5
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\9.0.100-preview.5\microsoft.net.sdk.android\34.99.0-preview.5.308\WorkloadManifest.json
   Install Type:              Msi

 [ios]
   Installation Source: SDK 9.0.100-preview.5, VS 17.10.35004.147
   Manifest Version:    17.2.9639-net9-p5/9.0.100-preview.5
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\9.0.100-preview.5\microsoft.net.sdk.ios\17.2.9639-net9-p5\WorkloadManifest.json
   Install Type:              Msi

 [maccatalyst]
   Installation Source: SDK 9.0.100-preview.5, VS 17.10.35004.147
   Manifest Version:    17.2.9639-net9-p5/9.0.100-preview.5
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\9.0.100-preview.5\microsoft.net.sdk.maccatalyst\17.2.9639-net9-p5\WorkloadManifest.json
   Install Type:              Msi

 [maui-android]
   Installation Source: SDK 9.0.100-preview.5
   Manifest Version:    9.0.0-preview.5.24307.10/9.0.100-preview.5
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\9.0.100-preview.5\microsoft.net.sdk.maui\9.0.0-preview.5.24307.10\WorkloadManifest.json
   Install Type:              Msi

 [maui-ios]
   Installation Source: SDK 9.0.100-preview.5
   Manifest Version:    9.0.0-preview.5.24307.10/9.0.100-preview.5
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\9.0.100-preview.5\microsoft.net.sdk.maui\9.0.0-preview.5.24307.10\WorkloadManifest.json
   Install Type:              Msi

 [maui-maccatalyst]
   Installation Source: SDK 9.0.100-preview.5
   Manifest Version:    9.0.0-preview.5.24307.10/9.0.100-preview.5
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\9.0.100-preview.5\microsoft.net.sdk.maui\9.0.0-preview.5.24307.10\WorkloadManifest.json
   Install Type:              Msi

 [maui-tizen]
   Installation Source: SDK 9.0.100-preview.5
   Manifest Version:    9.0.0-preview.5.24307.10/9.0.100-preview.5
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\9.0.100-preview.5\microsoft.net.sdk.maui\9.0.0-preview.5.24307.10\WorkloadManifest.json
   Install Type:              Msi

 [maui-windows]
   Installation Source: SDK 9.0.100-preview.5, VS 17.10.35004.147
   Manifest Version:    9.0.0-preview.5.24307.10/9.0.100-preview.5
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\9.0.100-preview.5\microsoft.net.sdk.maui\9.0.0-preview.5.24307.10\WorkloadManifest.json
   Install Type:              Msi


Host:
  Version:      9.0.0-preview.5.24306.7
  Architecture: x64
  Commit:       a5cc707d97

.NET SDKs installed:
  8.0.300 [C:\Program Files\dotnet\sdk]
  9.0.100-preview.5.24307.3 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 9.0.0-preview.5.24306.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.31 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 9.0.0-preview.5.24306.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 6.0.30 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.31 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 9.0.0-preview.5.24306.8 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download

Other information

This is not specific to any MAUI version and only occurs if skiasharp has to be loaded. I have attached some logs that may be useful below. A managed stack trace is not available and I have no other logs besides ones directly shared with Microsoft Staff. Contact @grendello for the detailed log.

07-03 13:33:30.771 11963 11963 D Mono    : Assembly SkiaSharp[0xb400007a87c410c0] added to ALC 'Default'[0xb400007af7c2cf50], ref_count=1
07-03 13:33:30.772 11963 11963 E libsigchain: reverting to SIG_DFL handler for signal 11, ucontext 0x7cc0229e20
07-03 13:33:30.781 11963 11963 E libsigchain:   #00 pc 00006aa4  /apex/com.android.art/lib64/libsigchain.so (LogStack()+164) (BuildId: 438b5b7c0260735178ea56d10fc400a0)
07-03 13:33:30.781 11963 11963 E libsigchain:   #01 pc 000072d4  /apex/com.android.art/lib64/libsigchain.so (art::SignalChain::Handler(int, siginfo*, void*)+1024) (BuildId: 438b5b7c0260735178ea56d10fc400a0)
07-03 13:33:30.781 11963 11963 E libsigchain:   #02 pc 0000088c  [vdso] (+0) (BuildId: )
07-03 13:33:30.781 11963 11963 E libsigchain:   #03 pc 0013c970  /data/app/~~-ShlTI18gqQe9dIMjUUvFw==/com.my_app-KycGcojmE-p1uUYXr1r2YA==/split_config.arm64_v8a.apk (+0) (BuildId: d9a5ed7a80e9461ccb069853eda5e09f2423f875)
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jul 3, 2024
@lambdageek lambdageek added area-Codegen-AOT-mono and removed area-Setup untriaged New issue has not been triaged by the area owner labels Jul 3, 2024
@lambdageek lambdageek added this to the 9.0.0 milestone Jul 3, 2024
@lambdageek
Copy link
Member

lambdageek commented Jul 3, 2024

Discussed with @grendello and tehre's actually two problems here:

  1. Android SDK doesn't deal well with collisions when there's a native library (libSkiaSharp.so) that has a name that matches the base name of a managed assembly (SkiaSharp.dll) because the AOT image lookup may ask for libSkiaSharp.so if it doesn't find its preferred name SkiaSharp.dll.so (I believe this is the first one we ask for). That's DSO handling for like-named libraries is broken android#9081

  2. On the runtime side, we need to be more graceful here:

if (!info) {
find_symbol (sofile, globals, "mono_aot_version", (gpointer *) &version_symbol);
find_symbol (sofile, globals, "mono_aot_file_info", (gpointer*)&info);
}
// Copy aotid to MonoImage
memcpy(&assembly->image->aotid, info->aotid, 16);
if (version_symbol) {
/* Old file format */
version = atoi (version_symbol);
} else {
g_assert (info);
version = info->version;
}

if we found sofile, but it doesn't have either the mono_aot_version symbol or the mono_aot_file_info symbol we should set usable = FALSE and fail out gracefully. Instead the first thing we try to do is memcpy from info->aotid even if info might be null.

@lambdageek
Copy link
Member

/cc @vitek-karas @steveisok

@lambdageek
Copy link
Member

A more robust solution here is to make the runtime native library loader do exactly what the Android SDK is attempting to hack in using the existing native library fallback hooks.

From dotnet/android#9081 (comment):

In order to do that, I think the best way would be if Mono JIT asked to load aot-Assembly.so when it loads Assembly.dll and looks for its AOT image - this could be implemented only on Android or it could be used on all the platforms.

I think if we were to implement this, I would suggest something like mono-aot://assembly.dll and then Android SDK's native loader hook could resolve that however it wants.

(This code path would be primarily android only - ios and wasm use static linking for AOT images so they never do the dlopen call at all)

I think we can do this pretty surgically by adding a new mono cmake flag MONO_AOT_EXACT_SCHEME which means that the host application (the Android SDK in this case) wants to see names from the AOT runtime using the mono-aot:// scheme and without the .so suffix. That is, ifdef here:

aot_name = g_strdup_printf ("%s%s", assembly->image->name, MONO_SOLIB_EXT);

#ifndef MONO_AOT_EXACT_SCHEME
aot_name = g_strdup_printf ("%s%s", assembly->image->name, MONO_SOLIB_EXT);
#else
#define MONO_AOT_SCHEME_PREFIX "mono-aot://"
aot_name = g_strdup_printf ("%s%s", MONO_AOT_SCHEME_PREFIX, assembly->image->name);
#endif

Furthermore, AOT image loader should then refrain from asking for other permutations on the name, i.e. it should not follow with the attempt to load Assembly.so etc.

I think we can probably add a new flags to mono_dl_open - something like MONO_DL_FALLBACK_ONLY that would just pass the name to the native loader fallback function and not attempt a native dlopen at all

Then at

sofile = mono_dl_open (aot_name, MONO_DL_LAZY, load_error);

#ifndef MONO_AOT_EXACT_SCHEME
sofile = mono_dl_open (aot_name, MONO_DL_LAZY, load_error);
#else
sofile = mono_dl_open (aot_name, MONO_DL_LAZY | MONO_DL_FALLBACK_ONLY, load_error);
#endif

and also ifdef out this backup probing:

if (!sofile) {
GList *l;
for (l = mono_aot_paths; l; l = l->next) {
char *path = (char*)l->data;
char *basename = g_path_get_basename (assembly->image->name);
aot_name = g_strdup_printf ("%s/%s%s", path, basename, MONO_SOLIB_EXT);
ERROR_DECL (load_error);
sofile = mono_dl_open (aot_name, MONO_DL_LAZY, load_error);
if (sofile)
found_aot_name = g_strdup (aot_name);
else
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_AOT, "AOT: image '%s' not found: %s", aot_name, mono_error_get_message_without_fields (load_error));
mono_error_cleanup (load_error);
g_free (basename);
g_free (aot_name);
if (sofile)
break;
}
}

And finally implement the new MONO_DL_FALLBACK_ONLY flags in mono-dl.c:

// No GC safe transition because this is called early in main.c
lib = mono_dl_open_file (name, lflags, load_error);
if (!lib && !is_ok (load_error) && mono_error_get_error_code (load_error) == MONO_ERROR_BAD_IMAGE) {
char *error_msg = mono_dl_current_error_string ();
mono_error_set_error (error, MONO_ERROR_BAD_IMAGE, "%s", error_msg);
g_free (error_msg);
mono_error_cleanup (load_error);
return NULL;
}
mono_error_cleanup (load_error);
if (lib)
found_name = g_strdup (name);
if (!lib) {
GSList *node;
for (node = fallback_handlers; node != NULL; node = node->next){
MonoDlFallbackHandler *handler = (MonoDlFallbackHandler *) node->data;
char *error_msg = NULL;
lib = handler->load_func (name, lflags, &error_msg, handler->user_data);
g_free (error_msg);
if (lib != NULL){
dl_fallback = handler;
found_name = g_strdup (name);
break;
}
}
}

don't call mono_dl_open_file if FALLBACK_ONLY is specified, and don't do the name variations near here:

if (!lib && !dl_fallback) {
char *lname;
char *llname;
const char *suff;
const char *ext;
/* This platform does not support dlopen */
if (name == NULL) {
g_free (module);
mono_error_set_not_supported (error, NULL);
return NULL;
}
suff = ".la";
ext = strrchr (name, '.');
if (ext && strcmp (ext, ".la") == 0)
suff = "";
lname = g_strconcat (name, suff, (const char*)NULL);
llname = get_dl_name_from_libtool (lname);
g_free (lname);
if (llname) {
error_init_reuse (load_error);
lib = mono_dl_open_file (llname, lflags, load_error);
mono_error_cleanup (load_error);
if (lib)
found_name = g_strdup (llname);
#if defined (_AIX)
/*
* HACK: deal with AIX archive members because libtool
* underspecifies when using --with-aix-soname=svr4 -
* without this check, Mono can't find System.Native
* at build time.
* XXX: Does this also need to be in other places?
*/
if (!lib && is_library_ar_archive (llname)) {
/* try common suffix */
char *llaixname;
llaixname = g_strconcat (llname, "(shr_64.o)", (const char*)NULL);
error_init_reuse (load_error);
lib = mono_dl_open_file (llaixname, lflags, load_error);
mono_error_cleanup (load_error);
if (lib)
found_name = g_strdup (llaixname);
/* XXX: try another suffix like (shr.o)? */
g_free (llaixname);
}
#endif
g_free (llname);
}
if (!lib) {
char *error_msg = mono_dl_current_error_string ();
mono_error_set_error (error, MONO_ERROR_FILE_NOT_FOUND, "%s", error_msg);
g_free (error_msg);
g_free (module);
return NULL;
}
}

@daltzctr
Copy link
Author

Any update on this?

@matouskozak
Copy link
Member

#104397 (comment) has been address in #106026 and is going to be part of .NET 9. The remaining work #104397 (comment) is moved to .NET 10.

@matouskozak matouskozak modified the milestones: 9.0.0, 10.0.0 Aug 7, 2024
jonpryor pushed a commit to dotnet/android that referenced this issue Aug 27, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants