Database, Functions, Storage (Android): Fix crash on failed init and embedded class lookup

app/src/util_android.cc

@@ -297,12 +297,23 @@ static jobject GetParentLoader() {
   return g_class_loaders->back();
 }
 
+// Convert a JNI class name (e.g. "java/lang/String"), as used by
+// JNIEnv::FindClass(), to the binary name (e.g. "java.lang.String") expected by
+// ClassLoader methods.
+static std::string JniClassNameToBinaryName(const char* class_name) {
+  if (!class_name) return std::string();
+  std::string binary_name(class_name);
+  std::replace(binary_name.begin(), binary_name.end(), '/', '.');
+  return binary_name;
+}
+
 // Try to find a class in the list of class loaders.
 static jclass FindOrLoadClassFromLoaders(JNIEnv* env, const char* class_name) {
   assert(g_class_loaders);
   static const class_loader::Method kFindLoadClassMethods[] = {
       class_loader::kFindLoadedClass, class_loader::kLoadClass};
-  jstring class_name_object = env->NewStringUTF(class_name);
+  jstring class_name_object =
+      env->NewStringUTF(JniClassNameToBinaryName(class_name).c_str());
   jclass class_object = nullptr;
   for (size_t i = 0; i < FIREBASE_ARRAYSIZE(kFindLoadClassMethods); ++i) {
     for (auto it = g_class_loaders->begin();
@@ -1669,8 +1680,6 @@ jclass FindClassInFiles(
       firebase::util::JniStringToString(env, cache_dir_path_jstring);
 
 #if defined(FIREBASE_ANDROID_FOR_DESKTOP)
-  static const char kPathSeparator = '/';
-
   jobject cache_uri =
       env->CallObjectMethod(cache_dir, file::GetMethodId(file::kToUri));
   CheckAndClearJniExceptions(env);
@@ -1698,11 +1707,9 @@ jclass FindClassInFiles(
       url_path_array, GetParentLoader());
   env->DeleteLocalRef(url_path_array);
 
-  std::string class_name_str(class_name);
-  std::replace(class_name_str.begin(), class_name_str.end(), kPathSeparator,
-               '.');
-  LogDebug("Load class %s (a.k.a. %s)", class_name_str.c_str(), class_name);
-  jstring class_name_object = env->NewStringUTF(class_name_str.c_str());
+  std::string binary_name = JniClassNameToBinaryName(class_name);
+  LogDebug("Load class %s (a.k.a. %s)", binary_name.c_str(), class_name);
+  jstring class_name_object = env->NewStringUTF(binary_name.c_str());
   jclass loaded_class = static_cast<jclass>(env->CallObjectMethod(
       class_loader_obj,
       url_class_loader::GetMethodId(url_class_loader::kLoadClass),
@@ -1745,8 +1752,9 @@ jclass FindClassInFiles(
   env->DeleteLocalRef(code_cache_dir_path);
   env->DeleteLocalRef(dex_path_string);
 
-  LogDebug("Load class %s", class_name);
-  jstring class_name_object = env->NewStringUTF(class_name);
+  std::string binary_name = JniClassNameToBinaryName(class_name);
+  LogDebug("Load class %s (a.k.a. %s)", binary_name.c_str(), class_name);
+  jstring class_name_object = env->NewStringUTF(binary_name.c_str());
   jclass loaded_class = static_cast<jclass>(env->CallObjectMethod(
       class_loader_obj,
       dex_class_loader::GetMethodId(dex_class_loader::kLoadClass),

database/src/common/database.cc

@@ -122,15 +122,20 @@ void Database::DeleteInternal() {
 
   if (!internal_) return;
 
+  // An instance whose initialization failed was never registered with the
+  // App's CleanupNotifier or added to g_databases, and app() may be null.
+  if (!internal_->initialized()) {
+    delete internal_;
+    internal_ = nullptr;
+    return;
+  }
+
   App* my_app = app();
   std::string url = internal_->constructor_url();
 
-  // Only need to unregister if internal_ is initialized
-  if (internal_->initialized()) {
-    CleanupNotifier* app_notifier = CleanupNotifier::FindByOwner(my_app);
-    assert(app_notifier);
-    app_notifier->UnregisterObject(this);
-  }
+  CleanupNotifier* app_notifier = CleanupNotifier::FindByOwner(my_app);
+  assert(app_notifier);
+  app_notifier->UnregisterObject(this);
 
   {
     MutexLock db_ref_lock(internal::g_database_reference_constructor_mutex);

functions/src/common/functions.cc

@@ -122,6 +122,14 @@ void Functions::DeleteInternal() {
 
   if (!internal_) return;
 
+  // An instance whose initialization failed was never registered with the
+  // App's CleanupNotifier or added to g_functions, and app() may be null.
+  if (!internal_->initialized()) {
+    delete internal_;
+    internal_ = nullptr;
+    return;
+  }
+
   CleanupNotifier* app_notifier = CleanupNotifier::FindByOwner(app());
   assert(app_notifier);
   app_notifier->UnregisterObject(this);

release_build_files/readme.md

@@ -615,6 +615,12 @@ code.
 ## Release Notes
 ### Upcoming
 - Changes
+    - Database, Functions, Storage (Android): Fixed a crash in `GetInstance()`
+      when initialization fails. It now returns `nullptr` and reports
+      `kInitResultFailedMissingDependency`.
+    - General (Android): Fixed loading of embedded Java classes when the app
+      runs under a custom class loader setup, such as being hosted in another
+      app's process. (#1936)
     - General (Android): Fixed a potential buffer overflow in `LogException` when
       formatting long JNI exception messages.
 

storage/src/common/storage.cc

@@ -136,6 +136,14 @@ void Storage::DeleteInternal() {
   MutexLock lock(g_storages_lock);
   if (!internal_) return;
 
+  // An instance whose initialization failed was never registered with the
+  // App's CleanupNotifier or added to g_storages, and app() may be null.
+  if (!internal_->initialized()) {
+    delete internal_;
+    internal_ = nullptr;
+    return;
+  }
+
   CleanupNotifier* app_notifier = CleanupNotifier::FindByOwner(app());
   assert(app_notifier);
   app_notifier->UnregisterObject(this);