Skip to content

Commit bfa8eab

Browse files
authored
[monodroid] Attach threads before asking for current domain (dotnet#6223)
Fixes: dotnet#6211 Mono VM will return a valid AppDomain pointer (both in "legacy" and NET6 cases) only if the current thread is attached to some domain. It is possible that when managed code is called from an unattached Java thread, `mono_domain_get()` will return `nullptr` instead of a valid one. If we further pass `nullptr` to other Mono functions, a segfault may occur if Mono fails to validate the passed pointer. Sample code which may trigger the problem: public override void OnCreate() { AppDomain.CurrentDomain.UnhandledException += (sender, e) => { Console.WriteLine("!!! UnhandledException: " + e.ExceptionObject.GetType().FullName); }; base.OnCreate(); } protected override void OnStart() { base.OnStart(); Java.Util.Concurrent.Executors.NewSingleThreadExecutor() .Execute(new Runnable(() => { throw new Exception("Exception from Java Executor!"); })); } Possible crash caused by the above code: F libc : Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0xb8 in tid 19241 (pool-1-thread-1), pid 19214 (ndroidcrashtest) F DEBUG : backtrace: I hwservicemanager: getTransport: Cannot find entry [email protected]::IMapper/default in either framework or device manifest. F DEBUG : #00 pc 0011b72f /apex/com.android.runtime/lib/bionic/libc.so (pthread_mutex_lock+31) (BuildId: 471745f0fbbcedb3db1553d5bd6fcd8b) F DEBUG : #1 pc 0015240d /data/app/com.companyname.androidcrashtest-sqgk-Mnu8GZM5hKPu1yWEQ==/lib/x86/libmonosgen-2.0.so (mono_domain_assembly_search+61) F libc : Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0xb8 in tid 19763 (pool-1-thread-1), pid 19737 (ndroidcrashtest) F DEBUG : backtrace: F DEBUG : #00 pc 0011b72f /apex/com.android.runtime/lib/bionic/libc.so (pthread_mutex_lock+31) (BuildId: 471745f0fbbcedb3db1553d5bd6fcd8b) F DEBUG : #1 pc 0026b91a /data/app/com.companyname.androidcrashtest-JQoYc3YFwZtEoSJlTqX_BA==/lib/x86/libmonosgen-2.0.so (mono_os_mutex_lock+42) To avoid the above situation, wrap the `mono_domain_get()` call into `utils.get_current_domain()` which checks the return value of `mono_domain_get()` and, if it's `nullptr`, calls `mono_get_root_domain()` and attaches the current thread to that root domain.
1 parent 21bfc47 commit bfa8eab

File tree

6 files changed

+34
-13
lines changed

6 files changed

+34
-13
lines changed

src/monodroid/jni/embedded-assemblies.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ EmbeddedAssemblies::typemap_java_to_managed (const char *java_type_name)
494494
return nullptr;
495495
}
496496

