[Mono.Android] JNI handles are now in a "control block" (#10179)

Originally from: #10125
Context: dotnet/java-interop#1339

These are changes to Mono's GC bridge implementation, such that the
same code can be used on both Mono and CoreCLR.

The base `JavaObject` type has changes such as:

```diff
--[NonSerialized] IntPtr                  handle;
--[NonSerialized] JniObjectReferenceType  handle_type;
--[NonSerialized] IntPtr                  weak_handle;
--[NonSerialized] int                     refs_added;
++unsafe  JniObjectReferenceControlBlock* jniObjectReferenceControlBlock;
```

And the new `JniObjectReferenceControlBlock` struct is defined as:

    internal struct JniObjectReferenceControlBlock {
        public	IntPtr  handle;
        public  int     handle_type;
        public  IntPtr  weak_handle;
        public  int     refs_added;
    }

* Each `JavaObject` allocation now has a `NativeMemory.AllocZeroed()`
  call for this struct.

* However, we only have to read a single field now using Mono's native
  embedding API:

```diff
--info->handle            = mono_class_get_field_from_name (info->klass, const_cast<char*> ("handle"));
--info->handle_type       = mono_class_get_field_from_name (info->klass, const_cast<char*> ("handle_type"));
--info->refs_added        = mono_class_get_field_from_name (info->klass, const_cast<char*> ("refs_added"));
--info->weak_handle       = mono_class_get_field_from_name (info->klass, const_cast<char*> ("weak_handle"));
++info->jniObjectReferenceControlBlock    = mono_class_get_field_from_name (info->klass, const_cast<char*> ("jniObjectReferenceControlBlock"));
```

We are hoping this results in ~the same performance as before, with
the flexibility to support multiple GC bridge implementations and
runtimes.

Other changes:

* Fix SIGSEGV when using the new control block

We were using it incorrectly - there's no need to fetch/set values of managed object fields.
The correct thing to do is to read/write them on the control block itself.

Co-authored-by: Marek Habersack <[email protected]>

Author: jonathanpeppers

external/Java.Interop

@@ -2,14 +2,10 @@
 <linker>
   <assembly fullname="Java.Interop">
     <type fullname="Java.Interop.JavaObject">
-      <field name="handle" />
-      <field name="handle_type" />
-      <field name="refs_added" />
+      <field name="jniObjectReferenceControlBlock" />
     </type>
     <type fullname="Java.Interop.JavaException">
-      <field name="handle" />
-      <field name="handle_type" />
-      <field name="refs_added" />
+      <field name="jniObjectReferenceControlBlock" />
     </type>
   </assembly>
 </linker>

src/Microsoft.Android.Sdk.ILLink/PreserveLists/Java.Interop.xml

@@ -793,22 +793,15 @@ MonodroidRuntime::create_domain (JNIEnv *env, jstring_array_wrapper &runtimeApks
 MonodroidRuntime::lookup_bridge_info (MonoClass *klass, const OSBridge::MonoJavaGCBridgeType *type, OSBridge::MonoJavaGCBridgeInfo *info) noexcept
 {
 	info->klass             = klass;
-	info->handle            = mono_class_get_field_from_name (info->klass, const_cast<char*> ("handle"));
-	info->handle_type       = mono_class_get_field_from_name (info->klass, const_cast<char*> ("handle_type"));
-	info->refs_added        = mono_class_get_field_from_name (info->klass, const_cast<char*> ("refs_added"));
-	info->key_handle        = mono_class_get_field_from_name (info->klass, const_cast<char*> ("key_handle"));
+	info->jniObjectReferenceControlBlock = mono_class_get_field_from_name (info->klass, const_cast<char*>("jniObjectReferenceControlBlock"));
 
-	// key_handle is optional, as Java.Interop.JavaObject doesn't currently have it
-	if (info->klass == nullptr || info->handle == nullptr || info->handle_type == nullptr || info->refs_added == nullptr) {
+	if (info->klass == nullptr || info->jniObjectReferenceControlBlock == nullptr) {
 		Helpers::abort_application (
-			Util::monodroid_strdup_printf (
-				"The type `%s.%s` is missing required instance fields! handle=%p handle_type=%p refs_added=%p key_handle=%p",
+			std::format (
+				"The type `{}.{} is missing required instance fields! jniObjectReferenceControlBlock={:p}",
 				type->_namespace,
 				type->_typename,
-				info->handle,
-				info->handle_type,
-				info->refs_added,
-				info->key_handle
+				static_cast<void*>(info->jniObjectReferenceControlBlock)
 			)
 		);
 	}

src/native/mono/monodroid/monodroid-glue.cc

@@ -42,9 +42,6 @@ const uint32_t OSBridge::NUM_GC_BRIDGE_TYPES    = NUM_XA_GC_BRIDGE_TYPES + NUM_J
 OSBridge::MonoJavaGCBridgeInfo OSBridge::mono_java_gc_bridge_info [NUM_GC_BRIDGE_TYPES];
 
 OSBridge::MonoJavaGCBridgeInfo OSBridge::empty_bridge_info = {
-	nullptr,
-	nullptr,
-	nullptr,
 	nullptr,
 	nullptr
 };
@@ -76,9 +73,14 @@ OSBridge::clear_mono_java_gc_bridge_info ()
 	for (uint32_t c = 0; c < NUM_GC_BRIDGE_TYPES; c++) {
 		MonoJavaGCBridgeInfo *info = &mono_java_gc_bridge_info [c];
 		info->klass = nullptr;
-		info->handle = nullptr;
-		info->handle_type = nullptr;
-		info->refs_added = nullptr;
+		auto control_block = reinterpret_cast<JniObjectReferenceControlBlock*>(info->jniObjectReferenceControlBlock);
+		if (control_block == nullptr) [[unlikely]] {
+			continue;
+		}
+		control_block->handle = nullptr;
+		control_block->handle_type = 0;
+		control_block->weak_handle = nullptr;
+		control_block->refs_added = 0;
 	}
 }
 
@@ -102,6 +104,19 @@ OSBridge::get_gc_bridge_index (MonoClass *klass)
 		: -1;
 }
 
+OSBridge::JniObjectReferenceControlBlock*
+OSBridge::get_gc_control_block_for_object (MonoObject *obj)
+{
+	MonoJavaGCBridgeInfo *bridge_info = get_gc_bridge_info_for_object (obj);
+	if (bridge_info == nullptr) {
+		return nullptr;
+	}
+
+	JniObjectReferenceControlBlock *control_block = nullptr;
+	mono_field_get_value (obj, bridge_info->jniObjectReferenceControlBlock, &control_block);
+	return control_block;
+}
+
 OSBridge::MonoJavaGCBridgeInfo *
 OSBridge::get_gc_bridge_info_for_class (MonoClass *klass)
 {
@@ -456,15 +471,15 @@ OSBridge::monodroid_disable_gc_hooks ()
 mono_bool
 OSBridge::take_global_ref_jni (JNIEnv *env, MonoObject *obj)
 {
-	jobject handle, weak;
 	int type = JNIGlobalRefType;
 
-	MonoJavaGCBridgeInfo    *bridge_info    = get_gc_bridge_info_for_object (obj);
-	if (bridge_info == nullptr)
+	JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (obj);
+	if (control_block == nullptr) {
 		return 0;
+	}
 
-	mono_field_get_value (obj, bridge_info->handle, &weak);
-	handle = env->NewGlobalRef (weak);
+	jobject weak = control_block->handle;
+	jobject handle = env->NewGlobalRef (weak);
 	if (gref_log) {
 		fprintf (gref_log, "*try_take_global obj=%p -> wref=%p handle=%p\n", obj, weak, handle);
 		fflush (gref_log);
@@ -476,8 +491,8 @@ OSBridge::take_global_ref_jni (JNIEnv *env, MonoObject *obj)
 				"   at [[gc:take_global_ref_jni]]", 0);
 	} else if (Logger::gc_spew_enabled ()) [[unlikely]] {
 		void   *key_handle  = nullptr;
-		if (bridge_info->key_handle) {
-			mono_field_get_value (obj, bridge_info->key_handle, &key_handle);
+		if (control_block->weak_handle != nullptr) {
+			key_handle = control_block->weak_handle;
 		}
 
 		MonoClass *klass = mono_object_get_class (obj);
@@ -491,8 +506,8 @@ OSBridge::take_global_ref_jni (JNIEnv *env, MonoObject *obj)
 		free (message);
 	}
 
-	mono_field_set_value (obj, bridge_info->handle, &handle);
-	mono_field_set_value (obj, bridge_info->handle_type, &type);
+	control_block->handle = handle;
+	control_block->handle_type = type;
 
 	_monodroid_weak_gref_delete (weak, get_object_ref_type (env, weak),
 			"finalizer", gettid (), "   at [[gc:take_global_ref_jni]]", 0);
@@ -504,26 +519,26 @@ OSBridge::take_global_ref_jni (JNIEnv *env, MonoObject *obj)
 mono_bool
 OSBridge::take_weak_global_ref_jni (JNIEnv *env, MonoObject *obj)
 {
-	jobject handle, weak;
 	int type = JNIWeakGlobalRefType;
 
-	MonoJavaGCBridgeInfo    *bridge_info    = get_gc_bridge_info_for_object (obj);
-	if (bridge_info == nullptr)
+	JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (obj);
+	if (control_block == nullptr) {
 		return 0;
+	}
 
-	mono_field_get_value (obj, bridge_info->handle, &handle);
+	jobject handle = control_block->handle;
 	if (gref_log) {
 		fprintf (gref_log, "*take_weak obj=%p; handle=%p\n", obj, handle);
 		fflush (gref_log);
 	}
 
-	weak = env->NewWeakGlobalRef (handle);
+	jobject weak = env->NewWeakGlobalRef (handle);
 	_monodroid_weak_gref_new (handle, get_object_ref_type (env, handle),
 			weak, get_object_ref_type (env, weak),
 			"finalizer", gettid (), "   at [[gc:take_weak_global_ref_jni]]", 0);
 
-	mono_field_set_value (obj, bridge_info->handle, &weak);
-	mono_field_set_value (obj, bridge_info->handle_type, &type);
+	control_block->handle = weak;
+	control_block->handle_type = type;
 
 	_monodroid_gref_log_delete (handle, get_object_ref_type (env, handle),
 			"finalizer", gettid (), "   at [[gc:take_weak_global_ref_jni]]", 0);
@@ -558,14 +573,22 @@ OSBridge::gc_bridge_class_kind (MonoClass *klass)
 mono_bool
 OSBridge::gc_is_bridge_object (MonoObject *object)
 {
-	void *handle;
+	if (object == nullptr) [[unlikely]] {
+		log_debug (LOG_GC, "gc_is_bridge_object was passed a NULL object pointer");
+		return FALSE;
+	}
 
-	MonoJavaGCBridgeInfo    *bridge_info    = get_gc_bridge_info_for_object (object);
-	if (bridge_info == nullptr)
-		return 0;
+	JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (object);
+	if (control_block == nullptr) {
+		return FALSE;
+	}
 
-	mono_field_get_value (object, bridge_info->handle, &handle);
-	if (handle == nullptr) {
+	if (control_block->handle == nullptr) {
+		log_warn (LOG_GC, "gc_is_bridge_object: control block's handle is NULL");
+		return FALSE;
+	}
+
+	if (control_block->handle == nullptr) {
 #if DEBUG
 		MonoClass *mclass = mono_object_get_class (object);
 		log_info (LOG_GC,
@@ -574,10 +597,10 @@ OSBridge::gc_is_bridge_object (MonoObject *object)
 			optional_string (mono_class_get_name (mclass))
 		);
 #endif
-		return 0;
+		return FALSE;
 	}
 
-	return 1;
+	return TRUE;
 }
 
 // Add a reference from an IGCUserPeer jobject to another jobject
@@ -603,11 +626,17 @@ OSBridge::add_reference_jobject (JNIEnv *env, jobject handle, jobject reffed_han
 mono_bool
 OSBridge::load_reference_target (OSBridge::AddReferenceTarget target, OSBridge::MonoJavaGCBridgeInfo** bridge_info, jobject *handle)
 {
+	if (handle == nullptr) [[unlikely]] {
+		return FALSE;
+	}
+
 	if (target.is_mono_object) {
-		*bridge_info = get_gc_bridge_info_for_object (target.obj);
-		if (!*bridge_info)
+		JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (target.obj);
+		if (control_block == nullptr) {
 			return FALSE;
-		mono_field_get_value (target.obj, (*bridge_info)->handle, handle);
+		}
+
+		*handle = control_block->handle;
 	} else {
 		*handle = target.jobj;
 	}
@@ -648,8 +677,11 @@ OSBridge::add_reference (JNIEnv *env, OSBridge::AddReferenceTarget target, OSBri
 	// Flag MonoObjects so they can be cleared in gc_cleanup_after_java_collection.
 	// Java temporaries do not need this because the entire GCUserPeer is discarded.
 	if (success && target.is_mono_object) {
-		int ref_val = 1;
-		mono_field_set_value (target.obj, bridge_info->refs_added, &ref_val);
+		JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (target.obj);
+		if (control_block == nullptr) {
+			return FALSE;
+		}
+		control_block->refs_added = 1;
 	}
 
 #if DEBUG
@@ -867,15 +899,15 @@ OSBridge::gc_cleanup_after_java_collection (JNIEnv *env, int num_sccs, MonoGCBri
 		sccs [i]->is_alive = 0;
 
 		for (j = 0; j < sccs [i]->num_objs; j++) {
-			MonoJavaGCBridgeInfo    *bridge_info;
-
 			obj = sccs [i]->objs [j];
 
-			bridge_info = get_gc_bridge_info_for_object (obj);
-			if (bridge_info == nullptr)
+			JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (obj);
+			if (control_block == nullptr) {
 				continue;
-			mono_field_get_value (obj, bridge_info->handle, &jref);
-			if (jref) {
+			}
+
+			jref = control_block->handle;
+			if (jref != nullptr) {
 				alive++;
 				if (j > 0) {
 					abort_unless (
@@ -884,8 +916,8 @@ OSBridge::gc_cleanup_after_java_collection (JNIEnv *env, int num_sccs, MonoGCBri
 					);
 				}
 				sccs [i]->is_alive = 1;
-				mono_field_get_value (obj, bridge_info->refs_added, &refs_added);
-				if (refs_added) {
+				refs_added = control_block->refs_added;
+				if (refs_added != 0) {
 					jclass java_class = env->GetObjectClass (jref);
 					clear_method_id = env->GetMethodID (java_class, "monodroidClearReferences", "()V");
 					env->DeleteLocalRef (java_class);
@@ -951,13 +983,13 @@ OSBridge::gc_cross_references (int num_sccs, MonoGCBridgeSCC **sccs, int num_xre
 			for (j = 0; j < sccs [i]->num_objs; ++j) {
 				MonoObject *obj = sccs [i]->objs [j];
 
-				MonoJavaGCBridgeInfo    *bridge_info    = get_gc_bridge_info_for_object (obj);
+				JniObjectReferenceControlBlock *control_block = get_gc_control_block_for_object (obj);
 				jobject handle      = 0;
 				void   *key_handle  = nullptr;
-				if (bridge_info != nullptr) {
-					mono_field_get_value (obj, bridge_info->handle, &handle);
-					if (bridge_info->key_handle != nullptr) {
-						mono_field_get_value (obj, bridge_info->key_handle, &key_handle);
+				if (control_block != nullptr) {
+					handle = control_block->handle;
+					if (control_block->weak_handle != nullptr) {
+						key_handle = control_block->weak_handle;
 					}
 				}
 				MonoClass *klass = mono_object_get_class (obj);

src/native/mono/monodroid/osbridge.cc

@@ -37,10 +37,15 @@ namespace xamarin::android::internal
 		struct MonoJavaGCBridgeInfo
 		{
 			MonoClass       *klass;
-			MonoClassField  *handle;
-			MonoClassField  *handle_type;
-			MonoClassField  *refs_added;
-			MonoClassField  *key_handle;
+			MonoClassField  *jniObjectReferenceControlBlock;
+		};
+
+		struct JniObjectReferenceControlBlock
+		{
+			jobject handle;
+			int     handle_type;
+			jobject weak_handle;
+			int     refs_added;
 		};
 
 		// add_reference can work with objects which are either MonoObjects with java peers, or raw jobjects
@@ -127,6 +132,7 @@ namespace xamarin::android::internal
 
 	private:
 		int get_gc_bridge_index (MonoClass *klass);
+		JniObjectReferenceControlBlock* get_gc_control_block_for_object (MonoObject *obj);
 		MonoJavaGCBridgeInfo* get_gc_bridge_info_for_class (MonoClass *klass);
 		MonoJavaGCBridgeInfo* get_gc_bridge_info_for_object (MonoObject *object);
 		char get_object_ref_type (JNIEnv *env, void *handle);