From 747d4526495c20ec23f25119eaa34ca4e6c9f724 Mon Sep 17 00:00:00 2001 From: a-maurice Date: Wed, 5 Jun 2024 12:14:38 -0700 Subject: [PATCH 1/3] Add null checks in Messaging task completions --- messaging/src/android/cpp/messaging.cc | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/messaging/src/android/cpp/messaging.cc b/messaging/src/android/cpp/messaging.cc index c0ad63040f..e4ed258150 100644 --- a/messaging/src/android/cpp/messaging.cc +++ b/messaging/src/android/cpp/messaging.cc @@ -826,8 +826,12 @@ static void CompleteVoidCallback(JNIEnv* env, jobject result, FutureHandle handle(future_id); Error error = (result_code == util::kFutureResultSuccess) ? kErrorNone : kErrorUnknown; - ReferenceCountedFutureImpl* api = FutureData::Get()->api(); - api->Complete(handle, error, status_message); + if (FutureData::Get() && FutureData::Get()->api()) { + ReferenceCountedFutureImpl* api = FutureData::Get()->api(); + api->Complete(handle, error, status_message); + } else { + LogWarning("Failed to complete Future as it was likely already deleted."); + } if (result) env->DeleteLocalRef(result); } @@ -843,8 +847,12 @@ static void CompleteStringCallback(JNIEnv* env, jobject result, SafeFutureHandle* handle = reinterpret_cast*>(callback_data); Error error = success ? kErrorNone : kErrorUnknown; - ReferenceCountedFutureImpl* api = FutureData::Get()->api(); - api->CompleteWithResult(*handle, error, status_message, result_value); + if (FutureData::Get() && FutureData::Get()->api()) { + ReferenceCountedFutureImpl* api = FutureData::Get()->api(); + api->CompleteWithResult(*handle, error, status_message, result_value); + } else { + LogWarning("Failed to complete Future as it was likely already deleted."); + } delete handle; } From c64057c0faf0c617c81428c4608a03bd42688105 Mon Sep 17 00:00:00 2001 From: a-maurice Date: Thu, 6 Jun 2024 15:03:46 -0700 Subject: [PATCH 2/3] Add mutex and update readme --- messaging/src/android/cpp/messaging.cc | 10 +++++++++- release_build_files/readme.md | 5 +++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/messaging/src/android/cpp/messaging.cc b/messaging/src/android/cpp/messaging.cc index e4ed258150..3c0c42712a 100644 --- a/messaging/src/android/cpp/messaging.cc +++ b/messaging/src/android/cpp/messaging.cc @@ -124,6 +124,9 @@ static std::string* g_lockfile_path; static Mutex* g_file_locker_mutex; +// Mutex for managing if FutureData is safe to use, or if it has been deleted. +static Mutex g_future_data_mutex; + static const char* kMessagingNotInitializedError = "Messaging not initialized."; static void HandlePendingSubscriptions(); @@ -706,7 +709,10 @@ void Terminate() { SetListener(nullptr); ReleaseClasses(env); util::Terminate(env); - FutureData::Destroy(); + { + MutexLock(g_future_data_mutex); + FutureData::Destroy(); + } } // Start a service which will communicate with the Firebase Cloud Messaging @@ -826,6 +832,7 @@ static void CompleteVoidCallback(JNIEnv* env, jobject result, FutureHandle handle(future_id); Error error = (result_code == util::kFutureResultSuccess) ? kErrorNone : kErrorUnknown; + MutexLock(g_future_data_mutex); if (FutureData::Get() && FutureData::Get()->api()) { ReferenceCountedFutureImpl* api = FutureData::Get()->api(); api->Complete(handle, error, status_message); @@ -847,6 +854,7 @@ static void CompleteStringCallback(JNIEnv* env, jobject result, SafeFutureHandle* handle = reinterpret_cast*>(callback_data); Error error = success ? kErrorNone : kErrorUnknown; + MutexLock(g_future_data_mutex); if (FutureData::Get() && FutureData::Get()->api()) { ReferenceCountedFutureImpl* api = FutureData::Get()->api(); api->CompleteWithResult(*handle, error, status_message, result_value); diff --git a/release_build_files/readme.md b/release_build_files/readme.md index 2edf2fe37c..32895c900a 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -631,6 +631,11 @@ workflow use only during the development of your app, not for publicly shipping code. ## Release Notes +### Upcoming +- Changes + - Messaging (Android): Addressed potential race condition on receiving + messages after cleanup. + ### 12.0.0 - Changes - General (Android): Update to Firebase Android BoM version 33.0.0. From 2d83d33b78ef369248a6dc7361abaa2ce7bbe9a6 Mon Sep 17 00:00:00 2001 From: a-maurice Date: Thu, 6 Jun 2024 15:16:11 -0700 Subject: [PATCH 3/3] Update messaging.cc --- messaging/src/android/cpp/messaging.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/messaging/src/android/cpp/messaging.cc b/messaging/src/android/cpp/messaging.cc index 3c0c42712a..43c8035d71 100644 --- a/messaging/src/android/cpp/messaging.cc +++ b/messaging/src/android/cpp/messaging.cc @@ -710,7 +710,7 @@ void Terminate() { ReleaseClasses(env); util::Terminate(env); { - MutexLock(g_future_data_mutex); + MutexLock lock(g_future_data_mutex); FutureData::Destroy(); } } @@ -832,7 +832,7 @@ static void CompleteVoidCallback(JNIEnv* env, jobject result, FutureHandle handle(future_id); Error error = (result_code == util::kFutureResultSuccess) ? kErrorNone : kErrorUnknown; - MutexLock(g_future_data_mutex); + MutexLock lock(g_future_data_mutex); if (FutureData::Get() && FutureData::Get()->api()) { ReferenceCountedFutureImpl* api = FutureData::Get()->api(); api->Complete(handle, error, status_message); @@ -854,7 +854,7 @@ static void CompleteStringCallback(JNIEnv* env, jobject result, SafeFutureHandle* handle = reinterpret_cast*>(callback_data); Error error = success ? kErrorNone : kErrorUnknown; - MutexLock(g_future_data_mutex); + MutexLock lock(g_future_data_mutex); if (FutureData::Get() && FutureData::Get()->api()) { ReferenceCountedFutureImpl* api = FutureData::Get()->api(); api->CompleteWithResult(*handle, error, status_message, result_value);