Skip to content

Commit

Permalink
[monodroid] Attach threads before asking for current domain (#6223)
Browse files Browse the repository at this point in the history
Fixes: #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.
  • Loading branch information
grendello authored Aug 26, 2021
1 parent 21bfc47 commit bfa8eab
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 13 deletions.
4 changes: 2 additions & 2 deletions src/monodroid/jni/embedded-assemblies.cc
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ EmbeddedAssemblies::typemap_java_to_managed (const char *java_type_name)
return nullptr;
}

MonoReflectionType *ret = mono_type_get_object (mono_domain_get (), type);
MonoReflectionType *ret = mono_type_get_object (utils.get_current_domain (), type);
if (XA_UNLIKELY (ret == nullptr)) {
log_warn (LOG_ASSEMBLY, "typemap: unable to instantiate managed type '%s'", managed_type_name);
return nullptr;
Expand Down Expand Up @@ -552,7 +552,7 @@ EmbeddedAssemblies::typemap_java_to_managed (const char *java_type_name)
// calls `mono_get_root_domain`. Thus, we can save on a one function call here by passing `nullptr`
constexpr MonoDomain *domain = nullptr;
#else
MonoDomain *domain = mono_domain_get ();
MonoDomain *domain = utils.get_current_domain ();
#endif
MonoReflectionType *ret = mono_type_get_object (domain, mono_class_get_type (klass));
if (ret == nullptr) {
Expand Down
10 changes: 6 additions & 4 deletions src/monodroid/jni/monodroid-glue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1741,14 +1741,16 @@ MonodroidRuntime::load_assembly (MonoDomain *domain, jstring_wrapper &assembly)
log_debug (LOG_ASSEMBLY, "Dynamically opened assembly %s", mono_assembly_name_get_name (aname));
} else
#endif
if (domain != mono_domain_get ()) {
MonoDomain *current = mono_domain_get ();
{
MonoDomain *current = utils.get_current_domain ();
if (domain != current) {
mono_domain_set (domain, FALSE);
mono_assembly_load_full (aname, NULL, NULL, 0);
mono_domain_set (current, FALSE);
} else {
mono_assembly_load_full (aname, NULL, NULL, 0);
}
}

mono_assembly_name_free (aname);

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

MonoMethod *register_jni_natives = registerType;
#if !defined (NET6)
MonoDomain *domain = mono_domain_get ();
MonoDomain *domain = utils.get_current_domain (/* attach_thread_if_needed */ false);
mono_jit_thread_attach (domain);
// Refresh current domain as it might have been modified by the above call
domain = mono_domain_get ();
Expand Down Expand Up @@ -2405,7 +2407,7 @@ JNICALL Java_mono_android_Runtime_propagateUncaughtException (JNIEnv *env, [[may
#if defined (NET6)
monodroidRuntime.propagate_uncaught_exception (env, javaThread, javaException);
#else // def NET6
MonoDomain *domain = mono_domain_get ();
MonoDomain *domain = utils.get_current_domain ();
monodroidRuntime.propagate_uncaught_exception (domain, env, javaThread, javaException);
#endif // ndef NET6
}
2 changes: 1 addition & 1 deletion src/monodroid/jni/osbridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,7 @@ OSBridge::ensure_jnienv (void)
JNIEnv *env;
jvm->GetEnv ((void**)&env, JNI_VERSION_1_6);
if (env == nullptr) {
mono_thread_attach (mono_domain_get ());
mono_thread_attach (utils.get_current_domain (/* attach_thread_if_needed */ false));
jvm->GetEnv ((void**)&env, JNI_VERSION_1_6);
}
return env;
Expand Down
2 changes: 1 addition & 1 deletion src/monodroid/jni/timezones.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ init ()
if (AndroidEnvironment_NotifyTimeZoneChanged)
return;

Mono_Android_dll = utils.monodroid_load_assembly (mono_domain_get (), SharedConstants::MONO_ANDROID_ASSEMBLY_NAME);
Mono_Android_dll = utils.monodroid_load_assembly (utils.get_current_domain (), SharedConstants::MONO_ANDROID_ASSEMBLY_NAME);
Mono_Android_image = mono_assembly_get_image (Mono_Android_dll);
AndroidEnvironment = mono_class_from_name (Mono_Android_image, SharedConstants::ANDROID_RUNTIME_NS_NAME, SharedConstants::ANDROID_ENVIRONMENT_CLASS_NAME);
AndroidEnvironment_NotifyTimeZoneChanged = mono_class_get_method_from_name (AndroidEnvironment, "NotifyTimeZoneChanged", 0);
Expand Down
10 changes: 5 additions & 5 deletions src/monodroid/jni/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ Util::monodroid_load_assembly (MonoDomain *domain, const char *basename)
MonoImageOpenStatus status;

aname = mono_assembly_name_new (basename);
MonoDomain *current = mono_domain_get ();
MonoDomain *current = get_current_domain ();

if (domain != current) {
mono_domain_set (domain, FALSE);
Expand All @@ -233,7 +233,7 @@ Util::monodroid_load_assembly (MonoDomain *domain, const char *basename)
MonoObject *
Util::monodroid_runtime_invoke (MonoDomain *domain, MonoMethod *method, void *obj, void **params, MonoObject **exc)
{
MonoDomain *current = mono_domain_get ();
MonoDomain *current = get_current_domain ();
if (domain == current) {
return mono_runtime_invoke (method, obj, params, exc);
}
Expand All @@ -247,7 +247,7 @@ Util::monodroid_runtime_invoke (MonoDomain *domain, MonoMethod *method, void *ob
void
Util::monodroid_property_set (MonoDomain *domain, MonoProperty *property, void *obj, void **params, MonoObject **exc)
{
MonoDomain *current = mono_domain_get ();
MonoDomain *current = get_current_domain ();
if (domain == current) {
mono_property_set_value (property, obj, params, exc);
return;
Expand Down Expand Up @@ -289,7 +289,7 @@ MonoClass*
Util::monodroid_get_class_from_name ([[maybe_unused]] MonoDomain *domain, const char* assembly, const char *_namespace, const char *type)
{
#if !defined (NET6)
MonoDomain *current = mono_domain_get ();
MonoDomain *current = get_current_domain ();

if (domain != current)
mono_domain_set (domain, FALSE);
Expand Down Expand Up @@ -318,7 +318,7 @@ Util::monodroid_get_class_from_name ([[maybe_unused]] MonoDomain *domain, const
MonoClass*
Util::monodroid_get_class_from_image (MonoDomain *domain, MonoImage *image, const char *_namespace, const char *type)
{
MonoDomain *current = mono_domain_get ();
MonoDomain *current = get_current_domain ();

if (domain != current)
mono_domain_set (domain, FALSE);
Expand Down
19 changes: 19 additions & 0 deletions src/monodroid/jni/util.hh
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ constexpr int FALSE = 0;
#include <jni.h>
#include <mono/metadata/assembly.h>
#include <mono/metadata/appdomain.h>
#include <mono/metadata/threads.h>

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

MonoDomain *get_current_domain (bool attach_thread_if_needed = true) const noexcept
{
MonoDomain *ret = mono_domain_get ();
if (ret != nullptr) {
return ret;
}

// It's likely that we got a nullptr because the current thread isn't attached (see
// https://github.com/xamarin/xamarin-android/issues/6211), so we need to attach the thread to the root
// domain
ret = mono_get_root_domain ();
if (attach_thread_if_needed) {
mono_thread_attach (ret);
}

return ret;
}

private:
//char *monodroid_strdup_printf (const char *format, va_list vargs);
#if !defined (NET6)
Expand Down

0 comments on commit bfa8eab

Please sign in to comment.