From 3db18663fc5a888e675b489e369ba44a96b72359 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 27 Jun 2023 13:11:59 -0400 Subject: [PATCH 1/3] Fix the Android global refs exhaustion issue by using ArenaRef instead of Global [skip actions] --- firestore/CMakeLists.txt | 2 + .../integration_test_internal/CMakeLists.txt | 1 + .../src/android/arena_ref_android_test.cc | 1138 +++++++++++++++++ .../src/document_reference_test.cc | 2 +- .../src/field_value_test.cc | 9 + .../src/android/document_reference_android.cc | 4 +- firestore/src/android/field_value_android.cc | 89 +- firestore/src/android/field_value_android.h | 26 +- firestore/src/android/firestore_android.cc | 2 + firestore/src/android/query_android.cc | 7 +- firestore/src/android/util_android.cc | 9 +- firestore/src/android/util_android.h | 2 +- firestore/src/android/wrapper.cc | 11 +- firestore/src/android/wrapper.h | 7 +- firestore/src/android/write_batch_android.cc | 5 +- firestore/src/android/write_batch_android.h | 2 +- firestore/src/jni/arena_ref.cc | 246 ++++ firestore/src/jni/arena_ref.h | 116 ++ firestore/src/jni/env.h | 8 + firestore/src/jni/loader.h | 2 + .../firestore/internal/cpp/ObjectArena.java | 48 + release_build_files/readme.md | 4 + 22 files changed, 1670 insertions(+), 70 deletions(-) create mode 100644 firestore/integration_test_internal/src/android/arena_ref_android_test.cc create mode 100644 firestore/src/jni/arena_ref.cc create mode 100644 firestore/src/jni/arena_ref.h create mode 100644 firestore/src_java/com/google/firebase/firestore/internal/cpp/ObjectArena.java diff --git a/firestore/CMakeLists.txt b/firestore/CMakeLists.txt index 762d217554..0cea8d1435 100644 --- a/firestore/CMakeLists.txt +++ b/firestore/CMakeLists.txt @@ -149,6 +149,8 @@ set(android_SRCS src/jni/array.h src/jni/array_list.cc src/jni/array_list.h + src/jni/arena_ref.cc + src/jni/arena_ref.h src/jni/boolean.cc src/jni/boolean.h src/jni/call_traits.h diff --git a/firestore/integration_test_internal/CMakeLists.txt b/firestore/integration_test_internal/CMakeLists.txt index 5aa57311f3..791500961a 100644 --- a/firestore/integration_test_internal/CMakeLists.txt +++ b/firestore/integration_test_internal/CMakeLists.txt @@ -123,6 +123,7 @@ set(FIREBASE_INTEGRATION_TEST_PORTABLE_TEST_SRCS # These sources contain the actual tests that run on Android only. set(FIREBASE_INTEGRATION_TEST_ANDROID_TEST_SRCS + src/android/arena_ref_android_test.cc src/android/field_path_portable_test.cc src/android/firestore_integration_test_android_test.cc src/android/geo_point_android_test.cc diff --git a/firestore/integration_test_internal/src/android/arena_ref_android_test.cc b/firestore/integration_test_internal/src/android/arena_ref_android_test.cc new file mode 100644 index 0000000000..cffbc0b1af --- /dev/null +++ b/firestore/integration_test_internal/src/android/arena_ref_android_test.cc @@ -0,0 +1,1138 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "firestore/src/jni/arena_ref.h" + +#include +#include +#include +#include + +#include "android/firestore_integration_test_android.h" +#include "firestore/src/jni/env.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace { + +using firebase::firestore::FirestoreAndroidIntegrationTest; +using firebase::firestore::RefersToSameJavaObjectAs; +using firebase::firestore::jni::ArenaRef; +using firebase::firestore::jni::Env; +using firebase::firestore::jni::Local; +using firebase::firestore::jni::Object; + +class ArenaRefTest : public FirestoreAndroidIntegrationTest { + public: + ~ArenaRefTest() override { + for (jobject created_java_object : created_java_objects_) { + env().get()->DeleteGlobalRef(created_java_object); + } + } + + /** + * Creates and returns a brand new Java object. + * + * The returned object is a global reference that this object retains + * ownership of. The global reference will be automatically deleted by this + * object's destructor. + * + * If creating the new Java object fails, such as if this function is called + * with a pending Java exception, then a null object is returned and the + * calling test case will fail. + * + * @return a global reference to the newly-created Java object, or null if + * creating the object fails. + */ + jobject NewJavaObject() { + SCOPED_TRACE("NewJavaObject"); + + JNIEnv* jni_env = env().get(); + if (jni_env->ExceptionCheck()) { + jni_env->ExceptionDescribe(); + ADD_FAILURE() << "NewJavaObject() called with a pending exception"; + return {}; + } + + jclass object_class = jni_env->FindClass("java/lang/Object"); + if (jni_env->ExceptionCheck()) { + jni_env->ExceptionDescribe(); + ADD_FAILURE() << "JNIEnv::FindClass() failed"; + return {}; + } + + jmethodID object_constructor_id = + jni_env->GetMethodID(object_class, "", "()V"); + if (jni_env->ExceptionCheck()) { + jni_env->ExceptionDescribe(); + ADD_FAILURE() << "JNIEnv::GetMethodID() failed"; + return {}; + } + + jobject object_local_ref = + jni_env->NewObject(object_class, object_constructor_id); + if (jni_env->ExceptionCheck()) { + jni_env->ExceptionDescribe(); + ADD_FAILURE() << "JNIEnv::NewObject() failed"; + return {}; + } + + jobject object_global_ref = jni_env->NewGlobalRef(object_local_ref); + jni_env->DeleteLocalRef(object_local_ref); + if (jni_env->ExceptionCheck()) { + jni_env->ExceptionDescribe(); + ADD_FAILURE() << "JNIEnv::NewGlobalRef() failed"; + return {}; + } + + created_java_objects_.push_back(object_global_ref); + return object_global_ref; + } + + private: + std::vector created_java_objects_; +}; + +/** + * A googletest "matcher" that verifies that an ArenaRef refers to a null Java + * object. + * + * Example: + * ArenaRef arena_ref; + * EXPECT_THAT(arena_ref, RefersToNullJavaObject()); + */ +MATCHER(RefersToNullJavaObject, "") { + static_assert(std::is_same::value, ""); + + Env env; + if (!env.ok()) { + ADD_FAILURE() << "RefersToNullJavaObject called with pending exception"; + return false; + } + + Local object = arg.get(env); + if (!env.ok()) { + ADD_FAILURE() << "RefersToNullJavaObject ArenaRef.get() threw an exception"; + return false; + } + + return object.get() == nullptr; +} + +/** + * A googletest "matcher" that verifies that an ArenaRef refers to the given + * Java object, compared as if using the `==` operator in Java. + * + * Example: + * jobject java_object = NewJavaObject(); + * ArenaRef arena_ref(env, java_object); + * EXPECT_THAT(arena_ref, RefersToJavaObject(java_object)); + */ +MATCHER_P(RefersToJavaObject, expected_jobject, "") { + static_assert(std::is_same::value, ""); + static_assert(std::is_same::value, ""); + + Env env; + if (!env.ok()) { + ADD_FAILURE() << "RefersToJavaObject called with pending exception"; + return false; + } + + Local object = arg.get(env); + if (!env.ok()) { + ADD_FAILURE() << "RefersToJavaObject ArenaRef.get() threw an exception"; + return false; + } + + return env.get()->IsSameObject(object.get(), expected_jobject); +} + +//////////////////////////////////////////////////////////////////////////////// +// Tests for ArenaRef::ArenaRef() +//////////////////////////////////////////////////////////////////////////////// + +TEST_F(ArenaRefTest, DefaultConstructorShouldReferToNull) { + ArenaRef arena_ref; + + EXPECT_THAT(arena_ref, RefersToNullJavaObject()); +} + +TEST_F(ArenaRefTest, + DefaultConstructorShouldSucceedIfCalledWithPendingException) { + ThrowException(); + + ArenaRef arena_ref; + + env().ClearExceptionOccurred(); + EXPECT_THAT(arena_ref, RefersToNullJavaObject()); +} + +//////////////////////////////////////////////////////////////////////////////// +// Tests for ArenaRef::ArenaRef(Env&, jobject) +//////////////////////////////////////////////////////////////////////////////// + +TEST_F(ArenaRefTest, AdoptingConstructorWithNullptrShouldReferToNull) { + const ArenaRef arena_ref(env(), nullptr); + + EXPECT_THAT(arena_ref, RefersToNullJavaObject()); +} + +TEST_F(ArenaRefTest, AdoptingConstructorShouldReferToTheGivenObject) { + jobject java_object = NewJavaObject(); + + ArenaRef arena_ref(env(), java_object); + + EXPECT_THAT(arena_ref, RefersToJavaObject(java_object)); +} + +TEST_F(ArenaRefTest, + AdoptingConstructorShouldReferToNullIfCalledWithPendingException) { + jobject java_object = NewJavaObject(); + ThrowException(); + + ArenaRef arena_ref(env(), java_object); + + env().ClearExceptionOccurred(); + EXPECT_THAT(arena_ref, RefersToNullJavaObject()); +} + +//////////////////////////////////////////////////////////////////////////////// +// Tests for ArenaRef::ArenaRef(const ArenaRef&) a.k.a. (copy constructor) +//////////////////////////////////////////////////////////////////////////////// + +TEST_F(ArenaRefTest, CopyConstructorWithDefaultConstructedInstance) { + const ArenaRef default_arena_ref; + + ArenaRef arena_ref_copy_dest(default_arena_ref); + + EXPECT_THAT(arena_ref_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); +} + +TEST_F(ArenaRefTest, CopyConstructorWithNull) { + const ArenaRef arena_ref_referring_to_null(env(), nullptr); + + ArenaRef arena_ref_copy_dest(arena_ref_referring_to_null); + + EXPECT_THAT(arena_ref_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); +} + +TEST_F(ArenaRefTest, CopyConstructorWithNonNull) { + jobject java_object = NewJavaObject(); + const ArenaRef arena_ref_referring_to_non_null(env(), java_object); + + ArenaRef arena_ref_copy_dest(arena_ref_referring_to_non_null); + + EXPECT_THAT(arena_ref_copy_dest, RefersToJavaObject(java_object)); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToJavaObject(java_object)); +} + +TEST_F(ArenaRefTest, CopyConstructorShouldCopyIfCalledWithPendingException) { + const ArenaRef default_arena_ref; + const ArenaRef arena_ref_referring_to_null(env(), nullptr); + jobject java_object = NewJavaObject(); + const ArenaRef arena_ref_referring_to_non_null(env(), java_object); + ThrowException(); + + ArenaRef default_arena_ref_copy_dest(default_arena_ref); + ArenaRef arena_ref_referring_to_null_copy_dest(arena_ref_referring_to_null); + ArenaRef arena_ref_referring_to_non_null_copy_dest( + arena_ref_referring_to_non_null); + + env().ClearExceptionOccurred(); + EXPECT_THAT(default_arena_ref_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null_copy_dest, + RefersToJavaObject(java_object)); +} + +TEST_F( + ArenaRefTest, + ObjectCreatedWithTheCopyConstructorShouldBeUnaffectedByChangesToTheCopiedObject) { + auto default_arena_ref = std::make_unique(); + const ArenaRef default_arena_ref_copy_dest(*default_arena_ref); + auto arena_ref_referring_to_null = std::make_unique(env(), nullptr); + const ArenaRef arena_ref_referring_to_null_copy_dest( + *arena_ref_referring_to_null); + jobject java_object = NewJavaObject(); + auto arena_ref_referring_to_non_null = + std::make_unique(env(), java_object); + const ArenaRef arena_ref_referring_to_non_null_copy_dest( + *arena_ref_referring_to_non_null); + + default_arena_ref->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_null->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_non_null->reset(env(), Object(NewJavaObject())); + + EXPECT_THAT(default_arena_ref_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null_copy_dest, + RefersToJavaObject(java_object)); + + default_arena_ref.reset(); + arena_ref_referring_to_null.reset(); + arena_ref_referring_to_non_null.reset(); + + EXPECT_THAT(default_arena_ref_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null_copy_dest, + RefersToJavaObject(java_object)); +} + +TEST_F( + ArenaRefTest, + ChangesToAnObjectCreatedWithTheCopyConstructorShouldNotAffectTheCopiedObject) { + const ArenaRef default_arena_ref; + auto default_arena_ref_copy_dest = + std::make_unique(default_arena_ref); + const ArenaRef arena_ref_referring_to_null(env(), nullptr); + auto arena_ref_referring_to_null_copy_dest = + std::make_unique(arena_ref_referring_to_null); + jobject java_object = NewJavaObject(); + const ArenaRef arena_ref_referring_to_non_null(env(), java_object); + auto arena_ref_referring_to_non_null_copy_dest = + std::make_unique(arena_ref_referring_to_non_null); + + default_arena_ref_copy_dest->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_null_copy_dest->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_non_null_copy_dest->reset(env(), + Object(NewJavaObject())); + + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToJavaObject(java_object)); + + default_arena_ref_copy_dest.reset(); + arena_ref_referring_to_null_copy_dest.reset(); + arena_ref_referring_to_non_null_copy_dest.reset(); + + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToJavaObject(java_object)); +} + +//////////////////////////////////////////////////////////////////////////////// +// Tests for ArenaRef::ArenaRef(ArenaRef&&) (move constructor) +//////////////////////////////////////////////////////////////////////////////// + +TEST_F(ArenaRefTest, MoveConstructorWithDefaultConstructedInstance) { + ArenaRef default_arena_ref; + + ArenaRef arena_ref_move_dest(std::move(default_arena_ref)); + + EXPECT_THAT(arena_ref_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); +} + +TEST_F(ArenaRefTest, MoveConstructorWithNull) { + ArenaRef arena_ref_referring_to_null(env(), nullptr); + + ArenaRef arena_ref_move_dest(std::move(arena_ref_referring_to_null)); + + EXPECT_THAT(arena_ref_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); +} + +TEST_F(ArenaRefTest, MoveConstructorWithNonNull) { + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_referring_to_non_null(env(), java_object); + + ArenaRef arena_ref_move_dest(std::move(arena_ref_referring_to_non_null)); + + EXPECT_THAT(arena_ref_move_dest, RefersToJavaObject(java_object)); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToNullJavaObject()); +} + +TEST_F(ArenaRefTest, + MoveConstructorShouldSuccessfullyMoveEvenIfCalledWithPendingException) { + ArenaRef default_arena_ref; + ArenaRef arena_ref_referring_to_null(env(), nullptr); + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_referring_to_non_null(env(), java_object); + ThrowException(); + + ArenaRef default_arena_ref_move_dest(std::move(default_arena_ref)); + ArenaRef arena_ref_referring_to_null_move_dest( + std::move(arena_ref_referring_to_null)); + ArenaRef arena_ref_referring_to_non_null_move_dest( + std::move(arena_ref_referring_to_non_null)); + + env().ClearExceptionOccurred(); + EXPECT_THAT(default_arena_ref_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null_move_dest, + RefersToJavaObject(java_object)); +} + +TEST_F( + ArenaRefTest, + ObjectCreatedWithTheMoveConstructorShouldBeUnaffectedByChangesToTheMovedFromObject) { + auto default_arena_ref = std::make_unique(); + ArenaRef default_arena_ref_move_dest(std::move(*default_arena_ref)); + auto arena_ref_referring_to_null = std::make_unique(env(), nullptr); + ArenaRef arena_ref_referring_to_null_move_dest( + std::move(*arena_ref_referring_to_null)); + jobject java_object = NewJavaObject(); + auto arena_ref_referring_to_non_null = + std::make_unique(env(), java_object); + ArenaRef arena_ref_referring_to_non_null_move_dest( + std::move(*arena_ref_referring_to_non_null)); + + default_arena_ref->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_null->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_non_null->reset(env(), Object(NewJavaObject())); + + EXPECT_THAT(default_arena_ref_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null_move_dest, + RefersToJavaObject(java_object)); + + default_arena_ref.reset(); + arena_ref_referring_to_null.reset(); + arena_ref_referring_to_non_null.reset(); + + EXPECT_THAT(default_arena_ref_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null_move_dest, + RefersToJavaObject(java_object)); +} + +TEST_F( + ArenaRefTest, + ChangesToAnObjectCreatedTheWithMoveConstructorShouldNotAffectTheMovedFromObject) { + ArenaRef default_arena_ref; + auto default_arena_ref_move_dest = + std::make_unique(std::move(default_arena_ref)); + ArenaRef arena_ref_referring_to_null(env(), nullptr); + auto arena_ref_referring_to_null_move_dest = + std::make_unique(std::move(arena_ref_referring_to_null)); + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_referring_to_non_null(env(), java_object); + auto arena_ref_referring_to_non_null_move_dest = + std::make_unique(std::move(arena_ref_referring_to_non_null)); + + default_arena_ref_move_dest->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_null_move_dest->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_non_null_move_dest->reset(env(), + Object(NewJavaObject())); + + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToNullJavaObject()); + + default_arena_ref_move_dest.reset(); + arena_ref_referring_to_null_move_dest.reset(); + arena_ref_referring_to_non_null_move_dest.reset(); + + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToNullJavaObject()); +} + +//////////////////////////////////////////////////////////////////////////////// +// Tests for ArenaRef::operator=(const ArenaRef&) (copy assignment operator) +//////////////////////////////////////////////////////////////////////////////// + +TEST_F(ArenaRefTest, + CopyAssignmentOpCorrectlyAssignsADefaultInstanceFromADefaultInstance) { + ArenaRef arena_ref_copy_dest; + const ArenaRef default_arena_ref; + + const ArenaRef& return_value = (arena_ref_copy_dest = default_arena_ref); + + EXPECT_THAT(arena_ref_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_copy_dest); +} + +TEST_F( + ArenaRefTest, + CopyAssignmentOpCorrectlyAssignsADefaultInstanceFromAnInstanceReferringToNull) { + ArenaRef arena_ref_copy_dest; + const ArenaRef arena_ref_referring_to_null(env(), nullptr); + + const ArenaRef& return_value = + (arena_ref_copy_dest = arena_ref_referring_to_null); + + EXPECT_THAT(arena_ref_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_copy_dest); +} + +TEST_F( + ArenaRefTest, + CopyAssignmentOpCorrectlyAssignsADefaultInstanceFromAnInstanceReferringToNonNull) { + ArenaRef arena_ref_copy_dest; + jobject java_object = NewJavaObject(); + const ArenaRef arena_ref_referring_to_non_null(env(), java_object); + + const ArenaRef& return_value = + (arena_ref_copy_dest = arena_ref_referring_to_non_null); + + EXPECT_THAT(arena_ref_copy_dest, RefersToJavaObject(java_object)); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToJavaObject(java_object)); + EXPECT_EQ(&return_value, &arena_ref_copy_dest); +} + +TEST_F( + ArenaRefTest, + CopyAssignmentOpCorrectlyAssignsAnInstanceReferringToNullFromADefaultInstance) { + ArenaRef arena_ref_copy_dest(env(), nullptr); + const ArenaRef default_arena_ref; + + const ArenaRef& return_value = (arena_ref_copy_dest = default_arena_ref); + + EXPECT_THAT(arena_ref_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_copy_dest); +} + +TEST_F( + ArenaRefTest, + CopyAssignmentOpCorrectlyAssignsAnInstanceReferringToNullFromAnInstanceReferringToNull) { + ArenaRef arena_ref_copy_dest(env(), nullptr); + const ArenaRef arena_ref_referring_to_null(env(), nullptr); + + const ArenaRef& return_value = + (arena_ref_copy_dest = arena_ref_referring_to_null); + + EXPECT_THAT(arena_ref_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_copy_dest); +} + +TEST_F( + ArenaRefTest, + CopyAssignmentOpCorrectlyAssignsAnInstanceReferringToNullFromAnInstanceReferringToNonNull) { + ArenaRef arena_ref_copy_dest(env(), nullptr); + jobject java_object = NewJavaObject(); + const ArenaRef arena_ref_referring_to_non_null(env(), java_object); + + const ArenaRef& return_value = + (arena_ref_copy_dest = arena_ref_referring_to_non_null); + + EXPECT_THAT(arena_ref_copy_dest, RefersToJavaObject(java_object)); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToJavaObject(java_object)); + EXPECT_EQ(&return_value, &arena_ref_copy_dest); +} + +TEST_F( + ArenaRefTest, + CopyAssignmentOpCorrectlyAssignsAnInstanceReferringToNonNullFromADefaultInstance) { + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_copy_dest(env(), java_object); + const ArenaRef default_arena_ref; + + const ArenaRef& return_value = (arena_ref_copy_dest = default_arena_ref); + + EXPECT_THAT(arena_ref_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_copy_dest); +} + +TEST_F( + ArenaRefTest, + CopyAssignmentOpCorrectlyAssignsAnInstanceReferringToNonNullFromAnInstanceReferringToNull) { + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_copy_dest(env(), java_object); + const ArenaRef arena_ref_referring_to_null(env(), nullptr); + + const ArenaRef& return_value = + (arena_ref_copy_dest = arena_ref_referring_to_null); + + EXPECT_THAT(arena_ref_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_copy_dest); +} + +TEST_F( + ArenaRefTest, + CopyAssignmentOpCorrectlyAssignsAnInstanceReferringToNonNullFromAnInstanceReferringToNonNull) { + jobject original_java_object = NewJavaObject(); + ArenaRef arena_ref_copy_dest(env(), original_java_object); + jobject new_java_object = NewJavaObject(); + const ArenaRef arena_ref_referring_to_non_null(env(), new_java_object); + + const ArenaRef& return_value = + (arena_ref_copy_dest = arena_ref_referring_to_non_null); + + EXPECT_THAT(arena_ref_copy_dest, RefersToJavaObject(new_java_object)); + EXPECT_THAT(arena_ref_referring_to_non_null, + RefersToJavaObject(new_java_object)); + EXPECT_EQ(&return_value, &arena_ref_copy_dest); +} + +TEST_F(ArenaRefTest, + CopyAssignmentOpCorrectlyAssignsSelfWhenSelfIsDefaultInstance) { + ArenaRef default_arena_ref; + + const ArenaRef& return_value = (default_arena_ref = default_arena_ref); + + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &default_arena_ref); +} + +TEST_F( + ArenaRefTest, + CopyAssignmentOpCorrectlyAssignsSelfWhenSelfIsAnInstanceReferringToNull) { + ArenaRef arena_ref_referring_to_null(env(), nullptr); + + const ArenaRef& return_value = + (arena_ref_referring_to_null = arena_ref_referring_to_null); + + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_referring_to_null); +} + +TEST_F( + ArenaRefTest, + CopyAssignmentOpCorrectlyAssignsSelfWhenSelfIsAnInstanceReferringToNonNull) { + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_referring_to_non_null(env(), java_object); + + const ArenaRef& return_value = + (arena_ref_referring_to_non_null = arena_ref_referring_to_non_null); + + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToJavaObject(java_object)); + EXPECT_EQ(&return_value, &arena_ref_referring_to_non_null); +} + +TEST_F( + ArenaRefTest, + CopyAssignmentOpOnADefaultInstanceShouldCopyIfCalledWithPendingException) { + ArenaRef default_arena_ref; + jobject java_object = NewJavaObject(); + const ArenaRef arena_ref_referring_to_non_null(env(), java_object); + ThrowException(); + + const ArenaRef& return_value = + (default_arena_ref = arena_ref_referring_to_non_null); + + env().ClearExceptionOccurred(); + EXPECT_THAT(default_arena_ref, RefersToJavaObject(java_object)); + EXPECT_EQ(&return_value, &default_arena_ref); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToJavaObject(java_object)); +} + +TEST_F( + ArenaRefTest, + CopyAssignmentOpOnAnInstanceReferringToNullShouldCopyIfCalledWithPendingException) { + ArenaRef arena_ref_referring_to_null(env(), nullptr); + jobject java_object = NewJavaObject(); + const ArenaRef arena_ref_referring_to_non_null(env(), java_object); + ThrowException(); + + const ArenaRef& return_value = + (arena_ref_referring_to_null = arena_ref_referring_to_non_null); + + env().ClearExceptionOccurred(); + EXPECT_THAT(arena_ref_referring_to_null, RefersToJavaObject(java_object)); + EXPECT_EQ(&return_value, &arena_ref_referring_to_null); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToJavaObject(java_object)); +} + +TEST_F( + ArenaRefTest, + CopyAssignmentOpOnAnInstanceReferringToNonNullShouldCopyIfCalledWithPendingException) { + ArenaRef arena_ref_referring_to_non_null(env(), NewJavaObject()); + jobject java_object = NewJavaObject(); + const ArenaRef another_arena_ref_referring_to_non_null(env(), java_object); + ThrowException(); + + const ArenaRef& return_value = (arena_ref_referring_to_non_null = + another_arena_ref_referring_to_non_null); + + env().ClearExceptionOccurred(); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToJavaObject(java_object)); + EXPECT_EQ(&return_value, &arena_ref_referring_to_non_null); + EXPECT_THAT(another_arena_ref_referring_to_non_null, + RefersToJavaObject(java_object)); +} + +TEST_F( + ArenaRefTest, + DestObjectOfCopyAssignmentOperatorShouldBeUnaffectedByChangesToSourceObject) { + auto default_arena_ref = std::make_unique(); + ArenaRef default_arena_ref_copy_dest; + auto arena_ref_referring_to_null = std::make_unique(env(), nullptr); + ArenaRef arena_ref_referring_to_null_copy_dest; + jobject java_object = NewJavaObject(); + auto arena_ref_referring_to_non_null = + std::make_unique(env(), java_object); + ArenaRef arena_ref_referring_to_non_null_copy_dest; + + default_arena_ref_copy_dest = *default_arena_ref; + arena_ref_referring_to_null_copy_dest = *arena_ref_referring_to_null; + arena_ref_referring_to_non_null_copy_dest = *arena_ref_referring_to_non_null; + + default_arena_ref->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_null->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_non_null->reset(env(), Object(NewJavaObject())); + + EXPECT_THAT(default_arena_ref_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null_copy_dest, + RefersToJavaObject(java_object)); + + default_arena_ref.reset(); + arena_ref_referring_to_null.reset(); + arena_ref_referring_to_non_null.reset(); + + EXPECT_THAT(default_arena_ref_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null_copy_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null_copy_dest, + RefersToJavaObject(java_object)); +} + +TEST_F( + ArenaRefTest, + SourceObjectOfCopyAssignmentOperatorShouldBeUnaffectedByChangesToDestObject) { + const ArenaRef default_arena_ref; + auto default_arena_ref_copy_dest = std::make_unique(); + const ArenaRef arena_ref_referring_to_null(env(), nullptr); + auto arena_ref_referring_to_null_copy_dest = std::make_unique(); + jobject java_object = NewJavaObject(); + const ArenaRef arena_ref_referring_to_non_null(env(), java_object); + auto arena_ref_referring_to_non_null_copy_dest = std::make_unique(); + + *default_arena_ref_copy_dest = default_arena_ref; + *arena_ref_referring_to_null_copy_dest = arena_ref_referring_to_null; + *arena_ref_referring_to_non_null_copy_dest = arena_ref_referring_to_non_null; + + default_arena_ref_copy_dest->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_null_copy_dest->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_non_null_copy_dest->reset(env(), + Object(NewJavaObject())); + + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToJavaObject(java_object)); + + default_arena_ref_copy_dest.reset(); + arena_ref_referring_to_null_copy_dest.reset(); + arena_ref_referring_to_non_null_copy_dest.reset(); + + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToJavaObject(java_object)); +} + +//////////////////////////////////////////////////////////////////////////////// +// Tests for ArenaRef::operator=(ArenaRef&&) (move assignment operator) +//////////////////////////////////////////////////////////////////////////////// + +TEST_F(ArenaRefTest, + MoveAssignmentOpCorrectlyAssignsADefaultInstanceFromADefaultInstance) { + ArenaRef arena_ref_move_dest; + ArenaRef default_arena_ref; + + const ArenaRef& return_value = + (arena_ref_move_dest = std::move(default_arena_ref)); + + EXPECT_THAT(arena_ref_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_move_dest); +} + +TEST_F( + ArenaRefTest, + MoveAssignmentOpCorrectlyAssignsADefaultInstanceFromAnInstanceReferringToNull) { + ArenaRef arena_ref_move_dest; + const ArenaRef arena_ref_referring_to_null(env(), nullptr); + + const ArenaRef& return_value = + (arena_ref_move_dest = std::move(arena_ref_referring_to_null)); + + EXPECT_THAT(arena_ref_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_move_dest); +} + +TEST_F( + ArenaRefTest, + MoveAssignmentOpCorrectlyAssignsADefaultInstanceFromAnInstanceReferringToNonNull) { + ArenaRef arena_ref_move_dest; + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_referring_to_non_null(env(), java_object); + + const ArenaRef& return_value = + (arena_ref_move_dest = std::move(arena_ref_referring_to_non_null)); + + EXPECT_THAT(arena_ref_move_dest, RefersToJavaObject(java_object)); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_move_dest); +} + +TEST_F( + ArenaRefTest, + MoveAssignmentOpCorrectlyAssignsAnInstanceReferringToNullFromADefaultInstance) { + ArenaRef arena_ref_move_dest(env(), nullptr); + ArenaRef default_arena_ref; + + const ArenaRef& return_value = + (arena_ref_move_dest = std::move(default_arena_ref)); + + EXPECT_THAT(arena_ref_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_move_dest); +} + +TEST_F( + ArenaRefTest, + MoveAssignmentOpCorrectlyAssignsAnInstanceReferringToNullFromAnInstanceReferringToNull) { + ArenaRef arena_ref_move_dest(env(), nullptr); + ArenaRef arena_ref_referring_to_null(env(), nullptr); + + const ArenaRef& return_value = + (arena_ref_move_dest = std::move(arena_ref_referring_to_null)); + + EXPECT_THAT(arena_ref_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_move_dest); +} + +TEST_F( + ArenaRefTest, + MoveAssignmentOpCorrectlyAssignsAnInstanceReferringToNullFromAnInstanceReferringToNonNull) { + ArenaRef arena_ref_move_dest(env(), nullptr); + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_referring_to_non_null(env(), java_object); + + const ArenaRef& return_value = + (arena_ref_move_dest = std::move(arena_ref_referring_to_non_null)); + + EXPECT_THAT(arena_ref_move_dest, RefersToJavaObject(java_object)); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_move_dest); +} + +TEST_F( + ArenaRefTest, + MoveAssignmentOpCorrectlyAssignsAnInstanceReferringToNonNullFromADefaultInstance) { + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_move_dest(env(), java_object); + ArenaRef default_arena_ref; + + const ArenaRef& return_value = + (arena_ref_move_dest = std::move(default_arena_ref)); + + EXPECT_THAT(arena_ref_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_move_dest); +} + +TEST_F( + ArenaRefTest, + MoveAssignmentOpCorrectlyAssignsAnInstanceReferringToNonNullFromAnInstanceReferringToNull) { + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_move_dest(env(), java_object); + ArenaRef arena_ref_referring_to_null(env(), nullptr); + + const ArenaRef& return_value = + (arena_ref_move_dest = std::move(arena_ref_referring_to_null)); + + EXPECT_THAT(arena_ref_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_move_dest); +} + +TEST_F( + ArenaRefTest, + MoveAssignmentOpCorrectlyAssignsAnInstanceReferringToNonNullFromAnInstanceReferringToNonNull) { + ArenaRef arena_ref_move_dest(env(), NewJavaObject()); + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_referring_to_non_null(env(), java_object); + + const ArenaRef& return_value = + (arena_ref_move_dest = std::move(arena_ref_referring_to_non_null)); + + EXPECT_THAT(arena_ref_move_dest, RefersToJavaObject(java_object)); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_move_dest); +} + +TEST_F(ArenaRefTest, + MoveAssignmentOpCorrectlyAssignsSelfWhenSelfIsDefaultInstance) { + ArenaRef default_arena_ref; + + const ArenaRef& return_value = + (default_arena_ref = std::move(default_arena_ref)); + + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &default_arena_ref); +} + +TEST_F( + ArenaRefTest, + MoveAssignmentOpCorrectlyAssignsSelfWhenSelfIsAnInstanceReferringToNull) { + ArenaRef arena_ref_referring_to_null(env(), nullptr); + + const ArenaRef& return_value = + (arena_ref_referring_to_null = std::move(arena_ref_referring_to_null)); + + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_EQ(&return_value, &arena_ref_referring_to_null); +} + +TEST_F( + ArenaRefTest, + MoveAssignmentOpCorrectlyAssignsSelfWhenSelfIsAnInstanceReferringToNonNull) { + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_referring_to_non_null(env(), java_object); + + const ArenaRef& return_value = (arena_ref_referring_to_non_null = std::move( + arena_ref_referring_to_non_null)); + + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToJavaObject(java_object)); + EXPECT_EQ(&return_value, &arena_ref_referring_to_non_null); +} + +TEST_F( + ArenaRefTest, + MoveAssignmentOpOnADefaultInstanceShouldMoveIfCalledWithPendingException) { + ArenaRef default_arena_ref; + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_referring_to_non_null(env(), java_object); + ThrowException(); + + const ArenaRef& return_value = + (default_arena_ref = std::move(arena_ref_referring_to_non_null)); + + env().ClearExceptionOccurred(); + EXPECT_THAT(default_arena_ref, RefersToJavaObject(java_object)); + EXPECT_EQ(&return_value, &default_arena_ref); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToNullJavaObject()); +} + +TEST_F( + ArenaRefTest, + MoveAssignmentOpOnAnInstanceReferringToNullShouldMoveIfCalledWithPendingException) { + ArenaRef arena_ref_referring_to_null(env(), nullptr); + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_referring_to_non_null(env(), java_object); + ThrowException(); + + const ArenaRef& return_value = (arena_ref_referring_to_null = std::move( + arena_ref_referring_to_non_null)); + + env().ClearExceptionOccurred(); + EXPECT_THAT(arena_ref_referring_to_null, RefersToJavaObject(java_object)); + EXPECT_EQ(&return_value, &arena_ref_referring_to_null); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToNullJavaObject()); +} + +TEST_F( + ArenaRefTest, + MoveAssignmentOpOnAnInstanceReferringToNonNullShouldMoveIfCalledWithPendingException) { + ArenaRef arena_ref_referring_to_non_null(env(), NewJavaObject()); + jobject java_object = NewJavaObject(); + ArenaRef another_arena_ref_referring_to_non_null(env(), java_object); + ThrowException(); + + const ArenaRef& return_value = (arena_ref_referring_to_non_null = std::move( + another_arena_ref_referring_to_non_null)); + + env().ClearExceptionOccurred(); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToJavaObject(java_object)); + EXPECT_EQ(&return_value, &arena_ref_referring_to_non_null); + EXPECT_THAT(another_arena_ref_referring_to_non_null, + RefersToNullJavaObject()); +} + +TEST_F( + ArenaRefTest, + DestObjectOfMoveAssignmentOperatorShouldBeUnaffectedByChangesToSourceObject) { + auto default_arena_ref = std::make_unique(); + ArenaRef default_arena_ref_move_dest; + auto arena_ref_referring_to_null = std::make_unique(env(), nullptr); + ArenaRef arena_ref_referring_to_null_move_dest; + jobject java_object = NewJavaObject(); + auto arena_ref_referring_to_non_null = + std::make_unique(env(), java_object); + ArenaRef arena_ref_referring_to_non_null_move_dest; + + default_arena_ref_move_dest = std::move(*default_arena_ref); + arena_ref_referring_to_null_move_dest = + std::move(*arena_ref_referring_to_null); + arena_ref_referring_to_non_null_move_dest = + std::move(*arena_ref_referring_to_non_null); + + default_arena_ref->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_null->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_non_null->reset(env(), Object(NewJavaObject())); + + EXPECT_THAT(default_arena_ref_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null_move_dest, + RefersToJavaObject(java_object)); + + default_arena_ref.reset(); + arena_ref_referring_to_null.reset(); + arena_ref_referring_to_non_null.reset(); + + EXPECT_THAT(default_arena_ref_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null_move_dest, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null_move_dest, + RefersToJavaObject(java_object)); +} + +TEST_F( + ArenaRefTest, + SourceObjectOfMoveAssignmentOperatorShouldBeUnaffectedByChangesToDestObject) { + ArenaRef default_arena_ref; + auto default_arena_ref_move_dest = std::make_unique(); + ArenaRef arena_ref_referring_to_null(env(), nullptr); + auto arena_ref_referring_to_null_move_dest = std::make_unique(); + jobject java_object = NewJavaObject(); + ArenaRef arena_ref_referring_to_non_null(env(), java_object); + auto arena_ref_referring_to_non_null_move_dest = std::make_unique(); + + *default_arena_ref_move_dest = std::move(default_arena_ref); + *arena_ref_referring_to_null_move_dest = + std::move(arena_ref_referring_to_null); + *arena_ref_referring_to_non_null_move_dest = + std::move(arena_ref_referring_to_non_null); + + default_arena_ref_move_dest->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_null_move_dest->reset(env(), Object(NewJavaObject())); + arena_ref_referring_to_non_null_move_dest->reset(env(), + Object(NewJavaObject())); + + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToNullJavaObject()); + + default_arena_ref_move_dest.reset(); + arena_ref_referring_to_null_move_dest.reset(); + arena_ref_referring_to_non_null_move_dest.reset(); + + EXPECT_THAT(default_arena_ref, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_null, RefersToNullJavaObject()); + EXPECT_THAT(arena_ref_referring_to_non_null, RefersToNullJavaObject()); +} + +//////////////////////////////////////////////////////////////////////////////// +// Tests for ArenaRef::get() +//////////////////////////////////////////////////////////////////////////////// + +TEST_F(ArenaRefTest, GetReturnsNullIfInvokedOnADefaultInstance) { + const ArenaRef default_arena_ref; + + Local return_value = default_arena_ref.get(env()); + + EXPECT_EQ(return_value.get(), nullptr); +} + +TEST_F(ArenaRefTest, GetReturnsNullIfInvokedOnAnInstanceThatAdoptedNull) { + const ArenaRef arena_ref_referring_to_null(env(), nullptr); + + Local return_value = arena_ref_referring_to_null.get(env()); + + EXPECT_EQ(return_value.get(), nullptr); +} + +TEST_F(ArenaRefTest, GetReturnsTheNonNullObjectThatItWasCreatedWith) { + jobject java_object = NewJavaObject(); + const ArenaRef arena_ref_referring_to_non_null(env(), java_object); + + Local return_value = arena_ref_referring_to_non_null.get(env()); + + EXPECT_THAT(return_value, RefersToSameJavaObjectAs(Object(java_object))); +} + +TEST_F(ArenaRefTest, GetShouldReturnNullIfCalledWithPendingException) { + jobject java_object = NewJavaObject(); + ArenaRef arena_ref(env(), java_object); + ThrowException(); + + Local return_value = arena_ref.get(env()); + + env().ClearExceptionOccurred(); + EXPECT_EQ(return_value.get(), nullptr); +} + +//////////////////////////////////////////////////////////////////////////////// +// Tests for ArenaRef::reset() +//////////////////////////////////////////////////////////////////////////////// + +TEST_F(ArenaRefTest, + ResetADefaultConstructedInstanceToANonNullObjectShouldWork) { + ArenaRef arena_ref; + jobject java_object = NewJavaObject(); + + arena_ref.reset(env(), Object(java_object)); + + EXPECT_THAT(arena_ref, RefersToJavaObject(java_object)); +} + +TEST_F(ArenaRefTest, ResetANullConstructedInstanceToANonNullObjectShouldWork) { + ArenaRef arena_ref(env(), nullptr); + jobject java_object = NewJavaObject(); + + arena_ref.reset(env(), Object(java_object)); + + EXPECT_THAT(arena_ref, RefersToJavaObject(java_object)); +} + +TEST_F(ArenaRefTest, + ResetANonNullConstructedInstanceToANonNullObjectShouldWork) { + ArenaRef arena_ref(env(), NewJavaObject()); + jobject java_object = NewJavaObject(); + + arena_ref.reset(env(), Object(java_object)); + + EXPECT_THAT(arena_ref, RefersToJavaObject(java_object)); +} + +TEST_F(ArenaRefTest, ResetADefaultConstructedInstanceToANullObjectShouldWork) { + ArenaRef arena_ref; + + arena_ref.reset(env(), Object()); + + EXPECT_THAT(arena_ref, RefersToNullJavaObject()); +} + +TEST_F(ArenaRefTest, ResetANullConstructedInstanceToANullObjectShouldWork) { + ArenaRef arena_ref(env(), nullptr); + + arena_ref.reset(env(), Object()); + + EXPECT_THAT(arena_ref, RefersToNullJavaObject()); +} + +TEST_F(ArenaRefTest, ResetANonNullConstructedInstanceToANullObjectShouldWork) { + ArenaRef arena_ref(env(), NewJavaObject()); + + arena_ref.reset(env(), Object()); + + EXPECT_THAT(arena_ref, RefersToNullJavaObject()); +} + +TEST_F(ArenaRefTest, ResetShouldSetToNullIfCalledWithPendingException) { + jobject original_java_object = NewJavaObject(); + ArenaRef arena_ref(env(), original_java_object); + jobject reset_java_object = NewJavaObject(); + ThrowException(); + + arena_ref.reset(env(), Object(reset_java_object)); + + env().ClearExceptionOccurred(); + EXPECT_THAT(arena_ref, RefersToNullJavaObject()); +} + +} // namespace diff --git a/firestore/integration_test_internal/src/document_reference_test.cc b/firestore/integration_test_internal/src/document_reference_test.cc index cffda408ab..6ecb257a05 100644 --- a/firestore/integration_test_internal/src/document_reference_test.cc +++ b/firestore/integration_test_internal/src/document_reference_test.cc @@ -59,7 +59,7 @@ TEST_F(DocumentReferenceTest, RecoverFirestore) { DocumentReference doc = Document(); ASSERT_EQ(db, doc.firestore()); // Sanity check - jni::Object doc_java = GetInternal(doc)->ToJava(); + jni::Local doc_java = GetInternal(doc)->ToJava(); result = DocumentReferenceInternal::Create(env, doc_java); ASSERT_EQ(db, result.firestore()); } diff --git a/firestore/integration_test_internal/src/field_value_test.cc b/firestore/integration_test_internal/src/field_value_test.cc index bd82c1aa90..e8ca703ef3 100644 --- a/firestore/integration_test_internal/src/field_value_test.cc +++ b/firestore/integration_test_internal/src/field_value_test.cc @@ -367,5 +367,14 @@ TEST_F(FieldValueTest, TestIncrementChoosesTheCorrectType) { // clang-format on } +// Regression test for https://github.com/firebase/firebase-unity-sdk/issues/569 +// As long as this test doesn't crash, then it passes. +TEST_F(FieldValueTest, GlobalRefsExhaustion) { + std::vector numbers; + for (int i = 0; i < 60000; i++) { + numbers.push_back(FieldValue::Integer(i)); + } +} + } // namespace firestore } // namespace firebase diff --git a/firestore/src/android/document_reference_android.cc b/firestore/src/android/document_reference_android.cc index f62e7f7407..c5d5176e44 100644 --- a/firestore/src/android/document_reference_android.cc +++ b/firestore/src/android/document_reference_android.cc @@ -147,14 +147,14 @@ Future DocumentReferenceInternal::Set(const MapFieldValue& data, Env env = GetEnv(); FieldValueInternal map_value(data); Local java_options = SetOptionsInternal::Create(env, options); - Local task = env.Call(obj_, kSet, map_value, java_options); + Local task = env.Call(obj_, kSet, map_value.ToJava(), java_options); return promises_.NewFuture(env, AsyncFn::kSet, task); } Future DocumentReferenceInternal::Update(const MapFieldValue& data) { Env env = GetEnv(); FieldValueInternal map_value(data); - Local task = env.Call(obj_, kUpdate, map_value); + Local task = env.Call(obj_, kUpdate, map_value.ToJava()); return promises_.NewFuture(env, AsyncFn::kUpdate, task); } diff --git a/firestore/src/android/field_value_android.cc b/firestore/src/android/field_value_android.cc index 071188a726..160e95cb22 100644 --- a/firestore/src/android/field_value_android.cc +++ b/firestore/src/android/field_value_android.cc @@ -38,6 +38,7 @@ namespace firebase { namespace firestore { namespace { +using jni::ArenaRef; using jni::Array; using jni::ArrayList; using jni::Boolean; @@ -98,39 +99,45 @@ FieldValue FieldValueInternal::Create(Env& env, FieldValueInternal::FieldValueInternal() : cached_type_(Type::kNull) {} FieldValueInternal::FieldValueInternal(const Object& object) - : object_(object), cached_type_(Type::kNull) {} + : cached_type_(Type::kNull) { + Env env = GetEnv(); + object_.reset(env, object); +} FieldValueInternal::FieldValueInternal(Type type, const Object& object) - : object_(object), cached_type_(type) {} + : cached_type_(type) { + Env env = GetEnv(); + object_.reset(env, object); +} FieldValueInternal::FieldValueInternal(bool value) : cached_type_(Type::kBoolean) { Env env = GetEnv(); - object_ = Boolean::Create(env, value); + object_.reset(env, Boolean::Create(env, value)); } FieldValueInternal::FieldValueInternal(int64_t value) : cached_type_(Type::kInteger) { Env env = GetEnv(); - object_ = Long::Create(env, value); + object_.reset(env, Long::Create(env, value)); } FieldValueInternal::FieldValueInternal(double value) : cached_type_(Type::kDouble) { Env env = GetEnv(); - object_ = Double::Create(env, value); + object_.reset(env, Double::Create(env, value)); } FieldValueInternal::FieldValueInternal(Timestamp value) : cached_type_(Type::kTimestamp) { Env env = GetEnv(); - object_ = TimestampInternal::Create(env, value); + object_.reset(env, TimestampInternal::Create(env, value)); } FieldValueInternal::FieldValueInternal(std::string value) : cached_type_(Type::kString) { Env env = GetEnv(); - object_ = env.NewStringUtf(value); + object_.reset(env, env.NewStringUtf(value)); } // We do not initialize cached_blob_ with value here as the instance constructed @@ -140,20 +147,21 @@ FieldValueInternal::FieldValueInternal(std::string value) FieldValueInternal::FieldValueInternal(const uint8_t* value, size_t size) : cached_type_(Type::kBlob) { Env env = GetEnv(); - object_ = BlobInternal::Create(env, value, size); + object_.reset(env, BlobInternal::Create(env, value, size)); } FieldValueInternal::FieldValueInternal(DocumentReference value) : cached_type_{Type::kReference} { if (value.internal_ != nullptr) { - object_ = value.internal_->ToJava(); + Env env = GetEnv(); + object_.reset(env, value.internal_->ToJava()); } } FieldValueInternal::FieldValueInternal(GeoPoint value) : cached_type_(Type::kGeoPoint) { Env env = GetEnv(); - object_ = GeoPointInternal::Create(env, value); + object_.reset(env, GeoPointInternal::Create(env, value)); } FieldValueInternal::FieldValueInternal(const std::vector& value) @@ -164,7 +172,7 @@ FieldValueInternal::FieldValueInternal(const std::vector& value) // TODO(b/150016438): don't conflate invalid `FieldValue`s and null. list.Add(env, ToJava(element)); } - object_ = list; + object_.reset(env, list); } FieldValueInternal::FieldValueInternal(const MapFieldValue& value) @@ -176,63 +184,64 @@ FieldValueInternal::FieldValueInternal(const MapFieldValue& value) Local key = env.NewStringUtf(kv.first); map.Put(env, key, ToJava(kv.second)); } - object_ = map; + object_.reset(env, map); } Type FieldValueInternal::type() const { if (cached_type_ != Type::kNull) { return cached_type_; } - if (!object_) { - return Type::kNull; - } // We do not have any knowledge on the type yet. Check the runtime type with // each known type. Env env = GetEnv(); - if (env.IsInstanceOf(object_, Boolean::GetClass())) { + Local object = object_.get(env); + if (!object) { + return Type::kNull; + } + if (env.IsInstanceOf(object, Boolean::GetClass())) { cached_type_ = Type::kBoolean; return Type::kBoolean; } - if (env.IsInstanceOf(object_, Long::GetClass())) { + if (env.IsInstanceOf(object, Long::GetClass())) { cached_type_ = Type::kInteger; return Type::kInteger; } - if (env.IsInstanceOf(object_, Double::GetClass())) { + if (env.IsInstanceOf(object, Double::GetClass())) { cached_type_ = Type::kDouble; return Type::kDouble; } - if (env.IsInstanceOf(object_, TimestampInternal::GetClass())) { + if (env.IsInstanceOf(object, TimestampInternal::GetClass())) { cached_type_ = Type::kTimestamp; return Type::kTimestamp; } - if (env.IsInstanceOf(object_, String::GetClass())) { + if (env.IsInstanceOf(object, String::GetClass())) { cached_type_ = Type::kString; return Type::kString; } - if (env.IsInstanceOf(object_, BlobInternal::GetClass())) { + if (env.IsInstanceOf(object, BlobInternal::GetClass())) { cached_type_ = Type::kBlob; return Type::kBlob; } - if (env.IsInstanceOf(object_, DocumentReferenceInternal::GetClass())) { + if (env.IsInstanceOf(object, DocumentReferenceInternal::GetClass())) { cached_type_ = Type::kReference; return Type::kReference; } - if (env.IsInstanceOf(object_, GeoPointInternal::GetClass())) { + if (env.IsInstanceOf(object, GeoPointInternal::GetClass())) { cached_type_ = Type::kGeoPoint; return Type::kGeoPoint; } - if (env.IsInstanceOf(object_, List::GetClass())) { + if (env.IsInstanceOf(object, List::GetClass())) { cached_type_ = Type::kArray; return Type::kArray; } - if (env.IsInstanceOf(object_, Map::GetClass())) { + if (env.IsInstanceOf(object, Map::GetClass())) { cached_type_ = Type::kMap; return Type::kMap; } FIREBASE_ASSERT_MESSAGE(false, "Unsupported FieldValue type: %s", - Class::GetClassName(env, object_).c_str()); + Class::GetClassName(env, object_.get(env)).c_str()); return Type::kNull; } @@ -389,15 +398,25 @@ bool operator==(const FieldValueInternal& lhs, const FieldValueInternal& rhs) { } template -T FieldValueInternal::Cast(jni::Env& env, Type type) const { +Local FieldValueInternal::Cast(jni::Env& env, Type type) const { if (cached_type_ == Type::kNull) { - FIREBASE_ASSERT(env.IsInstanceOf(object_, T::GetClass())); + FIREBASE_ASSERT(env.IsInstanceOf(object_.get(env), T::GetClass())); cached_type_ = type; } else { FIREBASE_ASSERT(cached_type_ == type); } - auto typed_value = static_cast>(object_.get()); - return T(typed_value); + return object_.get(env).CastTo(); +} + +template <> +Local FieldValueInternal::Cast(jni::Env& env, Type type) const { + if (cached_type_ == Type::kNull) { + FIREBASE_ASSERT(env.IsInstanceOf(object_.get(env), String::GetClass())); + cached_type_ = type; + } else { + FIREBASE_ASSERT(cached_type_ == type); + } + return env.NewStringUtf(object_.get(env).ToString(env)); } Local> FieldValueInternal::MakeArray( @@ -411,8 +430,14 @@ Local> FieldValueInternal::MakeArray( Env FieldValueInternal::GetEnv() { return FirestoreInternal::GetEnv(); } -Object FieldValueInternal::ToJava(const FieldValue& value) { - return value.internal_ ? value.internal_->object_ : Object(); +Local FieldValueInternal::ToJava() const { + Env env = GetEnv(); + return object_.get(env); +} + +Local FieldValueInternal::ToJava(const FieldValue& value) { + Env env = GetEnv(); + return value.internal_ ? value.internal_->object_.get(env) : Local(); } } // namespace firestore diff --git a/firestore/src/android/field_value_android.h b/firestore/src/android/field_value_android.h index ceb49d23da..e5d8c99cef 100644 --- a/firestore/src/android/field_value_android.h +++ b/firestore/src/android/field_value_android.h @@ -20,11 +20,13 @@ #include #include #include +#include #include "firebase/firestore/geo_point.h" #include "firebase/firestore/timestamp.h" #include "firestore/src/include/firebase/firestore/document_reference.h" #include "firestore/src/include/firebase/firestore/field_value.h" +#include "firestore/src/jni/arena_ref.h" #include "firestore/src/jni/jni_fwd.h" #include "firestore/src/jni/object.h" #include "firestore/src/jni/ownership.h" @@ -87,7 +89,7 @@ class FieldValueInternal { std::vector array_value() const; MapFieldValue map_value() const; - const jni::Global& ToJava() const { return object_; } + jni::Local ToJava() const; static FieldValue Delete(); static FieldValue ServerTimestamp(); @@ -96,7 +98,7 @@ class FieldValueInternal { static FieldValue IntegerIncrement(int64_t by_value); static FieldValue DoubleIncrement(double by_value); - static jni::Object ToJava(const FieldValue& value); + static jni::Local ToJava(const FieldValue& value); private: friend class FirestoreInternal; @@ -107,7 +109,10 @@ class FieldValueInternal { // This performs a run-time `instanceof` check to verify that the object // has the type `T::GetClass()`. template - T Cast(jni::Env& env, Type type) const; + jni::Local Cast(jni::Env& env, Type type) const; + + template <> + jni::Local Cast(jni::Env& env, Type type) const; static jni::Local> MakeArray( jni::Env& env, const std::vector& elements); @@ -116,7 +121,7 @@ class FieldValueInternal { static jni::Env GetEnv(); - jni::Global object_; + jni::ArenaRef object_; // Below are cached type information. It is costly to get type info from // jobject of Object type. So we cache it if we have already known. @@ -124,19 +129,6 @@ class FieldValueInternal { mutable std::shared_ptr> cached_blob_; }; -inline jni::Object ToJava(const FieldValue& value) { - // This indirection is required to make use of the `friend` in FieldValue. - return FieldValueInternal::ToJava(value); -} - -inline jobject ToJni(const FieldValueInternal* value) { - return value->ToJava().get(); -} - -inline jobject ToJni(const FieldValueInternal& value) { - return value.ToJava().get(); -} - } // namespace firestore } // namespace firebase diff --git a/firestore/src/android/firestore_android.cc b/firestore/src/android/firestore_android.cc index e3317b6fe7..624e182081 100644 --- a/firestore/src/android/firestore_android.cc +++ b/firestore/src/android/firestore_android.cc @@ -61,6 +61,7 @@ #include "firestore/src/android/write_batch_android.h" #include "firestore/src/common/hard_assert_common.h" #include "firestore/src/include/firebase/firestore.h" +#include "firestore/src/jni/arena_ref.h" #include "firestore/src/jni/array.h" #include "firestore/src/jni/array_list.h" #include "firestore/src/jni/boolean.h" @@ -318,6 +319,7 @@ bool FirestoreInternal::Initialize(App* app) { jni::List::Initialize(loader); jni::Long::Initialize(loader); jni::Map::Initialize(loader); + jni::ArenaRef::Initialize(loader); InitializeFirestore(loader); InitializeFirestoreTasks(loader); diff --git a/firestore/src/android/query_android.cc b/firestore/src/android/query_android.cc index 81fdd418e0..b0dacecc3b 100644 --- a/firestore/src/android/query_android.cc +++ b/firestore/src/android/query_android.cc @@ -280,7 +280,8 @@ Query QueryInternal::Where(const FieldPath& field, const FieldValue& value) const { Env env = GetEnv(); Local java_field = FieldPathConverter::Create(env, field); - Local query = env.Call(obj_, method, java_field, ToJava(value)); + Local query = + env.Call(obj_, method, java_field, FieldValueInternal::ToJava(value)); return firestore_->NewQuery(env, query); } @@ -292,7 +293,7 @@ Query QueryInternal::Where(const FieldPath& field, size_t size = values.size(); Local java_values = ArrayList::Create(env, size); for (size_t i = 0; i < size; ++i) { - java_values.Add(env, ToJava(values[i])); + java_values.Add(env, FieldValueInternal::ToJava(values[i])); } Local java_field = FieldPathConverter::Create(env, field); @@ -350,7 +351,7 @@ Local> QueryInternal::ConvertFieldValues( size_t size = field_values.size(); Local> result = env.NewArray(size, Object::GetClass()); for (size_t i = 0; i < size; ++i) { - result.Set(env, i, ToJava(field_values[i])); + result.Set(env, i, FieldValueInternal::ToJava(field_values[i])); } return result; } diff --git a/firestore/src/android/util_android.cc b/firestore/src/android/util_android.cc index 5e9a534499..f7b6126caf 100644 --- a/firestore/src/android/util_android.cc +++ b/firestore/src/android/util_android.cc @@ -36,7 +36,7 @@ Local MakeJavaMap(Env& env, const MapFieldValue& data) { Local result = HashMap::Create(env); for (const auto& kv : data) { Local key = env.NewStringUtf(kv.first); - const Object& value = ToJava(kv.second); + Local value = FieldValueInternal::ToJava(kv.second); result.Put(env, key, value); } return result; @@ -49,7 +49,7 @@ UpdateFieldPathArgs MakeUpdateFieldPathArgs(Env& env, FIREBASE_DEV_ASSERT_MESSAGE(iter != end, "data must be non-empty"); Local first_field = FieldPathConverter::Create(env, iter->first); - const Object& first_value = ToJava(iter->second); + Local first_value = FieldValueInternal::ToJava(iter->second); ++iter; const auto size = std::distance(iter, end) * 2; @@ -58,13 +58,14 @@ UpdateFieldPathArgs MakeUpdateFieldPathArgs(Env& env, int index = 0; for (; iter != end; ++iter) { Local field = FieldPathConverter::Create(env, iter->first); - const Object& value = ToJava(iter->second); + Local value = FieldValueInternal::ToJava(iter->second); varargs.Set(env, index++, field); varargs.Set(env, index++, value); } - return UpdateFieldPathArgs{Move(first_field), first_value, Move(varargs)}; + return UpdateFieldPathArgs{Move(first_field), Move(first_value), + Move(varargs)}; } } // namespace firestore diff --git a/firestore/src/android/util_android.h b/firestore/src/android/util_android.h index c0e43c0ae0..2dcd03d32b 100644 --- a/firestore/src/android/util_android.h +++ b/firestore/src/android/util_android.h @@ -70,7 +70,7 @@ jni::Local MakeJavaMap(jni::Env& env, const MapFieldValue& data); */ struct UpdateFieldPathArgs { jni::Local first_field; - jni::Object first_value; + jni::Local first_value; jni::Local> varargs; }; diff --git a/firestore/src/android/wrapper.cc b/firestore/src/android/wrapper.cc index c4304abe29..0d02edec8f 100644 --- a/firestore/src/android/wrapper.cc +++ b/firestore/src/android/wrapper.cc @@ -29,14 +29,18 @@ namespace firebase { namespace firestore { namespace { +using jni::ArenaRef; using jni::Env; +using jni::Local; using jni::Object; } // namespace Wrapper::Wrapper(FirestoreInternal* firestore, const Object& obj) - : firestore_(firestore), obj_(obj) { + : firestore_(firestore) { FIREBASE_ASSERT(obj); + Env env = GetEnv(); + obj_.reset(env, obj); } Wrapper::Wrapper() { @@ -58,8 +62,9 @@ Wrapper::~Wrapper() = default; jni::Env Wrapper::GetEnv() const { return firestore_->GetEnv(); } -Object Wrapper::ToJava(const FieldValue& value) { - return FieldValueInternal::ToJava(value); +Local Wrapper::ToJava() const { + Env env = GetEnv(); + return obj_.get(env); } } // namespace firestore diff --git a/firestore/src/android/wrapper.h b/firestore/src/android/wrapper.h index b1da5fce65..97454579f6 100644 --- a/firestore/src/android/wrapper.h +++ b/firestore/src/android/wrapper.h @@ -17,6 +17,7 @@ #ifndef FIREBASE_FIRESTORE_SRC_ANDROID_WRAPPER_H_ #define FIREBASE_FIRESTORE_SRC_ANDROID_WRAPPER_H_ +#include "firestore/src/jni/arena_ref.h" #include "firestore/src/jni/jni_fwd.h" #include "firestore/src/jni/object.h" #include "firestore/src/jni/ownership.h" @@ -44,9 +45,7 @@ class Wrapper { Wrapper& operator=(Wrapper&& wrapper) = delete; FirestoreInternal* firestore_internal() { return firestore_; } - const jni::Object& ToJava() const { return obj_; } - - static jni::Object ToJava(const FieldValue& value); + jni::Local ToJava() const; protected: // Default constructor. Subclass is expected to set the obj_ a meaningful @@ -59,7 +58,7 @@ class Wrapper { jni::Env GetEnv() const; FirestoreInternal* firestore_ = nullptr; // not owning - jni::Global obj_; + jni::ArenaRef obj_; private: friend class FirestoreInternal; diff --git a/firestore/src/android/write_batch_android.cc b/firestore/src/android/write_batch_android.cc index 31b659308f..9eeadd0616 100644 --- a/firestore/src/android/write_batch_android.cc +++ b/firestore/src/android/write_batch_android.cc @@ -107,8 +107,9 @@ Future WriteBatchInternal::Commit() { return promises_.NewFuture(env, AsyncFn::kCommit, task); } -jni::Object WriteBatchInternal::ToJava(const DocumentReference& reference) { - return reference.internal_ ? reference.internal_->ToJava() : jni::Object(); +Local WriteBatchInternal::ToJava( + const DocumentReference& reference) { + return reference.internal_ ? reference.internal_->ToJava() : Local(); } } // namespace firestore diff --git a/firestore/src/android/write_batch_android.h b/firestore/src/android/write_batch_android.h index 86c7247bbc..e309ae3d78 100644 --- a/firestore/src/android/write_batch_android.h +++ b/firestore/src/android/write_batch_android.h @@ -64,7 +64,7 @@ class WriteBatchInternal : public Wrapper { */ // TODO(mcg): Move this out of WriteBatchInternal // This needs to be here now because of existing friend relationships. - static jni::Object ToJava(const DocumentReference& reference); + static jni::Local ToJava(const DocumentReference& reference); PromiseFactory promises_; }; diff --git a/firestore/src/jni/arena_ref.cc b/firestore/src/jni/arena_ref.cc new file mode 100644 index 0000000000..d8afb28fd6 --- /dev/null +++ b/firestore/src/jni/arena_ref.cc @@ -0,0 +1,246 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include "firestore/src/jni/arena_ref.h" + +#include + +#include + +#include "app/src/assert.h" +#include "app/src/log.h" +#include "firestore/src/jni/env.h" +#include "firestore/src/jni/loader.h" +#include "firestore/src/jni/object.h" +#include "firestore/src/jni/ownership.h" + +namespace firebase { +namespace firestore { +namespace jni { + +namespace { + +constexpr char kObjectArenaClassName[] = PROGUARD_KEEP_CLASS + "com/google/firebase/firestore/internal/cpp/ObjectArena"; + +// A convenience class for repeatedly loading Java JNI method IDs from a given +// Java class. +class MethodLoader { + public: + MethodLoader(Loader& loader, jclass java_class) + : loader_(loader), java_class_(java_class) {} + + jmethodID LoadMethodId(const char* name, const char* signature) const { + if (!loader_.ok()) { + return {}; + } + jmethodID method_id = + loader_.env()->GetStaticMethodID(java_class_, name, signature); + if (!loader_.ok()) { + return {}; + } + return method_id; + } + + private: + Loader& loader_; + jclass java_class_{}; +}; + +// Helper class for calling methods on the Java `ObjectArena` class. +class ObjectArena { + public: + // Disable copy and move, since this class has a global singleton instance. + ObjectArena(const ObjectArena&) = delete; + ObjectArena(ObjectArena&&) = delete; + ObjectArena& operator=(const ObjectArena&) = delete; + ObjectArena& operator=(ObjectArena&&) = delete; + + // Delete the destructor to prevent the global singleton instance from being + // deleted. + ~ObjectArena() = delete; + + // Initialize the global singleton instance of this class. + // This function must be invoked prior to invoking any other static or + // non-static member functions in this class. + // This function is NOT thread-safe, and must not be invoked concurrently. + static void Initialize(Loader& loader) { + GetOrCreateSingletonInstance().Initialize_(loader); + } + + // Returns a reference to the global singleton instance of this class. + // Note that `Initialize()` must be called before this function. + static const ObjectArena& GetInstance() { + const ObjectArena& instance = GetOrCreateSingletonInstance(); + FIREBASE_ASSERT_MESSAGE(instance.initialized_, + "ObjectArena should be initialized"); + return instance; + } + + // Calls the Java method ObjectArena.set() with the given arguments. + void Set(Env& env, jlong id, jobject value) const { + if (!env.ok()) { + return; + } + env.get()->CallStaticVoidMethod(java_class_, set_, id, value); + } + + // Calls the Java method ObjectArena.get() with the given argument, returning + // whatever it returns. + jobject Get(Env& env, jlong id) const { + if (!env.ok()) { + return nullptr; + } + jobject result = env.get()->CallStaticObjectMethod(java_class_, get_, id); + if (!env.ok()) { + return nullptr; + } + return result; + } + + // Calls the Java method ObjectArena.remove() with the given argument. + void Remove(Env& env, jlong id) const { + if (!env.ok()) { + return; + } + env.get()->CallStaticVoidMethod(java_class_, remove_, id); + } + + private: + // Make the constructor private so that instances cannot be created, except + // for the global singleton instance. + ObjectArena() = default; + + static ObjectArena& GetOrCreateSingletonInstance() { + // Create the global singleton instance on the heap so that the destructor + // is never invoked. This avoids potential use-after-free issues on + // application shutdown where some other static object's destructor tries to + // use the global singleton instance _after_ its destructor has run. + static auto* instance = new ObjectArena; + return *instance; + } + + void Initialize_(Loader& loader) { + if (initialized_) { + return; + } + + jclass java_class = java_class_; + if (!java_class) { + java_class = loader.LoadClass(kObjectArenaClassName); + if (!loader.ok()) { + return; + } + java_class = (jclass)loader.env()->NewGlobalRef(java_class); + if (!loader.ok()) { + return; + } + java_class_ = java_class; + } + + MethodLoader method_loader(loader, java_class); + get_ = method_loader.LoadMethodId("get", "(J)Ljava/lang/Object;"); + set_ = method_loader.LoadMethodId("set", "(JLjava/lang/Object;)V"); + remove_ = method_loader.LoadMethodId("remove", "(J)V"); + + initialized_ = loader.ok(); + } + + // Wrap the member variables below in `std::atomic` so that their values set + // by `Initialize()` are guaranteed to be visible to all threads. + std::atomic java_class_{}; + std::atomic get_{}; + std::atomic set_{}; + std::atomic remove_{}; + std::atomic initialized_{false}; +}; + +} // namespace + +// Manages an entry in the Java `ObjectArena` map, creating the entry in the +// constructor from a uniquely-generated `jlong` value, and removing the entry +// in the destructor. +class ArenaRef::ObjectArenaEntry final { + public: + ObjectArenaEntry(Env& env, jobject object) : id_(GenerateUniqueId()) { + ObjectArena::GetInstance().Set(env, id_, object); + } + + // Delete the copy and move constructors and assignment operators to enforce + // the requirement that every instance has a distinct value for its `id_` + // member variable. + ObjectArenaEntry(const ObjectArenaEntry&) = delete; + ObjectArenaEntry(ObjectArenaEntry&&) = delete; + ObjectArenaEntry& operator=(const ObjectArenaEntry&) = delete; + ObjectArenaEntry& operator=(ObjectArenaEntry&&) = delete; + + ~ObjectArenaEntry() { + Env env; + ExceptionClearGuard exception_clear_guard(env); + ObjectArena::GetInstance().Remove(env, id_); + + if (!env.ok()) { + env.get()->ExceptionDescribe(); + env.get()->ExceptionClear(); + firebase::LogWarning("~ObjectArenaEntry(): ObjectArena::Remove() failed"); + } + } + + Local GetReferent(Env& env) const { + jobject result = ObjectArena::GetInstance().Get(env, id_); + if (!env.ok()) { + return {}; + } + return {env.get(), result}; + } + + private: + static jlong GenerateUniqueId() { + // Start the IDs at a large number with an easily-identifiable prefix to + // make it easier to determine if an instance's ID is "valid" during + // debugging. Even though this initial value is large, it still leaves room + // for almost nine quintillion (8,799,130,036,854,775,807) positive values, + // which should be enough :) + static std::atomic gNextId(424242000000000000L); + return gNextId.fetch_add(1); + } + + // Mark `id_` as `const` to reinforce the expectation that instances are + // immutable and do not support copy/move assignment. + const jlong id_; +}; + +ArenaRef::ArenaRef(Env& env, jobject object) { reset(env, object); } + +Local ArenaRef::get(Env& env) const { + if (!entry_) { + return {}; + } + return entry_->GetReferent(env); +} + +void ArenaRef::reset(Env& env, const Object& object) { + reset(env, object.get()); +} + +void ArenaRef::reset(Env& env, jobject object) { + entry_ = std::make_shared(env, object); +} + +void ArenaRef::Initialize(Loader& loader) { ObjectArena::Initialize(loader); } + +} // namespace jni +} // namespace firestore +} // namespace firebase diff --git a/firestore/src/jni/arena_ref.h b/firestore/src/jni/arena_ref.h new file mode 100644 index 0000000000..9232a4d830 --- /dev/null +++ b/firestore/src/jni/arena_ref.h @@ -0,0 +1,116 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef FIREBASE_FIRESTORE_SRC_JNI_ARENA_REF_H_ +#define FIREBASE_FIRESTORE_SRC_JNI_ARENA_REF_H_ + +#include + +#include + +#include "firestore/src/jni/jni_fwd.h" + +namespace firebase { +namespace firestore { +namespace jni { + +/** + * An RAII wrapper for a global JNI reference, that automatically deletes the + * JNI global reference when it goes out of scope. + * + * This class is mostly a drop-in replacement for the `Global` template class; + * however, `ArenaRef` has the added benefit that it does _not_ consume a JNI + * global reference from Android's limitied global reference pool. In contrast, + * each `Global` instance consumes one JNI global reference. + * + * Instead, `ArenaRef` just stores a `long` unique ID, which is used as a key + * into a Java HashMap. When the referenced object is needed then `ArenaRef` + * retrieves the object from the hash table by its ID. + * + * This class supports move and copy semantics. Moves and copies are *very* + * efficient: they have the same cost as the corresponding operation on a + * std::shared_ptr (which is very small compared to a JNI call). + * + * This class is not thread safe; concurrent use of an instance of this class + * without external synchronization is undefined behavior. However, distinct + * instances can be created concurrently in multiple threads as access to the + * backing HashMap _is_ synchronized. + */ +class ArenaRef final { + public: + /** + * Creates an `ArenaRef` that does not refer to any object. + */ + ArenaRef() = default; + + ~ArenaRef() = default; + + /** + * Creates an `ArenaRef` that does not refer to the given object. + * + * The given `Env` is used to perform the JNI call to insert the key/value + * pair into the backing Java `HashMap`. The given `jobject` may be null, in + * which case retrieving the object will return a null value. + * + * If the JNI call to insert the key/value pair into the backing Java HashMap + * fails then this object will be in the same state as a default-constructed + * instance. + */ + ArenaRef(Env&, jobject); + + // Copy and move construction and assignment is supported. + ArenaRef(const ArenaRef&) = default; + ArenaRef(ArenaRef&&) = default; + ArenaRef& operator=(const ArenaRef&) = default; + ArenaRef& operator=(ArenaRef&&) = default; + + /** + * Returns the Java object referenced by this `ArenaRef`. + * + * This function returns a Java "null" object in the following scenarios: + * 1. This object was created using the default constructor. + * 2. The object given to the constructor was a Java "null" object. + * 3. The JNI call to retrieve the object from the backing Java HashMap fails, + * such as if there is a pending Java exception. + */ + Local get(Env&) const; + + /** + * Changes this object's referenced Java object to the given Java object. + * + * Subsequent invocations of `get()` will return the given object. The given + * object may be a Java "null" object. + * + * If invoked with a pending Java exception then this `ArenaRef` is set to a + * `null` Java object reference. + */ + void reset(Env& env, const Object&); + + static void Initialize(Loader&); + + private: + class ObjectArenaEntry; + + void reset(Env& env, jobject); + + std::shared_ptr entry_; +}; + +} // namespace jni +} // namespace firestore +} // namespace firebase + +#endif // FIREBASE_FIRESTORE_SRC_JNI_ARENA_REF_H_ diff --git a/firestore/src/jni/env.h b/firestore/src/jni/env.h index a7c3354d68..50be109da2 100644 --- a/firestore/src/jni/env.h +++ b/firestore/src/jni/env.h @@ -24,6 +24,7 @@ #include #include "app/meta/move.h" +#include "firestore/src/jni/arena_ref.h" #include "firestore/src/jni/call_traits.h" #include "firestore/src/jni/class.h" #include "firestore/src/jni/declaration.h" @@ -240,6 +241,13 @@ class Env { ToJni(Forward(args))...); } + template + ResultType Call(const ArenaRef& object, + const Method& method, + Args&&... args) { + return Call(object.get(*this), method, Forward(args)...); + } + // MARK: Accessing Static Fields jfieldID GetStaticFieldId(const Class& clazz, diff --git a/firestore/src/jni/loader.h b/firestore/src/jni/loader.h index 6eb665354b..06c56e9ad1 100644 --- a/firestore/src/jni/loader.h +++ b/firestore/src/jni/loader.h @@ -159,6 +159,8 @@ class Loader { void Unload(); + JNIEnv* env() { return env_; } + private: App* app_ = nullptr; JNIEnv* env_ = nullptr; diff --git a/firestore/src_java/com/google/firebase/firestore/internal/cpp/ObjectArena.java b/firestore/src_java/com/google/firebase/firestore/internal/cpp/ObjectArena.java new file mode 100644 index 0000000000..ed02fff202 --- /dev/null +++ b/firestore/src_java/com/google/firebase/firestore/internal/cpp/ObjectArena.java @@ -0,0 +1,48 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.firebase.firestore.internal.cpp; + +import java.util.HashMap; + +/** + * A companion class used by the C++ class firebase::firestore::jni::ArenaRef to + * map long primitive values to Objects. + * + * Technically, this could have been written using JNI calling HashMap methods + * directly; however, that is extremely slow and sidesteps an amazing + * optimization done by Android's ART runtime that elides the boxing and + * unboxing of the Long keys. + */ +public final class ObjectArena { + private static final HashMap map = new HashMap<>(); + + private ObjectArena() { + throw new UnsupportedOperationException(); + } + + public static synchronized void set(long id, Object object) { + map.put(id, object); + } + + public static synchronized Object get(long id) { + return map.get(id); + } + + public static synchronized void remove(long id) { + map.remove(id); + } +} diff --git a/release_build_files/readme.md b/release_build_files/readme.md index cc162d6479..5e3786424c 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -644,6 +644,10 @@ code. time zone name in the current system language contains an accented character or apostrophe. This adds a requirement for applications using Remote Config on Windows desktop to link the "icu.dll" system library. + - Firestore (Android): Fix the intermittent global references exhaustion + crash when working with documents with a large number of keys and/or large + map and/or array fields. + ([#NNNN](https://github.com/firebase/firebase-cpp-sdk/pull/NNNN)). ### 11.1.0 - Changes From 6f44f92a7f273da5663f762fad93b05923f2f2e3 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 27 Jun 2023 13:16:28 -0400 Subject: [PATCH 2/3] release_build_files/readme.md: update PR number --- release_build_files/readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release_build_files/readme.md b/release_build_files/readme.md index 5e3786424c..58ad89bba2 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -647,7 +647,7 @@ code. - Firestore (Android): Fix the intermittent global references exhaustion crash when working with documents with a large number of keys and/or large map and/or array fields. - ([#NNNN](https://github.com/firebase/firebase-cpp-sdk/pull/NNNN)). + ([#1364](https://github.com/firebase/firebase-cpp-sdk/pull/1364)). ### 11.1.0 - Changes From 9d666801b8d33676b14a08bff0fde1ab87db84f1 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Fri, 30 Jun 2023 11:11:46 -0400 Subject: [PATCH 3/3] update comments as suggested by code reviewer --- firestore/src/jni/arena_ref.h | 12 ++++++++++ .../firestore/internal/cpp/ObjectArena.java | 22 ++++++++++++++----- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/firestore/src/jni/arena_ref.h b/firestore/src/jni/arena_ref.h index 9232a4d830..c9e16fd75a 100644 --- a/firestore/src/jni/arena_ref.h +++ b/firestore/src/jni/arena_ref.h @@ -99,6 +99,18 @@ class ArenaRef final { */ void reset(Env& env, const Object&); + /** + * Performs one-time initialization of the `ArenaRef` class. + * + * This function _must_ be called before any instances of `ArenaRef` are + * created. Violating this requirement results in undefined behavior. + * + * It is safe to call this function multiple times: subsequent invocations + * have no effect. + * + * This function is _not_ thread-safe; calling concurrently from multiple + * threads results in undefined behavior. + */ static void Initialize(Loader&); private: diff --git a/firestore/src_java/com/google/firebase/firestore/internal/cpp/ObjectArena.java b/firestore/src_java/com/google/firebase/firestore/internal/cpp/ObjectArena.java index ed02fff202..c17085eb03 100644 --- a/firestore/src_java/com/google/firebase/firestore/internal/cpp/ObjectArena.java +++ b/firestore/src_java/com/google/firebase/firestore/internal/cpp/ObjectArena.java @@ -19,13 +19,23 @@ import java.util.HashMap; /** - * A companion class used by the C++ class firebase::firestore::jni::ArenaRef to - * map long primitive values to Objects. + * A companion class used by the C++ class + * {@code firebase::firestore::jni::ArenaRef} to map {@code long} primitive + * values to objects. * - * Technically, this could have been written using JNI calling HashMap methods - * directly; however, that is extremely slow and sidesteps an amazing - * optimization done by Android's ART runtime that elides the boxing and - * unboxing of the Long keys. + * This class cannot be instantiated; all uses must be done through the static + * methods. This effectively serves as a global singleton map that exists for + * the entire lifetime of the Java application. + * + * Technically, the logic in this class could have been written entirely in C++, + * using JNI to create the {@code HashMap} and call its methods; however, this + * proved to be slow, mostly due to the boxing and unboxing of the {@code long} + * primitive values to and from {@code Long} objects, respectively. By instead + * writing the class in Java, Android's ART runtime elides the boxing and + * unboxing since it can prove to itself that the code does not rely on object + * identity of the {@code Long} objects. This improves the performance by an + * order of magnitude compared the the equivalent JNI calls, which cannot have + * the boxing and unboxing optimized away. */ public final class ObjectArena { private static final HashMap map = new HashMap<>();