From d4b3ae0938a6f67be3ac80d98abc60de1e00a978 Mon Sep 17 00:00:00 2001
From: Kevin Thornberry <kthornberry@snapchat.com>
Date: Mon, 8 May 2023 20:59:10 +0100
Subject: [PATCH] Fixes for a couple of issues found when running with
 check:jni enabled

1. Add -Xcheck:jni to the Java test suite build to help catch issues during test runs
2. In PrimitiveArray::fromCpp, when unlocking the Java array with ReleasePrimitiveArrayCritical(), ensure any internal copies of the data are written back to the original. A JVM _might_ make a copy of the data when locking, the check:jni mode forces a copy to help catch when copies aren't written back.
3. A couple of places in the test suite where native->Java method calls are made but the exception state is not checked on return.
---
 support-lib/jni/Marshal.hpp        | 2 +-
 support-lib/jni/Outcome_jni.hpp    | 1 +
 support-lib/jni/djinni_support.cpp | 1 +
 test-suite/BUILD                   | 2 +-
 4 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/support-lib/jni/Marshal.hpp b/support-lib/jni/Marshal.hpp
index 90edffcc..371c452f 100644
--- a/support-lib/jni/Marshal.hpp
+++ b/support-lib/jni/Marshal.hpp
@@ -712,7 +712,7 @@ namespace djinni
         {
             auto j = T::makePrimitiveArray(jniEnv, static_cast<jint>(c.size()));
             if (c.size() > 0) {
-                auto deleter = [jniEnv, j] (void* c) {if (c) {jniEnv->ReleasePrimitiveArrayCritical(j, c, JNI_ABORT);}};
+                auto deleter = [jniEnv, j] (void* c) {if (c) {jniEnv->ReleasePrimitiveArrayCritical(j, c, 0);}};
                 std::unique_ptr<EJniType, decltype(deleter)> ptr(
                     reinterpret_cast<EJniType*>(jniEnv->GetPrimitiveArrayCritical(j, nullptr)),
                     deleter);
diff --git a/support-lib/jni/Outcome_jni.hpp b/support-lib/jni/Outcome_jni.hpp
index 2993c55c..a7efab94 100644
--- a/support-lib/jni/Outcome_jni.hpp
+++ b/support-lib/jni/Outcome_jni.hpp
@@ -53,6 +53,7 @@ class Outcome
             return RESULT::Boxed::toCpp(jniEnv, reinterpret_cast<typename RESULT::Boxed::JniType>(r.get()));
         } else {
             auto e = LocalRef<jobject>(jniEnv, jniEnv->CallObjectMethod(j, outcomeJniInfo.method_error_or_null));
+            jniExceptionCheck(jniEnv);
             // if result is not present then error must be present, we can skip the present check
             return make_unexpected(ERROR::Boxed::toCpp(jniEnv, reinterpret_cast<typename ERROR::Boxed::JniType>(e.get())));
         }
diff --git a/support-lib/jni/djinni_support.cpp b/support-lib/jni/djinni_support.cpp
index df9842ae..77facc6c 100644
--- a/support-lib/jni/djinni_support.cpp
+++ b/support-lib/jni/djinni_support.cpp
@@ -121,6 +121,7 @@ void jniInit(JavaVM * jvm) {
     jclass classClass = env->GetObjectClass(ourClass);
     jmethodID getClassLoaderMethod = env->GetMethodID(classClass, "getClassLoader", "()Ljava/lang/ClassLoader;");
     jobject tmp = env->CallObjectMethod(ourClass, getClassLoaderMethod);
+    jniExceptionCheck(env);
     g_ourClassLoader = (jobject)env->NewGlobalRef(tmp);
 
     jclass classLoaderClass = env->FindClass("java/lang/ClassLoader");
diff --git a/test-suite/BUILD b/test-suite/BUILD
index aa640d35..fa6cb8dd 100644
--- a/test-suite/BUILD
+++ b/test-suite/BUILD
@@ -132,7 +132,7 @@ java_test(
         "@maven_djinni//:io_reactivex_rxjava2_rxjava",
         "@com_google_protobuf//java/core:core",
     ],
-    jvm_flags = ["-Ddjinni.native_libs_dirs=./test-suite/libdjinni-tests-jni.so"],
+    jvm_flags = ["-Ddjinni.native_libs_dirs=./test-suite/libdjinni-tests-jni.so", "-Xcheck:jni"],
 )
 
 macos_unit_test(