v4.8.7

Author: fractalwrench

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## 4.8.7 (2021-03-30)
+
+### Bug fixes
+
+* Fix leaks from manual JNI string conversions
+  [#222](https://github.com/bugsnag/bugsnag-unity/pull/222)
+
 ## 4.8.6 (2021-03-16)
 
 ### Bug fixes

build.cake

@@ -5,7 +5,7 @@ var target = Argument("target", "Default");
 var solution = File("./BugsnagUnity.sln");
 var configuration = Argument("configuration", "Release");
 var project = File("./src/BugsnagUnity/BugsnagUnity.csproj");
-var version = "4.8.6";
+var version = "4.8.7";
 
 Task("Restore-NuGet-Packages")
     .Does(() => NuGetRestore(solution));

src/BugsnagUnity/Native/Android/NativeInterface.cs

@@ -398,32 +398,53 @@ private Dictionary<string, object> GetJavaMapData(string methodName) {
       return value;
     }
 
-    private void CallNativeVoidMethod(string methodName, string methodSig, object[] args)
+    // Manually converts any C# strings in the arguments, replacing invalid chars with the replacement char..
+    // If we don't do this, C# will coerce them using NewStringUTF, which crashes on invalid UTF-8.
+    // Arg lists processed this way must be released using ReleaseConvertedStringArgs.
+    private object[] ConvertStringArgsToNative(object[] args)
     {
-      if (!CanRunJNI()) {
-        return;
-      }
-      bool isAttached = bsg_unity_isJNIAttached();
-      if (!isAttached) {
-        AndroidJNI.AttachCurrentThread();
-      }
-
       object[] itemsAsJavaObjects = new object[args.Length];
       for (int i = 0; i < args.Length; i++) {
         var obj = args[i];
 
         if (obj is string) {
-          //TODO:SM Leaking ref here
           itemsAsJavaObjects[i] = BuildJavaStringDisposable(obj as string);
         } else {
           itemsAsJavaObjects[i] = obj;
         }
       }
+      return itemsAsJavaObjects;
+    }
+
+    // Release any strings in a processed argument list.
+    // @param originalArgs: The original C# args.
+    // @param convertedArgs: The args list returned by ConvertStringArgsToNative.
+    private void ReleaseConvertedStringArgs(object[] originalArgs, object[] convertedArgs)
+    {
+      for (int i = 0; i < originalArgs.Length; i++) {
+        if (originalArgs[i] is string) {
+          (convertedArgs[i] as AndroidJavaObject).Dispose();
+        }
+      }
+    }
 
-      jvalue[] jargs = AndroidJNIHelper.CreateJNIArgArray(itemsAsJavaObjects);
+    private void CallNativeVoidMethod(string methodName, string methodSig, object[] args)
+    {
+      if (!CanRunJNI()) {
+        return;
+      }
+      bool isAttached = bsg_unity_isJNIAttached();
+      if (!isAttached) {
+        AndroidJNI.AttachCurrentThread();
+      }
+
+      object[] convertedArgs = ConvertStringArgsToNative(args);
+      jvalue[] jargs = AndroidJNIHelper.CreateJNIArgArray(convertedArgs);
       IntPtr methodID = AndroidJNI.GetStaticMethodID(BugsnagNativeInterface, methodName, methodSig);
       AndroidJNI.CallStaticVoidMethod(BugsnagNativeInterface, methodID, jargs);
-      AndroidJNIHelper.DeleteJNIArgArray(itemsAsJavaObjects, jargs);
+      AndroidJNIHelper.DeleteJNIArgArray(convertedArgs, jargs);
+      ReleaseConvertedStringArgs(args, convertedArgs);
+
       if (!isAttached) {
         AndroidJNI.DetachCurrentThread();
       }
@@ -439,10 +460,12 @@ private IntPtr CallNativeObjectMethodRef(string methodName, string methodSig, ob
         AndroidJNI.AttachCurrentThread();
       }
 
-      jvalue[] jargs = AndroidJNIHelper.CreateJNIArgArray(args);
+      object[] convertedArgs = ConvertStringArgsToNative(args);
+      jvalue[] jargs = AndroidJNIHelper.CreateJNIArgArray(convertedArgs);
       IntPtr methodID = AndroidJNI.GetStaticMethodID(BugsnagNativeInterface, methodName, methodSig);
       IntPtr nativeValue = AndroidJNI.CallStaticObjectMethod(BugsnagNativeInterface, methodID, jargs);
       AndroidJNIHelper.DeleteJNIArgArray(args, jargs);
+      ReleaseConvertedStringArgs(args, convertedArgs);
 
       if (!isAttached) {
         AndroidJNI.DetachCurrentThread();
@@ -483,10 +506,12 @@ private string CallNativeStringMethod(string methodName, string methodSig, objec
         AndroidJNI.AttachCurrentThread();
       }
 
-      jvalue[] jargs = AndroidJNIHelper.CreateJNIArgArray(args);
+      object[] convertedArgs = ConvertStringArgsToNative(args);
+      jvalue[] jargs = AndroidJNIHelper.CreateJNIArgArray(convertedArgs);
       IntPtr methodID = AndroidJNI.GetStaticMethodID(BugsnagNativeInterface, methodName, methodSig);
       IntPtr nativeValue = AndroidJNI.CallStaticObjectMethod(BugsnagNativeInterface, methodID, jargs);
       AndroidJNIHelper.DeleteJNIArgArray(args, jargs);
+      ReleaseConvertedStringArgs(args, convertedArgs);
 
       string value = null;
       if (nativeValue != null && nativeValue != IntPtr.Zero) {