[native, Mono.Android] Remove WeakReference support (#9454)

Context: dotnet/java-interop#1255
Context: https://github.com/xamarin/monodroid/commit/a5c52905293d7eecdb3c078da62100081b338045
Context: https://issuetracker.google.com/issues/36986478
Context: https://github.com/dotnet/android/blob/b56d68fe2a2c2dffa47eb7e41ae08a00fda86367/Documentation/workflow/SystemProperties.md#debugmonowref

Android did not support [JNI Weak Global References][0] until API-8
(Android 2.2).  To allow Mono for Android applications to run on
API-7 and earlier devices, while using JNI Weak Global References
when possible (fewer GC allocations, better performance), the GC
bridge supported two modes of operation:

  * `debug.mono.wref=jni`: Use JNI Weak Global References.
  * `debug.mono.wref=java`: Use [`java.lang.ref.WeakReference`][1].

With the exception of a ["minor" (lol) issue][2] in KitKat v4.4.2
(API-19) -- in which `JNIEnv::NewGlobalRef()` around a collected
JNI Weak Global Reference would return a non-`NULL` value, which
would result in app crashes; xamarin/monodroid@a5c52905 added
a workaround to use the `WeakReference` backend on Android 4.4.2 --
the `debug.mono.wref=java` backend has not been used since *2014*.

Apps *could* use [`@(AndroidEnvironment)`][3] to set
`debug.mono.wref=java`, but there is no reason to do so (worse
performance!), [nor has anyone done so on github.com][4]
(no matches for `debug.mono.wref` within files that would be used
with `@(AndroidEnvironment)`).

Cleanup src/native and src/Mono.Android, and *remove* support for the
`debug.mono.wref=java` backend.

[0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#weak_global_references
[1]: https://developer.android.com/reference/java/lang/ref/WeakReference
[2]: https://issuetracker.google.com/issues/36986478
[3]: https://learn.microsoft.com/dotnet/android/building-apps/build-items#androidenvironment
[4]: https://github.com/search?type=code&q=debug.mono.wref

Author: jonpryor

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

@@ -5,13 +5,11 @@
       <field name="handle" />
       <field name="handle_type" />
       <field name="refs_added" />
-      <field name="weak_handle" />
     </type>
     <type fullname="Java.Interop.JavaException">
       <field name="handle" />
       <field name="handle_type" />
       <field name="refs_added" />
-      <field name="weak_handle" />
     </type>
   </assembly>
 </linker>

src/Microsoft.Android.Sdk.ILLink/PreserveLists/Mono.Android.xml

@@ -39,14 +39,12 @@
 			<field name="handle" />
 			<field name="handle_type" />
 			<field name="refs_added" />
-			<field name="weak_handle" />
 			<method name="SetHandleOnDeserialized" />
 		</type>
 		<type fullname="Java.Lang.Throwable">
 			<field name="handle" />
 			<field name="handle_type" />
 			<field name="refs_added" />
-			<field name="weak_handle" />
 		</type>
 	</assembly>
 </linker>

src/Mono.Android/Java.Lang/Object.cs

@@ -21,7 +21,6 @@ public partial class Object : IDisposable, IJavaObject, IJavaObjectEx
 	{
 		[NonSerialized] IntPtr key_handle;
 #pragma warning disable CS0649, CS0169, CS0414 // Suppress fields are never used warnings, these fields are used directly by monodroid-glue.cc
-		[NonSerialized] IntPtr weak_handle;
 		[NonSerialized] int refs_added;
 #pragma warning restore CS0649, CS0169, CS0414
 		[NonSerialized] JObjectRefType handle_type;
@@ -117,9 +116,6 @@ public virtual JniPeerMembers JniPeerMembers {
 		[EditorBrowsable (EditorBrowsableState.Never)]
 		public IntPtr Handle {
 			get {
-				if (weak_handle != IntPtr.Zero)
-					Logger.Log (LogLevel.Warn, "Mono.Android.dll", "Accessing object which is out for collection via original handle");
-
 				return handle;
 			}
 		}

src/Mono.Android/Java.Lang/Throwable.cs

@@ -21,7 +21,6 @@ public partial class Throwable : global::System.Exception, IJavaObject, IDisposa
 		IntPtr           key_handle;
 		JObjectRefType   handle_type;
 #pragma warning disable CS0649, CS0169, CS0414 // Suppress fields are never used warnings, these fields are used directly by monodroid-glue.cc
-		IntPtr           weak_handle;
 		int              refs_added;
 #pragma warning restore CS0649, CS0169, CS0414
 

src/native/monodroid/monodroid-glue.cc

@@ -781,17 +781,15 @@ MonodroidRuntime::lookup_bridge_info (MonoClass *klass, const OSBridge::MonoJava
 	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"));
-	if (info->klass == nullptr || info->handle == nullptr || info->handle_type == nullptr || info->refs_added == nullptr || info->weak_handle == nullptr) {
+	if (info->klass == nullptr || info->handle == nullptr || info->handle_type == nullptr || info->refs_added == 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 weak_handle=%p",
+				"The type `%s.%s` is missing required instance fields! handle=%p handle_type=%p refs_added=%p",
 				type->_namespace,
 				type->_typename,
 				info->handle,
 				info->handle_type,
-				info->refs_added,
-				info->weak_handle
+				info->refs_added
 			)
 		);
 	}

src/native/monodroid/osbridge.cc

@@ -47,7 +47,6 @@ OSBridge::MonoJavaGCBridgeInfo OSBridge::empty_bridge_info = {
 	nullptr,
 	nullptr,
 	nullptr,
-	nullptr,
 	nullptr
 };
 
@@ -81,7 +80,6 @@ OSBridge::clear_mono_java_gc_bridge_info ()
 		info->handle = nullptr;
 		info->handle_type = nullptr;
 		info->refs_added = nullptr;
-		info->weak_handle = nullptr;
 	}
 }
 
@@ -443,67 +441,6 @@ OSBridge::monodroid_disable_gc_hooks ()
 	gc_disabled = 1;
 }
 
-mono_bool
-OSBridge::take_global_ref_2_1_compat (JNIEnv *env, MonoObject *obj)
-{
-	jobject handle, weak;
-	int type = JNIGlobalRefType;
-
-	MonoJavaGCBridgeInfo    *bridge_info    = get_gc_bridge_info_for_object (obj);
-	if (bridge_info == nullptr)
-		return 0;
-
-	mono_field_get_value (obj, bridge_info->weak_handle, &weak);
-	handle = env->CallObjectMethod (weak, weakrefGet);
-	if (gref_log) {
-		fprintf (gref_log, "*try_take_global_2_1 obj=%p -> wref=%p handle=%p\n", obj, weak, handle);
-		fflush (gref_log);
-	}
-	if (handle) {
-		void* h = env->NewGlobalRef (handle);
-		env->DeleteLocalRef (handle);
-		handle = reinterpret_cast <jobject> (h);
-		_monodroid_gref_log_new (weak, get_object_ref_type (env, weak),
-		                         handle, get_object_ref_type (env, handle), "finalizer", gettid (), __PRETTY_FUNCTION__, 0);
-	}
-	_monodroid_weak_gref_delete (weak, get_object_ref_type (env, weak), "finalizer", gettid(), __PRETTY_FUNCTION__, 0);
-	env->DeleteGlobalRef (weak);
-	weak = nullptr;
-	mono_field_set_value (obj, bridge_info->weak_handle, &weak);
-
-	mono_field_set_value (obj, bridge_info->handle, &handle);
-	mono_field_set_value (obj, bridge_info->handle_type, &type);
-	return handle != nullptr;
-}
-
-mono_bool
-OSBridge::take_weak_global_ref_2_1_compat (JNIEnv *env, MonoObject *obj)
-{
-	jobject weaklocal;
-	jobject handle, weakglobal;
-
-	MonoJavaGCBridgeInfo    *bridge_info    = get_gc_bridge_info_for_object (obj);
-	if (bridge_info == nullptr)
-		return 0;
-
-	mono_field_get_value (obj, bridge_info->handle, &handle);
-	weaklocal = env->NewObject (weakrefClass, weakrefCtor, handle);
-	weakglobal = env->NewGlobalRef (weaklocal);
-	env->DeleteLocalRef (weaklocal);
-	if (gref_log) {
-		fprintf (gref_log, "*take_weak_2_1 obj=%p -> wref=%p handle=%p\n", obj, weakglobal, handle);
-		fflush (gref_log);
-	}
-	_monodroid_weak_gref_new (handle, get_object_ref_type (env, handle),
-	                          weakglobal, get_object_ref_type (env, weakglobal), "finalizer", gettid (), __PRETTY_FUNCTION__, 0);
-
-	_monodroid_gref_log_delete (handle, get_object_ref_type (env, handle), "finalizer", gettid (), __PRETTY_FUNCTION__, 0);
-
-	env->DeleteGlobalRef (handle);
-	mono_field_set_value (obj, bridge_info->weak_handle, &weakglobal);
-	return 1;
-}
-
 mono_bool
 OSBridge::take_global_ref_jni (JNIEnv *env, MonoObject *obj)
 {
@@ -1001,58 +938,13 @@ OSBridge::gc_cross_references (int num_sccs, MonoGCBridgeSCC **sccs, int num_xre
 	set_bridge_processing_field (domains_list, 0);
 }
 
-int
-OSBridge::platform_supports_weak_refs (void)
-{
-	char *value;
-	int api_level = 0;
-
-	if (AndroidSystem::monodroid_get_system_property ("ro.build.version.sdk", &value) > 0) {
-		api_level = atoi (value);
-		free (value);
-	}
-
-	if (AndroidSystem::monodroid_get_system_property (SharedConstants::DEBUG_MONO_WREF_PROPERTY, &value) > 0) {
-		int use_weak_refs = 0;
-		if (!strcmp ("jni", value))
-			use_weak_refs = 1;
-		else if (!strcmp ("java", value))
-			use_weak_refs = 0;
-		else {
-			use_weak_refs = -1;
-			log_warn (LOG_GC, "Unsupported debug.mono.wref value '%s'; "
-					"supported values are 'jni' and 'java'. Ignoring...",
-					value);
-		}
-		free (value);
-
-		if (use_weak_refs && api_level < 8)
-			log_warn (LOG_GC, "Using JNI weak references instead of "
-					"java.lang.WeakReference on API-%i. Are you sure you want to do this? "
-					"The GC may be compromised.",
-					api_level);
-
-		if (use_weak_refs >= 0)
-			return use_weak_refs;
-	}
-
-	return 1;
-}
-
 void
 OSBridge::register_gc_hooks (void)
 {
 	MonoGCBridgeCallbacks bridge_cbs;
 
-	if (platform_supports_weak_refs ()) {
-		take_global_ref = &OSBridge::take_global_ref_jni;
-		take_weak_global_ref = &OSBridge::take_weak_global_ref_jni;
-		log_info (LOG_GC, "environment supports jni NewWeakGlobalRef");
-	} else {
-		take_global_ref = &OSBridge::take_global_ref_2_1_compat;
-		take_weak_global_ref = &OSBridge::take_weak_global_ref_2_1_compat;
-		log_info (LOG_GC, "environment does not support jni NewWeakGlobalRef");
-	}
+	take_global_ref = &OSBridge::take_global_ref_jni;
+	take_weak_global_ref = &OSBridge::take_weak_global_ref_jni;
 
 	bridge_cbs.bridge_version = SGEN_BRIDGE_VERSION;
 	bridge_cbs.bridge_class_kind = gc_bridge_class_kind_cb;

src/native/monodroid/osbridge.hh

@@ -40,7 +40,6 @@ namespace xamarin::android::internal
 			MonoClassField  *handle;
 			MonoClassField  *handle_type;
 			MonoClassField  *refs_added;
-			MonoClassField  *weak_handle;
 		};
 
 		// add_reference can work with objects which are either MonoObjects with java peers, or raw jobjects
@@ -134,8 +133,6 @@ namespace xamarin::android::internal
 		int _monodroid_gref_dec ();
 		char* _get_stack_trace_line_end (char *m);
 		void _write_stack_trace (FILE *to, char *from, LogCategories = LOG_NONE);
-		mono_bool take_global_ref_2_1_compat (JNIEnv *env, MonoObject *obj);
-		mono_bool take_weak_global_ref_2_1_compat (JNIEnv *env, MonoObject *obj);
 		mono_bool take_global_ref_jni (JNIEnv *env, MonoObject *obj);
 		mono_bool take_weak_global_ref_jni (JNIEnv *env, MonoObject *obj);
 		mono_bool add_reference_jobject (JNIEnv *env, jobject handle, jobject reffed_handle);
@@ -152,7 +149,6 @@ namespace xamarin::android::internal
 		void gc_cleanup_after_java_collection (JNIEnv *env, int num_sccs, MonoGCBridgeSCC **sccs);
 		void java_gc (JNIEnv *env);
 		void set_bridge_processing_field (MonodroidBridgeProcessingInfo *list, mono_bool value);
-		int platform_supports_weak_refs ();
 
 #if DEBUG
 		char* describe_target (AddReferenceTarget target);