497-
MonoReflectionType *ret = mono_type_get_object (mono_domain_get (), type);
497+
MonoReflectionType *ret = mono_type_get_object (utils.get_current_domain (), type);
498498
if (XA_UNLIKELY (ret == nullptr)) {
499499
log_warn (LOG_ASSEMBLY, "typemap: unable to instantiate managed type '%s'", managed_type_name);
500500
return nullptr;
@@ -552,7 +552,7 @@ EmbeddedAssemblies::typemap_java_to_managed (const char *java_type_name)
552552
// calls `mono_get_root_domain`. Thus, we can save on a one function call here by passing `nullptr`
553553
constexpr MonoDomain *domain = nullptr;
554554
#else
555-
MonoDomain *domain = mono_domain_get ();
555+
MonoDomain *domain = utils.get_current_domain ();
556556
#endif
557557
MonoReflectionType *ret = mono_type_get_object (domain, mono_class_get_type (klass));
558558
if (ret == nullptr) {

src/monodroid/jni/monodroid-glue.cc

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1741,14 +1741,16 @@ MonodroidRuntime::load_assembly (MonoDomain *domain, jstring_wrapper &assembly)
17411741
log_debug (LOG_ASSEMBLY, "Dynamically opened assembly %s", mono_assembly_name_get_name (aname));
17421742
} else
17431743
#endif
1744-
if (domain != mono_domain_get ()) {
1745-
MonoDomain *current = mono_domain_get ();
1744+
{
1745+
MonoDomain *current = utils.get_current_domain ();
1746+
if (domain != current) {
17461747
mono_domain_set (domain, FALSE);
17471748
mono_assembly_load_full (aname, NULL, NULL, 0);
17481749
mono_domain_set (current, FALSE);
17491750
} else {
17501751
mono_assembly_load_full (aname, NULL, NULL, 0);
17511752
}
1753+
}
17521754

17531755
mono_assembly_name_free (aname);
17541756

@@ -2327,7 +2329,7 @@ MonodroidRuntime::Java_mono_android_Runtime_register (JNIEnv *env, jstring manag
23272329

23282330
MonoMethod *register_jni_natives = registerType;
23292331
#if !defined (NET6)
2330-
MonoDomain *domain = mono_domain_get ();
2332+
MonoDomain *domain = utils.get_current_domain (/* attach_thread_if_needed */ false);
23312333
mono_jit_thread_attach (domain);
23322334
// Refresh current domain as it might have been modified by the above call
23332335
domain = mono_domain_get ();
@@ -2405,7 +2407,7 @@ JNICALL Java_mono_android_Runtime_propagateUncaughtException (JNIEnv *env, [[may
24052407
#if defined (NET6)
24062408
monodroidRuntime.propagate_uncaught_exception (env, javaThread, javaException);
24072409
#else // def NET6
2408-
MonoDomain *domain = mono_domain_get ();
2410+
MonoDomain *domain = utils.get_current_domain ();
24092411
monodroidRuntime.propagate_uncaught_exception (domain, env, javaThread, javaException);
24102412
#endif // ndef NET6
24112413
}

src/monodroid/jni/osbridge.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1041,7 +1041,7 @@ OSBridge::ensure_jnienv (void)
10411041
JNIEnv *env;
10421042
jvm->GetEnv ((void**)&env, JNI_VERSION_1_6);
10431043
if (env == nullptr) {
1044-
mono_thread_attach (mono_domain_get ());
1044+
mono_thread_attach (utils.get_current_domain (/* attach_thread_if_needed */ false));
10451045
jvm->GetEnv ((void**)&env, JNI_VERSION_1_6);
10461046
}
10471047
return env;

src/monodroid/jni/timezones.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ init ()
3333
if (AndroidEnvironment_NotifyTimeZoneChanged)
3434
return;
3535

36-
Mono_Android_dll = utils.monodroid_load_assembly (mono_domain_get (), SharedConstants::MONO_ANDROID_ASSEMBLY_NAME);
36+
Mono_Android_dll = utils.monodroid_load_assembly (utils.get_current_domain (), SharedConstants::MONO_ANDROID_ASSEMBLY_NAME);
3737
Mono_Android_image = mono_assembly_get_image (Mono_Android_dll);
3838
AndroidEnvironment = mono_class_from_name (Mono_Android_image, SharedConstants::ANDROID_RUNTIME_NS_NAME, SharedConstants::ANDROID_ENVIRONMENT_CLASS_NAME);
3939
AndroidEnvironment_NotifyTimeZoneChanged = mono_class_get_method_from_name (AndroidEnvironment, "NotifyTimeZoneChanged", 0);

src/monodroid/jni/util.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ Util::monodroid_load_assembly (MonoDomain *domain, const char *basename)
210210
MonoImageOpenStatus status;
211211

212212
aname = mono_assembly_name_new (basename);
213-
MonoDomain *current = mono_domain_get ();
213+
MonoDomain *current = get_current_domain ();
214214

215215
if (domain != current) {
216216
mono_domain_set (domain, FALSE);
@@ -233,7 +233,7 @@ Util::monodroid_load_assembly (MonoDomain *domain, const char *basename)
233233
MonoObject *
234234
Util::monodroid_runtime_invoke (MonoDomain *domain, MonoMethod *method, void *obj, void **params, MonoObject **exc)
235235
{
236-
MonoDomain *current = mono_domain_get ();
236+
MonoDomain *current = get_current_domain ();
237237
if (domain == current) {
238238
return mono_runtime_invoke (method, obj, params, exc);
239239
}
@@ -247,7 +247,7 @@ Util::monodroid_runtime_invoke (MonoDomain *domain, MonoMethod *method, void *ob
247247
void
248248
Util::monodroid_property_set (MonoDomain *domain, MonoProperty *property, void *obj, void **params, MonoObject **exc)
249249
{
250-
MonoDomain *current = mono_domain_get ();
250+
MonoDomain *current = get_current_domain ();
251251
if (domain == current) {
252252
mono_property_set_value (property, obj, params, exc);
253253
return;
@@ -289,7 +289,7 @@ MonoClass*
289289
Util::monodroid_get_class_from_name ([[maybe_unused]] MonoDomain *domain, const char* assembly, const char *_namespace, const char *type)
290290
{
291291
#if !defined (NET6)
292-
MonoDomain *current = mono_domain_get ();
292+
MonoDomain *current = get_current_domain ();
293293

294294
if (domain != current)
295295
mono_domain_set (domain, FALSE);
@@ -318,7 +318,7 @@ Util::monodroid_get_class_from_name ([[maybe_unused]] MonoDomain *domain, const
318318
MonoClass*
319319
Util::monodroid_get_class_from_image (MonoDomain *domain, MonoImage *image, const char *_namespace, const char *type)
320320
{
321-
MonoDomain *current = mono_domain_get ();
321+
MonoDomain *current = get_current_domain ();
322322

323323
if (domain != current)
324324
mono_domain_set (domain, FALSE);

src/monodroid/jni/util.hh

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ constexpr int FALSE = 0;
3232
#include <jni.h>
3333
#include <mono/metadata/assembly.h>
3434
#include <mono/metadata/appdomain.h>
35+
#include <mono/metadata/threads.h>
3536

3637
#include "monodroid.h"
3738
#include "jni-wrappers.hh"
@@ -110,6 +111,24 @@ namespace xamarin::android
110111
return (log_categories & category) != 0;
111112
}
112113

114+
MonoDomain *get_current_domain (bool attach_thread_if_needed = true) const noexcept
115+
{
116+
MonoDomain *ret = mono_domain_get ();
117+
if (ret != nullptr) {
118+
return ret;
119+
}
120+
121+
// It's likely that we got a nullptr because the current thread isn't attached (see
122+
// https://github.com/xamarin/xamarin-android/issues/6211), so we need to attach the thread to the root
123+
// domain
124+
ret = mono_get_root_domain ();
125+
if (attach_thread_if_needed) {
126+
mono_thread_attach (ret);
127+
}
128+
129+
return ret;
130+
}
131+
113132
private:
114133
//char *monodroid_strdup_printf (const char *format, va_list vargs);
115134
#if !defined (NET6)

0 commit comments

Comments
 (0)