Skip to content

Commit

Permalink
[monodroid] Attach current thread to MonoVM when registering a type (#…
Browse files Browse the repository at this point in the history
…7560)

Fixes: #7532

Whenever any MonoVM APIs are called, the current thread of execution
must be attached to the runtime or we risk native code crashes, e.g.:

	Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x40 in tid 1860 (jg_fr_pool_thre), pid 1792 (yname.sampleapp)
	backtrace:
	  00 pc 0011588e  /data/app/~~lwgdvtQhIfK0II0Nd0mtag==/com.companyname.sampleapp-8yF54k9YuWCWpsuGRJvVQw==/lib/x86/libmonosgen-2.0.so
	  01 pc 001156ae  /data/app/~~lwgdvtQhIfK0II0Nd0mtag==/com.companyname.sampleapp-8yF54k9YuWCWpsuGRJvVQw==/lib/x86/libmonosgen-2.0.so
	  02 pc 00115575  /data/app/~~lwgdvtQhIfK0II0Nd0mtag==/com.companyname.sampleapp-8yF54k9YuWCWpsuGRJvVQw==/lib/x86/libmonosgen-2.0.so (mono_threads_enter_gc_unsafe_region_internal+53)
	  03 pc 000973c3  /data/app/~~lwgdvtQhIfK0II0Nd0mtag==/com.companyname.sampleapp-8yF54k9YuWCWpsuGRJvVQw==/lib/x86/libmonosgen-2.0.so (mono_runtime_invoke+51) (BuildId: c54662bbf82dbdadd595ca9d3e31dce29735885f)
	  04 pc 00027ace  /data/app/~~lwgdvtQhIfK0II0Nd0mtag==/com.companyname.sampleapp-8yF54k9YuWCWpsuGRJvVQw==/lib/x86/libmonodroid.so (xamarin::android::internal::MonodroidRuntime::Java_mono_android_Runtime_register(_JNIEnv*, _jstring*, _jclass*, _jstring*)+286)
	  05 pc 000279a0  /data/app/~~lwgdvtQhIfK0II0Nd0mtag==/com.companyname.sampleapp-8yF54k9YuWCWpsuGRJvVQw==/lib/x86/libmonodroid.so (Java_mono_android_Runtime_register+48)

The above trace translates to the following locations in the Mono runtime:

	copy_stack_data_internal
	/__w/1/s/src/mono/mono/utils/mono-threads-coop.c:191
	copy_stack_data
	/__w/1/s/src/mono/mono/utils/mono-threads-coop.c:246

	mono_threads_enter_gc_unsafe_region_unbalanced_with_info
	/__w/1/s/src/mono/mono/utils/mono-threads-coop.c:476

	mono_runtime_invoke
	/__w/1/s/src/mono/mono/metadata/object.c:2442

In this case, a pointer to Mono thread information structure
(`MonoThreadInfo*`) is null in `copy_stack_data()` which, in turn,
causes the segfault when the pointer is dereferenced.

In the case of issue #7532, `Java_mono_android_Runtime_register()` is
called on behalf of a 3rd party library on a thread that is, most
likely, created by that library and thus not (yet) attached to the
runtime by the time the registration attempt is made.

Attach thread to the runtime by calling
`mono_jit_thread_attach(nullptr)` in .NET builds to fix the issue.
`nullptr` is used because .NET only has a single AppDomain and
there's no need to pass around pointers to it.

TODO: Try to create a test case.  Our attempted unit tests don't
cause an app crash when the fix isn't applied -- i.e. when
`mono_jit_thread_attach(nullptr)` *isn't* called -- which suggests
that we don't fully understand the reported bug report.
  • Loading branch information
grendello authored and jonpryor committed Nov 30, 2022
1 parent d0f1e75 commit 087cf2b
Show file tree
Hide file tree
Showing 3 changed files with 162 additions and 3 deletions.
2 changes: 2 additions & 0 deletions src/monodroid/jni/monodroid-glue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2544,6 +2544,8 @@ MonodroidRuntime::Java_mono_android_Runtime_register (JNIEnv *env, jstring manag

utils.monodroid_runtime_invoke (domain, register_jni_natives, nullptr, args, nullptr);
#else // ndef NET
mono_jit_thread_attach (nullptr); // There's just one domain in .net

#if !defined (ANDROID)
mono_runtime_invoke (register_jni_natives, nullptr, args, nullptr);
#else
Expand Down
38 changes: 38 additions & 0 deletions tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,31 @@ public void TestMyPaintColor ()
}
}

[DllImport ("reuse-threads")]
static extern int rt_register_type_on_new_thread (string java_type_namem, IntPtr class_loader);

delegate void CB (IntPtr jnienv, IntPtr java_instance);

[DllImport ("reuse-threads")]
static extern int rt_invoke_callback_on_new_thread (CB cb);

[Test]
public void RegisterTypeOnNewNativeThread ()
{
Java.Lang.JavaSystem.LoadLibrary ("reuse-threads");
int ret = rt_register_type_on_new_thread ("from.NewThreadOne", Application.Context.ClassLoader.Handle);
Assert.AreEqual (0, ret, $"Java type registration on a new thread failed with code {ret}");
}

[Test]
public void RegisterTypeOnNewJavaThread ()
{
var thread = new MyRegistrationThread ();
thread.Start ();
thread.Join (5000);
Assert.AreNotEqual (null, thread.Instance, "Failed to register instance of a class on new thread");
}

[Test]
public void ThreadReuse ()
{
Expand Down Expand Up @@ -434,6 +454,24 @@ public void DoNotLeakWeakReferences ()
}
}

