Add implementation/test for GlobalString destruction.

This was actually causing premature releases of `GlobalString` when directly constructed with arguments because it wasn't promoting its `LocalObject` after construction. Also, add tests for graceful destruction of GlobalObject which isn't returned. PiperOrigin-RevId: 674475063
google · Sep 13, 2024 · 954558c · 954558c
1 parent 690a918
commit 954558c
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 3 deletions.
diff --git a/implementation/global_string.h b/implementation/global_string.h
@@ -48,6 +48,12 @@ class GlobalString : public GlobalStringImpl {
   GlobalString(LocalString &&local_string)
       : Base(LifecycleT::Promote(local_string.Release())) {}
 
+  template <typename... Ts>
+  GlobalString(Ts &&...vals) : Base(std::forward<Ts &&>(vals)...) {
+    RefBase<jstring>::object_ref_ =
+        LifecycleT::Promote(RefBase<jstring>::object_ref_);
+  }
+
   // Returns a StringView which possibly performs an expensive pinning
   // operation.  String objects can be pinned multiple times.
   UtfStringView Pin() { return {RefBase<jstring>::object_ref_}; }

diff --git a/javatests/com/jnibind/test/GlobalObjectTest.java b/javatests/com/jnibind/test/GlobalObjectTest.java
@@ -49,6 +49,13 @@ public static void doShutDown() {
     jniTearDown();
   }
 
+  static native void jniObjectGracefullyDies();
+
+  @Test
+  public void objectGracefullyDies() {
+    jniObjectGracefullyDies();
+  }
+
   @Test
   public void createNewGlobalObjectUsingNoArgConstructor() {
     assertThat(jniCreateNewObject()).isNotEqualTo(null);
@@ -87,7 +94,6 @@ public void createFromCopyAssignmentIntoGlobal() {
   native ObjectTestHelper jniBuildNewObjectsFromExistingObjects(
       GlobalObjectTest testHelperObject, ObjectTestHelper objectToMutate);
 
-
   ObjectTestHelper methodTakesGlobalObjectReturnsNewObject(ObjectTestHelper object) {
     return new ObjectTestHelper(object.intVal1 + 5, object.intVal2 + 6, object.intVal3 + 7);
   }
@@ -136,5 +142,4 @@ public void materializeNewGlobalObjectSetIntVal159() {
     assertThat(object.intVal2).isEqualTo(11);
     assertThat(object.intVal3).isEqualTo(16);
   }
-
 }
diff --git a/javatests/com/jnibind/test/StringTest.java b/javatests/com/jnibind/test/StringTest.java
@@ -66,7 +66,6 @@ public void multiFormPassTest() {
   native void jniVoidMethodTakesFiveStrings(
       StringTestHelper helper, String s1, String s2, String s3, String s4, String s5);
 
-
   // Calls each of the above native methods which will then call the similarly named method on the
   // StringTestHelper.  E.g. jniVoidMethodTakesString calls voidMethodTakesString.
   @Test
@@ -140,4 +139,11 @@ public void nullStringTests() {
     verify(rJniStringTestHelper).stringMethodTakesString(null);
   }
   */
+
+  native void jniReturnsAGlobalString();
+
+  @Test
+  public void globalReturnsCorrectlyOverJniBoundary() {
+    jniReturnsAGlobalString();
+  }
 }
diff --git a/javatests/com/jnibind/test/global_object_test_jni.cc b/javatests/com/jnibind/test/global_object_test_jni.cc
@@ -46,6 +46,12 @@ Java_com_jnibind_test_GlobalObjectTest_jniCreateNewObject(JNIEnv* env, jclass) {
   return jni::GlobalObject<kObjectTestHelperClass>{1, 2, 3}.Release();
 }
 
+JNIEXPORT void JNICALL
+Java_com_jnibind_test_GlobalObjectTest_jniObjectGracefullyDies(JNIEnv* env,
+                                                               jclass) {
+  jni::GlobalObject<kObjectTestHelperClass>{1, 2, 3};
+}
+
 JNIEXPORT jobject JNICALL
 Java_com_jnibind_test_GlobalObjectTest_jniCreateNewGlobalObjectSetIntVal123(
     JNIEnv* env, jclass) {

diff --git a/javatests/com/jnibind/test/string_test_jni.cc b/javatests/com/jnibind/test/string_test_jni.cc
@@ -145,4 +145,9 @@ Java_com_jnibind_test_StringTest_jniStringMethodTakesFiveStrings(
       .Release();
 }
 
+JNIEXPORT void JNICALL
+Java_com_jnibind_test_StringTest_jniReturnsAGlobalString(JNIEnv* env, jclass) {
+  GlobalString g{"fake"};
+}
+
 }  // extern "C"