Manage JClass references more robustly

FindClass just returns a local reference, but we want to keep this around for the entire lifetime of the program. The "correct" way to do this appears to be to make it a global reference. In practice the local reference seems to stay around forever, so not much actually changes here.
CasualPokePlayer · Apr 15, 2024 · e69a2ca · e69a2ca
1 parent e19ddde
commit e69a2ca
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 11 deletions.
diff --git a/GSR.Android/AndroidCryptography.cs b/GSR.Android/AndroidCryptography.cs
@@ -19,9 +19,9 @@ public static class AndroidCryptography
 	private static JMethodID _hashDataSha256MethodId;
 	private static JMethodID _getRandomInt32MethodId;
 
-	internal static void InitializeJNI(JNIEnvPtr env)
+	internal static void InitializeJNI(JNIEnvPtr env, JClass gsrActivityClassId)
 	{
-		_gsrActivityClassId = env.FindClass("org/psr/gsr/GSRActivity"u8);
+		_gsrActivityClassId = gsrActivityClassId;
 		_hashDataSha256MethodId = env.GetStaticMethodID(_gsrActivityClassId, "HashDataSHA256"u8, "(Ljava/nio/ByteBuffer;)[B"u8);
 		_getRandomInt32MethodId = env.GetStaticMethodID(_gsrActivityClassId, "GetRandomInt32"u8, "(I)I"u8);
 	}

diff --git a/GSR.Android/AndroidFile.cs b/GSR.Android/AndroidFile.cs
@@ -21,9 +21,9 @@ public static class AndroidFile
 	private static JMethodID _requestDocumentMethodId;
 	private static JMethodID _openContentMethodId;
 
-	internal static void InitializeJNI(JNIEnvPtr env)
+	internal static void InitializeJNI(JNIEnvPtr env, JClass gsrActivityClassId)
 	{
-		_gsrActivityClassId = env.FindClass("org/psr/gsr/GSRActivity"u8);
+		_gsrActivityClassId = gsrActivityClassId;
 		_requestDocumentMethodId = env.GetStaticMethodID(_gsrActivityClassId, "RequestDocument"u8, "()V"u8);
 		_openContentMethodId = env.GetStaticMethodID(_gsrActivityClassId, "OpenContent"u8, "(Ljava/lang/String;)I"u8);
 	}

diff --git a/GSR.Android/AndroidJNI.cs b/GSR.Android/AndroidJNI.cs
@@ -13,13 +13,16 @@ namespace GSR.Android;
 /// <summary>
 /// Handles Android JNI initialization
 /// </summary>
-public static class AndroidJNI
+public static unsafe class AndroidJNI
 {
 	// JNI version 1.4 (what SDL uses)
 	private const int JNI_VERSION_1_4 = 0x00010004;
 	private const int JNI_OK = 0;
 
-	public static unsafe int Initialize(nint vm)
+	private static JClass _gsrActivityClassId;
+	private static bool _nativeFunctionsRegistered;
+
+	public static int Initialize(nint vm)
 	{
 		var javaVM = (JavaVM*)vm;
 		JNIEnv* e;
@@ -33,8 +36,14 @@ public static unsafe int Initialize(nint vm)
 		try
 		{
 			var env = new JNIEnvPtr(e);
-			AndroidFile.InitializeJNI(env);
-			AndroidCryptography.InitializeJNI(env);
+			using (var gsrActivityClassId =
+			       new LocalRefWrapper<JClass>(env, env.FindClass("org/psr/gsr/GSRActivity"u8)))
+			{
+				_gsrActivityClassId = (JClass)env.NewGlobalRef(gsrActivityClassId.LocalRef);
+			}
+
+			AndroidFile.InitializeJNI(env, _gsrActivityClassId);
+			AndroidCryptography.InitializeJNI(env, _gsrActivityClassId);
 
 			fixed (byte*
 			       dispatchAndroidKeyEventName = "DispatchAndroidKeyEvent"u8,
@@ -51,18 +60,48 @@ public static unsafe int Initialize(nint vm)
 				nativeFunctions[1].Name = setDocumentRequestResultName;
 				nativeFunctions[1].Signature = setDocumentRequestResultSignature;
 				nativeFunctions[1].FnPtr = (delegate* unmanaged<JNIEnvPtr, JClass, JString, void>)&AndroidFile.SetDocumentRequestResult;
-
-				var gsrActivityClassId = env.FindClass("org/psr/gsr/GSRActivity"u8);
-				env.RegisterNatives(gsrActivityClassId, nativeFunctions);
+				env.RegisterNatives(_gsrActivityClassId, nativeFunctions);
+				_nativeFunctionsRegistered = true;
 			}
 
 			return JNI_VERSION_1_4;
 		}
 		catch (Exception ex)
 		{
+			Deinitialize(vm);
 			// can't show a message box here, as this is a Java thread, not a native thread
 			Console.Error.WriteLine($"JNI initialization has failed, this is fatal. Exception given: {ex}");
 			return -1;
 		}
 	}
+
+	private static void Deinitialize(nint vm)
+	{
+		if (!_gsrActivityClassId.IsNull)
+		{
+			var javaVM = (JavaVM*)vm;
+			JNIEnv* e;
+			var res = javaVM->Vtbl->GetEnv(javaVM, (void**)&e, JNI_VERSION_1_4);
+			if (res != JNI_OK)
+			{
+				Console.Error.WriteLine($"Failed to get JNIEnv* from JavaVM*, cannot properly deinitialize JNI. Error code given: {JNIException.GetErrorCodeString(res)}");
+				return;
+			}
+
+			var env = new JNIEnvPtr(e);
+			if (_nativeFunctionsRegistered)
+			{
+				try
+				{
+					env.UnregisterNatives(_gsrActivityClassId);
+				}
+				catch (Exception ex)
+				{
+					Console.Error.WriteLine($"Failed to unregister native functions, exception given: {ex}");
+				}
+			}
+
+			env.DeleteGlobalRef(_gsrActivityClassId);
+		}
+	}
 }
diff --git a/GSR/AndroidExports.cs b/GSR/AndroidExports.cs
@@ -45,6 +45,19 @@ public static int JNIOnLoad(nint vm, nint reserved)
 		_ = reserved;
 		return AndroidJNI.Initialize(vm);
 	}
+
+	/// <summary>
+	/// Called by Java side when the native library is garbage collected
+	/// JNI entrypoints should be unregistered at this point, and other JNI deinitialization should occur here too
+	/// </summary>
+	/// <param name="vm">pointer to JavaVM</param>
+	/// <param name="reserved">reserved</param>
+	[UnmanagedCallersOnly(EntryPoint = "JNI_OnUnload")]
+	public static void JNIOnUnload(nint vm, nint reserved)
+	{
+		_ = reserved;
+		AndroidJNI.Deinitialize(vm);
+	}
 }
 
 #endif