Add implementation/test for GlobalString destruction.

This was actually causing premature releases of GlobalString because it wasn't promoting its local representation. Also, add tests for graceful destruction of GlobalObject which isn't returned. PiperOrigin-RevId: 674322344
google · Sep 13, 2024 · efaba04 · efaba04
1 parent 690a918
commit efaba04
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"