From cb4ed067d75d72851df2381ecd9002344a3a8754 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Wed, 16 Nov 2022 18:13:18 +0100 Subject: [PATCH 01/16] [DotNet] Attach current thread to MonoVM when registering a type Fixes: https://github.com/xamarin/xamarin-android/issues/7532 Whenever any MonoVM APIs are called, the current thread of execution must be attached to the runtime or we risk native code crashes, for instance: 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 DotNet 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. --- src/monodroid/jni/monodroid-glue.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/monodroid/jni/monodroid-glue.cc b/src/monodroid/jni/monodroid-glue.cc index 5a8a3500951..7fb2a67dcb3 100644 --- a/src/monodroid/jni/monodroid-glue.cc +++ b/src/monodroid/jni/monodroid-glue.cc @@ -2537,6 +2537,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 From 7b1e1cf8fa7b29051efdfba43634a18062923b54 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 17 Nov 2022 16:44:13 +0100 Subject: [PATCH 02/16] Add a test --- .../Java.Interop/JnienvTest.cs | 14 ++++++ tests/Mono.Android-Tests/jni/reuse-threads.c | 46 +++++++++++++++++-- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs b/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs index 11ac6e04b18..38d3a6e1c6c 100644 --- a/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs +++ b/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs @@ -32,11 +32,21 @@ public void TestMyPaintColor () } } + [DllImport ("reuse-threads")] + static extern int rt_register_type_on_new_thread (string java_type_name); + 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 RegisterTypeOnNewThread () + { + Java.Lang.JavaSystem.LoadLibrary ("reuse-threads"); + rt_register_type_on_new_thread ("from/NewThread"); + } + [Test] public void ThreadReuse () { @@ -434,6 +444,10 @@ public void DoNotLeakWeakReferences () } } + [Register ("from/NewThread")] + class RegisterMeOnNewThread : Java.Lang.Object + {} + class MyCb : Java.Lang.Object, Java.Lang.IRunnable { public void Run () { diff --git a/tests/Mono.Android-Tests/jni/reuse-threads.c b/tests/Mono.Android-Tests/jni/reuse-threads.c index bdc1f415694..26d783751ec 100644 --- a/tests/Mono.Android-Tests/jni/reuse-threads.c +++ b/tests/Mono.Android-Tests/jni/reuse-threads.c @@ -113,9 +113,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, "", "()V"); jobject instance = (*env)->NewObject (env, Object_class, Object_ctor); @@ -154,7 +154,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; @@ -200,3 +200,43 @@ rt_invoke_callback_on_new_thread (CB cb) return 0; } +static void* +_register_type_from_new_thread (const char *java_type_name) +{ + JNIEnv *env = _get_env ("_register_type_from_new_thread"); + jobject instance = _create_java_instance (env, java_type_name); + + if (instance == NULL) { + __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: instance of class '%s' wasn't created!", java_type_name); + } + + return NULL; +} + +JNIEXPORT int JNICALL +rt_register_type_on_new_thread (const char *java_type_name) +{ + JNIEnv *env = _get_env ("rt_register_type_on_new_thread"); + pthread_t t; + + /* 1: Create a thread... */ + int r = pthread_create (&t, NULL, _register_type_from_new_thread, java_type_name); + + if (r) { + __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "RegisterOnNewThread: pthread_create() failed! %i: %s", r, strerror (r)); + return -1; + } + + /* 3(b): Ensure Dalvik gets a chance to cleanup the old JNIEnv* */ + sem_wait (&start_gc_on_main); + _gc (env); + _gc (env); /* for good measure... */ + + /* Allow (4) to execute... */ + sem_post (&finished_gc_on_main); + + void *tr; + pthread_join (t, &tr); + + return 0; +} From 0a9fa8d2e9f5883aa0f6b5082b9bee807e3bf21e Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 17 Nov 2022 17:09:20 +0100 Subject: [PATCH 03/16] Not needed for this test --- tests/Mono.Android-Tests/jni/reuse-threads.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/Mono.Android-Tests/jni/reuse-threads.c b/tests/Mono.Android-Tests/jni/reuse-threads.c index 26d783751ec..abb02ddc42c 100644 --- a/tests/Mono.Android-Tests/jni/reuse-threads.c +++ b/tests/Mono.Android-Tests/jni/reuse-threads.c @@ -227,14 +227,6 @@ rt_register_type_on_new_thread (const char *java_type_name) return -1; } - /* 3(b): Ensure Dalvik gets a chance to cleanup the old JNIEnv* */ - sem_wait (&start_gc_on_main); - _gc (env); - _gc (env); /* for good measure... */ - - /* Allow (4) to execute... */ - sem_post (&finished_gc_on_main); - void *tr; pthread_join (t, &tr); From baf60a8c2021d70a1e84690b4ab29cc6bf53a63d Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 17 Nov 2022 17:10:11 +0100 Subject: [PATCH 04/16] Testing if the test FAILS --- src/monodroid/jni/monodroid-glue.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/monodroid/jni/monodroid-glue.cc b/src/monodroid/jni/monodroid-glue.cc index 7fb2a67dcb3..afeb87d92ac 100644 --- a/src/monodroid/jni/monodroid-glue.cc +++ b/src/monodroid/jni/monodroid-glue.cc @@ -2537,7 +2537,7 @@ 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 +// mono_jit_thread_attach (nullptr); // There's just one domain in .net #if !defined (ANDROID) mono_runtime_invoke (register_jni_natives, nullptr, args, nullptr); From 5b88118025262e8c30692090d0ef311933ea1dbf Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 17 Nov 2022 22:29:30 +0100 Subject: [PATCH 05/16] Oops --- tests/Mono.Android-Tests/jni/reuse-threads.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Mono.Android-Tests/jni/reuse-threads.c b/tests/Mono.Android-Tests/jni/reuse-threads.c index abb02ddc42c..f28cd10f30f 100644 --- a/tests/Mono.Android-Tests/jni/reuse-threads.c +++ b/tests/Mono.Android-Tests/jni/reuse-threads.c @@ -154,7 +154,7 @@ _call_cb_from_new_thread (void *cb) } /* 5: Execution of T enters managed code... */ - jobject instance = _create_java_instance (env. "java/lang/Object"); + jobject instance = _create_java_instance (env, "java/lang/Object"); _cb (env, instance); return NULL; From 4e5c270699099f10456019a2ea7edb228b916106 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Fri, 18 Nov 2022 19:06:44 +0100 Subject: [PATCH 06/16] Update unit test --- .../Java.Interop/JnienvTest.cs | 5 ++-- tests/Mono.Android-Tests/jni/reuse-threads.c | 23 ++++++++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs b/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs index 38d3a6e1c6c..163484cc8c5 100644 --- a/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs +++ b/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs @@ -33,7 +33,7 @@ public void TestMyPaintColor () } [DllImport ("reuse-threads")] - static extern int rt_register_type_on_new_thread (string java_type_name); + static extern int rt_register_type_on_new_thread (string java_type_namem, IntPtr class_loader); delegate void CB (IntPtr jnienv, IntPtr java_instance); @@ -44,7 +44,8 @@ public void TestMyPaintColor () public void RegisterTypeOnNewThread () { Java.Lang.JavaSystem.LoadLibrary ("reuse-threads"); - rt_register_type_on_new_thread ("from/NewThread"); + int ret = rt_register_type_on_new_thread ("from/NewThread", Application.Context.ClassLoader.Handle); + Assert.AreEqual (0, ret, "Java type registration on a new thread failed"); } [Test] diff --git a/tests/Mono.Android-Tests/jni/reuse-threads.c b/tests/Mono.Android-Tests/jni/reuse-threads.c index f28cd10f30f..0dcdfe2f598 100644 --- a/tests/Mono.Android-Tests/jni/reuse-threads.c +++ b/tests/Mono.Android-Tests/jni/reuse-threads.c @@ -200,17 +200,28 @@ rt_invoke_callback_on_new_thread (CB cb) return 0; } -static void* -_register_type_from_new_thread (const char *java_type_name) +static int +_register_type_from_new_thread (const char *java_type_name, jobject class_loader) { - JNIEnv *env = _get_env ("_register_type_from_new_thread"); - jobject instance = _create_java_instance (env, java_type_name); + JNIEnv *env = _get_env ("_register_type_from_new_thread"); + jclass ClassLoader_class = (*env)->FindClass (env, "java/lang/ClassLoader"); + jmethodID loadClass = (*env)->GetMethodID (env, ClassLoader_class, "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;"); + jobject loaded_class = (*env)->CallObjectMethod (env, class_loader, loadClass, java_type_name); + + if ((*env)->ExceptionOccurred (env) != NULL) { + __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: class '%s' cannot be loaded, Java exception thrown!", java_type_name); + return -1; + } - if (instance == NULL) { + jmethodID Object_ctor = (*env)->GetMethodID (env, loaded_class, "", "()V"); + 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!", java_type_name); + return -1; } - return NULL; + return 0; } JNIEXPORT int JNICALL From 59a7d8e7dea03a0421de75fbb16bd7d41819f510 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Fri, 18 Nov 2022 20:49:28 +0100 Subject: [PATCH 07/16] Improve the unit tests, address feedback --- .../Java.Interop/JnienvTest.cs | 2 +- tests/Mono.Android-Tests/jni/reuse-threads.c | 20 +++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs b/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs index 163484cc8c5..2e31674b11a 100644 --- a/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs +++ b/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs @@ -44,7 +44,7 @@ public void TestMyPaintColor () public void RegisterTypeOnNewThread () { Java.Lang.JavaSystem.LoadLibrary ("reuse-threads"); - int ret = rt_register_type_on_new_thread ("from/NewThread", Application.Context.ClassLoader.Handle); + int ret = rt_register_type_on_new_thread ("from.NewThread", Application.Context.ClassLoader.Handle); Assert.AreEqual (0, ret, "Java type registration on a new thread failed"); } diff --git a/tests/Mono.Android-Tests/jni/reuse-threads.c b/tests/Mono.Android-Tests/jni/reuse-threads.c index 0dcdfe2f598..7dff29431ea 100644 --- a/tests/Mono.Android-Tests/jni/reuse-threads.c +++ b/tests/Mono.Android-Tests/jni/reuse-threads.c @@ -200,6 +200,8 @@ 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 (const char *java_type_name, jobject class_loader) { @@ -209,16 +211,18 @@ _register_type_from_new_thread (const char *java_type_name, jobject class_loader jobject loaded_class = (*env)->CallObjectMethod (env, class_loader, loadClass, java_type_name); if ((*env)->ExceptionOccurred (env) != NULL) { + (*env)->ExceptionClear (env); __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: class '%s' cannot be loaded, Java exception thrown!", java_type_name); - return -1; + return -2; } jmethodID Object_ctor = (*env)->GetMethodID (env, loaded_class, "", "()V"); jobject instance = (*env)->NewObject (env, loaded_class, Object_ctor); if ((*env)->ExceptionOccurred (env) != NULL || instance == NULL) { + (*env)->ExceptionClear (env); __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: instance of class '%s' wasn't created!", java_type_name); - return -1; + return -2; } return 0; @@ -239,7 +243,15 @@ rt_register_type_on_new_thread (const char *java_type_name) } void *tr; - pthread_join (t, &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 -1; + } - return 0; + if ((int)tr == -1 /* PTHREAD_CANCELED - not defined in bionic */) { + __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "RegisterOnNewThread: worker thread was canceled"); + return -1; + } + + return (int)tr; } From d969eed5460b20dbe253f968253832c43f698c0c Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Fri, 18 Nov 2022 20:51:37 +0100 Subject: [PATCH 08/16] Use unique return values to indicate where it failed --- tests/Mono.Android-Tests/jni/reuse-threads.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/Mono.Android-Tests/jni/reuse-threads.c b/tests/Mono.Android-Tests/jni/reuse-threads.c index 7dff29431ea..7151e76df07 100644 --- a/tests/Mono.Android-Tests/jni/reuse-threads.c +++ b/tests/Mono.Android-Tests/jni/reuse-threads.c @@ -213,7 +213,7 @@ _register_type_from_new_thread (const char *java_type_name, jobject class_loader if ((*env)->ExceptionOccurred (env) != NULL) { (*env)->ExceptionClear (env); __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: class '%s' cannot be loaded, Java exception thrown!", java_type_name); - return -2; + return -100; } jmethodID Object_ctor = (*env)->GetMethodID (env, loaded_class, "", "()V"); @@ -222,7 +222,7 @@ _register_type_from_new_thread (const char *java_type_name, jobject class_loader if ((*env)->ExceptionOccurred (env) != NULL || instance == NULL) { (*env)->ExceptionClear (env); __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: instance of class '%s' wasn't created!", java_type_name); - return -2; + return -101; } return 0; @@ -239,18 +239,18 @@ rt_register_type_on_new_thread (const char *java_type_name) if (r) { __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "RegisterOnNewThread: pthread_create() failed! %i: %s", r, strerror (r)); - return -1; + 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 -1; + 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 -1; + return -202; } return (int)tr; From 0b953e9c2b3d1039c58ba5ab34189df4716330be Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Fri, 18 Nov 2022 21:04:31 +0100 Subject: [PATCH 09/16] Remove a comment that's not really useful here --- tests/Mono.Android-Tests/jni/reuse-threads.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Mono.Android-Tests/jni/reuse-threads.c b/tests/Mono.Android-Tests/jni/reuse-threads.c index 7151e76df07..71e96bc1890 100644 --- a/tests/Mono.Android-Tests/jni/reuse-threads.c +++ b/tests/Mono.Android-Tests/jni/reuse-threads.c @@ -234,7 +234,6 @@ rt_register_type_on_new_thread (const char *java_type_name) JNIEnv *env = _get_env ("rt_register_type_on_new_thread"); pthread_t t; - /* 1: Create a thread... */ int r = pthread_create (&t, NULL, _register_type_from_new_thread, java_type_name); if (r) { From 3a8b10ad4e7e7e61a3478016d8efac7a4d980f0e Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 21 Nov 2022 18:44:22 +0100 Subject: [PATCH 10/16] Fix SNAFUs, doh --- tests/Mono.Android-Tests/jni/reuse-threads.c | 32 +++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/tests/Mono.Android-Tests/jni/reuse-threads.c b/tests/Mono.Android-Tests/jni/reuse-threads.c index 71e96bc1890..c7aea58c380 100644 --- a/tests/Mono.Android-Tests/jni/reuse-threads.c +++ b/tests/Mono.Android-Tests/jni/reuse-threads.c @@ -81,6 +81,12 @@ #include #include +typedef struct +{ + const char *java_type_name; + jobject class_loader; +} RegisterFromThreadContext; + typedef void (*CB)(JNIEnv *env, jobject self); static JavaVM *gvm; @@ -203,17 +209,23 @@ rt_invoke_callback_on_new_thread (CB cb) /* 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 (const char *java_type_name, jobject class_loader) +_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"); jclass ClassLoader_class = (*env)->FindClass (env, "java/lang/ClassLoader"); jmethodID loadClass = (*env)->GetMethodID (env, ClassLoader_class, "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;"); - jobject loaded_class = (*env)->CallObjectMethod (env, class_loader, loadClass, java_type_name); + jobject loaded_class = (*env)->CallObjectMethod (env, context->class_loader, loadClass, context->java_type_name); if ((*env)->ExceptionOccurred (env) != NULL) { (*env)->ExceptionClear (env); - __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: class '%s' cannot be loaded, Java exception thrown!", java_type_name); - return -100; + __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: class '%s' cannot be loaded, Java exception thrown!", context->java_type_name); + return -101; } jmethodID Object_ctor = (*env)->GetMethodID (env, loaded_class, "", "()V"); @@ -221,20 +233,24 @@ _register_type_from_new_thread (const char *java_type_name, jobject class_loader if ((*env)->ExceptionOccurred (env) != NULL || instance == NULL) { (*env)->ExceptionClear (env); - __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: instance of class '%s' wasn't created!", java_type_name); - return -101; + __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: instance of class '%s' wasn't created!", context->java_type_name); + return -102; } return 0; } JNIEXPORT int JNICALL -rt_register_type_on_new_thread (const char *java_type_name) +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, java_type_name); + 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)); From 22e437586a027211f4f1db5d05e2bee8a8d7d43a Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 21 Nov 2022 21:23:44 +0100 Subject: [PATCH 11/16] Address feedback --- tests/Mono.Android-Tests/jni/reuse-threads.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/Mono.Android-Tests/jni/reuse-threads.c b/tests/Mono.Android-Tests/jni/reuse-threads.c index c7aea58c380..abd8162e786 100644 --- a/tests/Mono.Android-Tests/jni/reuse-threads.c +++ b/tests/Mono.Android-Tests/jni/reuse-threads.c @@ -217,26 +217,35 @@ _register_type_from_new_thread (void *data) return -100; } + int ret = 0; JNIEnv *env = _get_env ("_register_type_from_new_thread"); jclass ClassLoader_class = (*env)->FindClass (env, "java/lang/ClassLoader"); jmethodID loadClass = (*env)->GetMethodID (env, ClassLoader_class, "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;"); jobject loaded_class = (*env)->CallObjectMethod (env, context->class_loader, loadClass, context->java_type_name); + jobject instance = NULL; if ((*env)->ExceptionOccurred (env) != NULL) { (*env)->ExceptionClear (env); __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: class '%s' cannot be loaded, Java exception thrown!", context->java_type_name); - return -101; + (*env)->ExceptionDescribe (env); + ret = -101; + goto cleanup; } - jmethodID Object_ctor = (*env)->GetMethodID (env, loaded_class, "", "()V"); - jobject instance = (*env)->NewObject (env, loaded_class, Object_ctor); + jmethodID Object_ctor = (*env)->GetMethodID (env, loaded_class, "", "()V"); + instance = (*env)->NewObject (env, loaded_class, Object_ctor); if ((*env)->ExceptionOccurred (env) != NULL || instance == NULL) { (*env)->ExceptionClear (env); __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: instance of class '%s' wasn't created!", context->java_type_name); - return -102; + (*env)->ExceptionDescribe (env); + ret = -102; } + cleanup: + (*env)->DeleteLocalRef (env, ClassLoader_class); + (*env)->DeleteLocalRef (env, loaded_class); + (*env)->DeleteLocalRef (env, instance); return 0; } From 3a97cfeed9c65797737000bd25f2a8bc84c32a14 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 21 Nov 2022 21:39:49 +0100 Subject: [PATCH 12/16] lref shortcut --- tests/Mono.Android-Tests/jni/reuse-threads.c | 23 +++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/tests/Mono.Android-Tests/jni/reuse-threads.c b/tests/Mono.Android-Tests/jni/reuse-threads.c index abd8162e786..c4d96083401 100644 --- a/tests/Mono.Android-Tests/jni/reuse-threads.c +++ b/tests/Mono.Android-Tests/jni/reuse-threads.c @@ -217,8 +217,20 @@ _register_type_from_new_thread (void *data) return -100; } + JNIEnv *env = _get_env ("_register_type_from_new_thread"); + + if ((*env)->PushLocalFrame (env, 3) < 0) { + __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: unable to create a local reference frame!"); + + if ((*env)->ExceptionOccurred (env)) { + (*env)->ExceptionClear (env); + (*env)->ExceptionDescribe (env); + } + + return -101; + } + int ret = 0; - JNIEnv *env = _get_env ("_register_type_from_new_thread"); jclass ClassLoader_class = (*env)->FindClass (env, "java/lang/ClassLoader"); jmethodID loadClass = (*env)->GetMethodID (env, ClassLoader_class, "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;"); jobject loaded_class = (*env)->CallObjectMethod (env, context->class_loader, loadClass, context->java_type_name); @@ -228,7 +240,7 @@ _register_type_from_new_thread (void *data) (*env)->ExceptionClear (env); __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: class '%s' cannot be loaded, Java exception thrown!", context->java_type_name); (*env)->ExceptionDescribe (env); - ret = -101; + ret = -102; goto cleanup; } @@ -239,13 +251,12 @@ _register_type_from_new_thread (void *data) (*env)->ExceptionClear (env); __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: instance of class '%s' wasn't created!", context->java_type_name); (*env)->ExceptionDescribe (env); - ret = -102; + ret = -103; } cleanup: - (*env)->DeleteLocalRef (env, ClassLoader_class); - (*env)->DeleteLocalRef (env, loaded_class); - (*env)->DeleteLocalRef (env, instance); + (*env)->PopLocalFrame (env, NULL); + return 0; } From 74f57767666f50f4a124a08dcb1243fbb602a727 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 22 Nov 2022 12:09:54 +0100 Subject: [PATCH 13/16] [WIP] Add more logging It won't fix the failure, but extra logging might be useful at some point. The current failure is: droid.NET_Test: java_vm_ext.cc:570] JNI DETECTED ERROR IN APPLICATION: use of invalid jobject 0x7100941bb830 backtrace: #00 pc 00000000000943f8 /apex/com.android.runtime/lib64/bionic/libc.so (syscall+24) (BuildId: a08a19770d6696739c847e29c3f5f650) #01 pc 0000000000097146 /apex/com.android.runtime/lib64/bionic/libc.so (abort+182) (BuildId: a08a19770d6696739c847e29c3f5f650) #02 pc 000000000055321f /apex/com.android.runtime/lib64/libart.so (art::Runtime::Abort(char const*)+2399) (BuildId: 8bb3225e7c408f2ca23abac3db0417f2) #03 pc 000000000000c873 /system/lib64/libbase.so (android::base::LogMessage::~LogMessage()+611) (BuildId: 40d2b536dbf0730fdc31abd2b469f94f) #04 pc 00000000003ede64 /apex/com.android.runtime/lib64/libart.so (art::JavaVMExt::JniAbort(char const*, char const*)+1604) (BuildId: 8bb3225e7c408f2ca23abac3db0417f2) #05 pc 00000000003ee16a /apex/com.android.runtime/lib64/libart.so (art::JavaVMExt::JniAbortF(char const*, char const*, ...)+234) (BuildId: 8bb3225e7c408f2ca23abac3db0417f2) #06 pc 00000000005adf7b /apex/com.android.runtime/lib64/libart.so (art::Thread::DecodeJObject(_jobject*) const+875) (BuildId: 8bb3225e7c408f2ca23abac3db0417f2) #07 pc 00000000003def7b /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::CheckInstance(art::ScopedObjectAccess&, art::(anonymous namespace)::ScopedCheck::InstanceKind, _jobject*, bool)+91) (BuildId: 8bb3225e7c408f2ca23abac3db0417f2) #08 pc 00000000003de1e5 /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::CheckPossibleHeapValue(art::ScopedObjectAccess&, char, art::(anonymous namespace)::JniValueType)+485) (BuildId: 8bb3225e7c408f2ca23abac3db0417f2) #09 pc 00000000003de2d4 /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::CheckPossibleHeapValue(art::ScopedObjectAccess&, char, art::(anonymous namespace)::JniValueType)+724) (BuildId: 8bb3225e7c408f2ca23abac3db0417f2) #10 pc 00000000003dd712 /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::ScopedCheck::Check(art::ScopedObjectAccess&, bool, char const*, art::(anonymous namespace)::JniValueType*)+690) (BuildId: 8bb3225e7c408f2ca23abac3db0417f2) #11 pc 00000000003e28c0 /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::CheckJNI::CheckCallArgs(art::ScopedObjectAccess&, art::(anonymous namespace)::ScopedCheck&, _JNIEnv*, _jobject*, _jclass*, _jmethodID*, art::InvokeType, art::(anonymous namespace)::VarArgs const*)+160) (BuildId: 8bb3225e7c408f2ca23abac3db0417f2) #12 pc 00000000003e1a9e /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::CheckJNI::CallMethodV(char const*, _JNIEnv*, _jobject*, _jclass*, _jmethodID*, __va_list_tag*, art::Primitive::Type, art::InvokeType)+910) (BuildId: 8bb3225e7c408f2ca23abac3db0417f2) #13 pc 00000000003cf551 /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::CheckJNI::CallObjectMethod(_JNIEnv*, _jobject*, _jmethodID*, ...)+177) (BuildId: 8bb3225e7c408f2ca23abac3db0417f2) #14 pc 00000000000013ec /data/app/Mono.Android.NET_Tests--O9vgexkYeCx3nX-AuvLTQ==/split_config.x86_64.apk!libreuse-threads.so (offset 0xa8d000) (BuildId: 562d86d81ebdd3bb6b7528e2a9235ff84827294e) #15 pc 0000000000100fce /apex/com.android.runtime/lib64/bionic/libc.so (__pthread_start(void*)+30) (BuildId: a08a19770d6696739c847e29c3f5f650) #16 pc 0000000000098fe7 /apex/com.android.runtime/lib64/bionic/libc.so (__start_thread+55) (BuildId: a08a19770d6696739c847e29c3f5f650) --- .../Java.Interop/JnienvTest.cs | 2 +- tests/Mono.Android-Tests/jni/reuse-threads.c | 42 +++++++++++++++---- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs b/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs index 2e31674b11a..e573fe3182a 100644 --- a/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs +++ b/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs @@ -45,7 +45,7 @@ public void RegisterTypeOnNewThread () { Java.Lang.JavaSystem.LoadLibrary ("reuse-threads"); int ret = rt_register_type_on_new_thread ("from.NewThread", Application.Context.ClassLoader.Handle); - Assert.AreEqual (0, ret, "Java type registration on a new thread failed"); + Assert.AreEqual (0, ret, $"Java type registration on a new thread failed with code {ret}"); } [Test] diff --git a/tests/Mono.Android-Tests/jni/reuse-threads.c b/tests/Mono.Android-Tests/jni/reuse-threads.c index c4d96083401..48a11a78add 100644 --- a/tests/Mono.Android-Tests/jni/reuse-threads.c +++ b/tests/Mono.Android-Tests/jni/reuse-threads.c @@ -232,32 +232,60 @@ _register_type_from_new_thread (void *data) int ret = 0; jclass ClassLoader_class = (*env)->FindClass (env, "java/lang/ClassLoader"); - jmethodID loadClass = (*env)->GetMethodID (env, ClassLoader_class, "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;"); - jobject loaded_class = (*env)->CallObjectMethod (env, context->class_loader, loadClass, context->java_type_name); - jobject instance = NULL; + 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; + } + + jobject loaded_class = (*env)->CallObjectMethod (env, context->class_loader, loadClass, context->java_type_name); if ((*env)->ExceptionOccurred (env) != NULL) { (*env)->ExceptionClear (env); __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: class '%s' cannot be loaded, Java exception thrown!", context->java_type_name); (*env)->ExceptionDescribe (env); - ret = -102; + 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, "", "()V"); - instance = (*env)->NewObject (env, loaded_class, Object_ctor); + 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) { (*env)->ExceptionClear (env); __android_log_print (ANDROID_LOG_INFO, "XA/RuntimeTest", "FAILURE: instance of class '%s' wasn't created!", context->java_type_name); (*env)->ExceptionDescribe (env); - ret = -103; + 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 0; + return ret; } JNIEXPORT int JNICALL From 8d901a2333f188fa3337888820df5bcb6dddd509 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 22 Nov 2022 18:05:23 +0100 Subject: [PATCH 14/16] Fix more snafus --- tests/Mono.Android-Tests/jni/reuse-threads.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/Mono.Android-Tests/jni/reuse-threads.c b/tests/Mono.Android-Tests/jni/reuse-threads.c index 48a11a78add..07223a60f37 100644 --- a/tests/Mono.Android-Tests/jni/reuse-threads.c +++ b/tests/Mono.Android-Tests/jni/reuse-threads.c @@ -219,18 +219,18 @@ _register_type_from_new_thread (void *data) JNIEnv *env = _get_env ("_register_type_from_new_thread"); - if ((*env)->PushLocalFrame (env, 3) < 0) { + 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)->ExceptionClear (env); (*env)->ExceptionDescribe (env); + (*env)->ExceptionClear (env); } return -101; } - int ret = 0; + int ret = 0; jclass ClassLoader_class = (*env)->FindClass (env, "java/lang/ClassLoader"); if (ClassLoader_class == NULL) { ret = -102; @@ -245,12 +245,13 @@ _register_type_from_new_thread (void *data) goto cleanup; } - jobject loaded_class = (*env)->CallObjectMethod (env, context->class_loader, loadClass, context->java_type_name); + 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) { - (*env)->ExceptionClear (env); __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; } @@ -271,9 +272,9 @@ _register_type_from_new_thread (void *data) jobject instance = (*env)->NewObject (env, loaded_class, Object_ctor); if ((*env)->ExceptionOccurred (env) != NULL || instance == NULL) { - (*env)->ExceptionClear (env); __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; } From 3dee77d975fff13893d4322015af1ec28ee39717 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 22 Nov 2022 23:27:04 +0100 Subject: [PATCH 15/16] Add a new test --- .../Java.Interop/JnienvTest.cs | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs b/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs index e573fe3182a..a1efbb26591 100644 --- a/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs +++ b/tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs @@ -41,13 +41,22 @@ public void TestMyPaintColor () static extern int rt_invoke_callback_on_new_thread (CB cb); [Test] - public void RegisterTypeOnNewThread () + public void RegisterTypeOnNewNativeThread () { Java.Lang.JavaSystem.LoadLibrary ("reuse-threads"); - int ret = rt_register_type_on_new_thread ("from.NewThread", Application.Context.ClassLoader.Handle); + 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 () { @@ -445,10 +454,24 @@ public void DoNotLeakWeakReferences () } } - [Register ("from/NewThread")] - class RegisterMeOnNewThread : Java.Lang.Object + [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 () { From 06db67d042e0447921abee09048902890fd6b3dc Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 28 Nov 2022 16:55:01 +0100 Subject: [PATCH 16/16] Re-enable the fix, we'll work on the test later --- src/monodroid/jni/monodroid-glue.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/monodroid/jni/monodroid-glue.cc b/src/monodroid/jni/monodroid-glue.cc index afeb87d92ac..7fb2a67dcb3 100644 --- a/src/monodroid/jni/monodroid-glue.cc +++ b/src/monodroid/jni/monodroid-glue.cc @@ -2537,7 +2537,7 @@ 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 + mono_jit_thread_attach (nullptr); // There's just one domain in .net #if !defined (ANDROID) mono_runtime_invoke (register_jni_natives, nullptr, args, nullptr);