[Register ("from/NewThreadOne")]
class RegisterMeOnNewThreadOne : Java.Lang.Object
{}

[Register ("from/NewThreadTwo")]
class RegisterMeOnNewThreadTwo : Java.Lang.Object
{}

class MyRegistrationThread : Java.Lang.Thread
{
public RegisterMeOnNewThreadTwo Instance { get; private set; }

public override void Run ()
{
Instance = new RegisterMeOnNewThreadTwo ();
}
}

class MyCb : Java.Lang.Object, Java.Lang.IRunnable {
public void Run ()
{
Expand Down
125 changes: 122 additions & 3 deletions tests/Mono.Android-Tests/jni/reuse-threads.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@
#include <android/log.h>
#include <jni.h>

typedef struct
{
const char *java_type_name;
jobject class_loader;
} RegisterFromThreadContext;

typedef void (*CB)(JNIEnv *env, jobject self);

static JavaVM *gvm;
Expand Down Expand Up @@ -113,9 +119,9 @@ _get_env (const char *where)
}

static jobject
_create_java_instance (JNIEnv *env)
_create_java_instance (JNIEnv *env, const char *class_name)
{
jclass Object_class = (*env)->FindClass (env, "java/lang/Object");
jclass Object_class = (*env)->FindClass (env, class_name);
jmethodID Object_ctor = (*env)->GetMethodID (env, Object_class, "<init>", "()V");

jobject instance = (*env)->NewObject (env, Object_class, Object_ctor);
Expand Down Expand Up @@ -154,7 +160,7 @@ _call_cb_from_new_thread (void *cb)
}

/* 5: Execution of T enters managed code... */
jobject instance = _create_java_instance (env);
jobject instance = _create_java_instance (env, "java/lang/Object");
_cb (env, instance);

return NULL;
Expand Down Expand Up @@ -200,3 +206,116 @@ rt_invoke_callback_on_new_thread (CB cb)
return 0;
}

/* We return -2 for errors, because -1 is reserved for the pthreads PTHREAD_CANCELED special value, indicating that the
* thread was canceled. */
static int
_register_type_from_new_thread (void *data)
{
RegisterFromThreadContext *context = (RegisterFromThreadContext*)data;

if (context == NULL) {
return -100;
}

JNIEnv *env = _get_env ("_register_type_from_new_thread");

if ((*env)->PushLocalFrame (env, 4) < 0) {
__android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: unable to create a local reference frame!");

if ((*env)->ExceptionOccurred (env)) {
(*env)->ExceptionDescribe (env);
(*env)->ExceptionClear (env);
}

return -101;
}

int ret = 0;
jclass ClassLoader_class = (*env)->FindClass (env, "java/lang/ClassLoader");
if (ClassLoader_class == NULL) {
ret = -102;
__android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: unable to find the 'java/lang/ClassLoader' class!");
goto cleanup;
}

jmethodID loadClass = (*env)->GetMethodID (env, ClassLoader_class, "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;");
if (loadClass == NULL) {
ret = -103;
__android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: unable to get id of method 'loadClass' in the 'java/lang/ClassLoader' class!");
goto cleanup;
}

jstring klass_name = (*env)->NewStringUTF (env, context->java_type_name);
jobject loaded_class = (*env)->CallObjectMethod (env, context->class_loader, loadClass, klass_name);

if ((*env)->ExceptionOccurred (env) != NULL) {
__android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: class '%s' cannot be loaded, Java exception thrown!", context->java_type_name);
(*env)->ExceptionDescribe (env);
(*env)->ExceptionClear (env);
ret = -104;
goto cleanup;
}

if (loaded_class == NULL) {
ret = -105;
__android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: 'java/lang/ClassLoader' wasn't able to load the '%s' class!", context->java_type_name);
goto cleanup;
}

jmethodID Object_ctor = (*env)->GetMethodID (env, loaded_class, "<init>", "()V");
if (Object_ctor == NULL) {
ret = -106;
__android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: unable to find the '%s' class constructor!", context->java_type_name);
goto cleanup;
}

jobject instance = (*env)->NewObject (env, loaded_class, Object_ctor);

if ((*env)->ExceptionOccurred (env) != NULL || instance == NULL) {
__android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: instance of class '%s' wasn't created!", context->java_type_name);
(*env)->ExceptionDescribe (env);
(*env)->ExceptionClear (env);
ret = -107;
}

if (instance == NULL) {
ret = -108;
__android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: unable to create instance of the '%s' class!", context->java_type_name);
}

cleanup:
(*env)->PopLocalFrame (env, NULL);

return ret;
}

JNIEXPORT int JNICALL
rt_register_type_on_new_thread (const char *java_type_name, jobject class_loader)
{
JNIEnv *env = _get_env ("rt_register_type_on_new_thread");
pthread_t t;
RegisterFromThreadContext context = {
java_type_name,
class_loader,
};

int r = pthread_create (&t, NULL, _register_type_from_new_thread, &context);

if (r) {
__android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "RegisterOnNewThread: pthread_create() failed! %i: %s", r, strerror (r));
return -200;
}

void *tr;
if (pthread_join (t, &tr) != 0) {
__android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "RegisterOnNewThread: pthread_join() failed! %i: %s", r, strerror (r));
return -201;
}

if ((int)tr == -1 /* PTHREAD_CANCELED - not defined in bionic */) {
__android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "RegisterOnNewThread: worker thread was canceled");
return -202;
}

return (int)tr;
}

0 comments on commit 087cf2b

Please sign in to comment.