Fix Mutex destructors being incorrectly run at exit (cl/364325997) (#345)

Author: dconeybe

app/rest/transport_curl.cc

@@ -312,12 +312,12 @@ CurlThread* g_curl_thread = nullptr;
 int g_initialize_count = 0;
 
 // Mutex for Curl initialization.
-Mutex g_initialize_mutex;  // NOLINT
+Mutex* g_initialize_mutex = new Mutex();
 
 }  // namespace
 
 void InitTransportCurl() {
-  MutexLock lock(g_initialize_mutex);
+  MutexLock lock(*g_initialize_mutex);
   if (g_initialize_count == 0) {
     // Initialize curl.
     CURLcode global_init_code = curl_global_init(CURL_GLOBAL_ALL);
@@ -333,7 +333,7 @@ void InitTransportCurl() {
 }
 
 void CleanupTransportCurl() {
-  MutexLock lock(g_initialize_mutex);
+  MutexLock lock(*g_initialize_mutex);
   assert(g_initialize_count > 0);
   g_initialize_count--;
   if (g_initialize_count == 0) {

app/rest/util.cc

@@ -45,23 +45,23 @@ const char kGet[] = "GET";
 const char kPost[] = "POST";
 
 // Mutex for utils initialization and distribution of CURL pointers.
-static ::firebase::Mutex g_util_curl_mutex;  // NOLINT
+static ::firebase::Mutex* g_util_curl_mutex = new Mutex();
 
 CURL* g_curl_instance = nullptr;
 int g_init_ref_count = 0;
 
 void* CreateCurlPtr() {  // NOLINT
-  MutexLock lock(g_util_curl_mutex);
+  MutexLock lock(*g_util_curl_mutex);
   return curl_easy_init();
 }
 
 void DestroyCurlPtr(void* curl_ptr) {
-  MutexLock lock(g_util_curl_mutex);
+  MutexLock lock(*g_util_curl_mutex);
   curl_easy_cleanup(static_cast<CURL*>(curl_ptr));
 }
 
 void Initialize() {
-  MutexLock curl_lock(g_util_curl_mutex);
+  MutexLock curl_lock(*g_util_curl_mutex);
   assert(g_init_ref_count >= 0);
   if (g_init_ref_count == 0) {
     g_curl_instance = reinterpret_cast<CURL*>(CreateCurlPtr());
@@ -70,7 +70,7 @@ void Initialize() {
 }
 
 void Terminate() {
-  MutexLock curl_lock(g_util_curl_mutex);
+  MutexLock curl_lock(*g_util_curl_mutex);
   --g_init_ref_count;
   assert(g_init_ref_count >= 0);
   if (g_init_ref_count == 0) {

app/src/app_common.cc

@@ -255,7 +255,7 @@ class LibraryRegistry {
 };
 
 // Guards g_apps and g_default_app.
-static Mutex g_app_mutex;  // NOLINT
+static Mutex* g_app_mutex = new Mutex();
 static std::map<std::string, UniquePtr<AppData>>* g_apps;
 static App* g_default_app = nullptr;
 LibraryRegistry* LibraryRegistry::library_registry_ = nullptr;
@@ -265,7 +265,7 @@ App* AddApp(App* app, std::map<std::string, InitResult>* results) {
   assert(app);
   App* existing_app = FindAppByName(app->name());
   FIREBASE_ASSERT_RETURN(nullptr, !existing_app);
-  MutexLock lock(g_app_mutex);
+  MutexLock lock(*g_app_mutex);
   if (IsDefaultAppName(app->name())) {
     assert(!g_default_app);
     g_default_app = app;
@@ -309,7 +309,7 @@ App* AddApp(App* app, std::map<std::string, InitResult>* results) {
 
 App* FindAppByName(const char* name) {
   assert(name);
-  MutexLock lock(g_app_mutex);
+  MutexLock lock(*g_app_mutex);
   if (g_apps) {
     auto it = g_apps->find(std::string(name));
     if (it == g_apps->end()) return nullptr;
@@ -325,7 +325,7 @@ App* GetAnyApp() {
     return g_default_app;
   }
 
-  MutexLock lock(g_app_mutex);
+  MutexLock lock(*g_app_mutex);
   if (g_apps && !g_apps->empty()) {
     return g_apps->begin()->second->app;
   }
@@ -334,7 +334,7 @@ App* GetAnyApp() {
 
 void RemoveApp(App* app) {
   assert(app);
-  MutexLock lock(g_app_mutex);
+  MutexLock lock(*g_app_mutex);
   if (g_apps) {
     auto it = g_apps->find(std::string(app->name()));
     bool last_app = false;
@@ -369,7 +369,7 @@ void RemoveApp(App* app) {
 void DestroyAllApps() {
   std::vector<App*> apps_to_delete;
   App* const default_app = GetDefaultApp();
-  MutexLock lock(g_app_mutex);
+  MutexLock lock(*g_app_mutex);
   if (g_apps) {
     for (auto it = g_apps->begin(); it != g_apps->end(); ++it) {
       if (it->second->app != default_app)
@@ -391,15 +391,15 @@ bool IsDefaultAppName(const char* name) {
 }
 
 void RegisterLibrary(const char* library, const char* version) {
-  MutexLock lock(g_app_mutex);
+  MutexLock lock(*g_app_mutex);
   LibraryRegistry* registry = LibraryRegistry::Initialize();
   if (registry->RegisterLibrary(library, version)) {
     registry->UpdateUserAgent();
   }
 }
 
 void RegisterLibrariesFromUserAgent(const char* user_agent) {
-  MutexLock lock(g_app_mutex);
+  MutexLock lock(*g_app_mutex);
   LibraryRegistry* registry = LibraryRegistry::Initialize();
   // Copy the string into a vector so that we can safely mutate the string.
   std::vector<char> user_agent_vector(user_agent,
@@ -427,12 +427,12 @@ void RegisterLibrariesFromUserAgent(const char* user_agent) {
 }
 
 const char* GetUserAgent() {
-  MutexLock lock(g_app_mutex);
+  MutexLock lock(*g_app_mutex);
   return LibraryRegistry::Initialize()->GetUserAgent();
 }
 
 std::string GetLibraryVersion(const char* library) {
-  MutexLock lock(g_app_mutex);
+  MutexLock lock(*g_app_mutex);
   return LibraryRegistry::Initialize()->GetLibraryVersion(library);
 }
 
@@ -442,7 +442,7 @@ void GetOuterMostSdkAndVersion(std::string* sdk, std::string* version) {
   sdk->clear();
   version->clear();
 
-  MutexLock lock(g_app_mutex);
+  MutexLock lock(*g_app_mutex);
   // Set of library versions to query in order of outer wrapper to inner.
   // We're only retrieving the outer-most SDK version here as we can only send
   // one component as part of the user agent string to the storage backend.
@@ -466,7 +466,7 @@ void GetOuterMostSdkAndVersion(std::string* sdk, std::string* version) {
 // Find a logger associated with an app by app name.
 Logger* FindAppLoggerByName(const char* name) {
   assert(name);
-  MutexLock lock(g_app_mutex);
+  MutexLock lock(*g_app_mutex);
   if (g_apps) {
     auto it = g_apps->find(std::string(name));
     if (it == g_apps->end()) return nullptr;

app/src/callback.cc

@@ -187,13 +187,13 @@ class CallbackDispatcher {
 
 static CallbackDispatcher* g_callback_dispatcher = nullptr;
 // Mutex that controls access to g_callback_dispatcher and g_callback_ref_count.
-static Mutex g_callback_mutex;  // NOLINT
+static Mutex* g_callback_mutex = new Mutex();
 static int g_callback_ref_count = 0;
 static Thread::Id g_callback_thread_id;
 static bool g_callback_thread_id_initialized = false;
 
 void Initialize() {
-  MutexLock lock(g_callback_mutex);
+  MutexLock lock(*g_callback_mutex);
   if (g_callback_ref_count == 0) {
     g_callback_dispatcher = new CallbackDispatcher();
   }
@@ -202,7 +202,7 @@ void Initialize() {
 
 // Add a reference to the module if it's already initialized.
 static bool InitializeIfInitialized() {
-  MutexLock lock(g_callback_mutex);
+  MutexLock lock(*g_callback_mutex);
   if (IsInitialized()) {
     Initialize();
     return true;
@@ -217,7 +217,7 @@ bool IsInitialized() { return g_callback_ref_count > 0; }
 static void Terminate(int number_of_references_to_remove) {
   CallbackDispatcher* dispatcher_to_destroy = nullptr;
   {
-    MutexLock lock(g_callback_mutex);
+    MutexLock lock(*g_callback_mutex);
     if (!g_callback_ref_count) {
       LogWarning("Callback module already shut down");
       return;
@@ -239,7 +239,7 @@ static void Terminate(int number_of_references_to_remove) {
 }
 
 void Terminate(bool flush_all) {
-  MutexLock lock(g_callback_mutex);
+  MutexLock lock(*g_callback_mutex);
   int ref_count = 1;
   // g_callback_ref_count is used to track the number of current references to
   // g_callback_dispatcher so we need to decrement the reference count by just
@@ -253,7 +253,7 @@ void Terminate(bool flush_all) {
 }
 
 void* AddCallback(Callback* callback) {
-  MutexLock lock(g_callback_mutex);
+  MutexLock lock(*g_callback_mutex);
   Initialize();
   return g_callback_dispatcher->AddCallback(callback);
 }

app/src/cleanup_notifier.cc

@@ -26,12 +26,12 @@
 
 namespace FIREBASE_NAMESPACE {
 
-Mutex CleanupNotifier::cleanup_notifiers_by_owner_mutex_;  // NOLINT
+Mutex* CleanupNotifier::cleanup_notifiers_by_owner_mutex_ = new Mutex();
 std::map<void *, CleanupNotifier *>
     *CleanupNotifier::cleanup_notifiers_by_owner_;
 
 CleanupNotifier::CleanupNotifier() : cleaned_up_(false) {
-  MutexLock lock(cleanup_notifiers_by_owner_mutex_);
+  MutexLock lock(*cleanup_notifiers_by_owner_mutex_);
   if (!cleanup_notifiers_by_owner_) {
     cleanup_notifiers_by_owner_ = new std::map<void *, CleanupNotifier *>();
   }
@@ -41,7 +41,7 @@ CleanupNotifier::~CleanupNotifier() {
   CleanupAll();
   UnregisterAllOwners();
   {
-    MutexLock lock(cleanup_notifiers_by_owner_mutex_);
+    MutexLock lock(*cleanup_notifiers_by_owner_mutex_);
     if (cleanup_notifiers_by_owner_ && cleanup_notifiers_by_owner_->empty()) {
       delete cleanup_notifiers_by_owner_;
       cleanup_notifiers_by_owner_ = nullptr;
@@ -78,14 +78,14 @@ void CleanupNotifier::CleanupAll() {
 }
 
 void CleanupNotifier::UnregisterAllOwners() {
-  MutexLock lock(cleanup_notifiers_by_owner_mutex_);
+  MutexLock lock(*cleanup_notifiers_by_owner_mutex_);
   while (owners_.begin() != owners_.end()) {
     UnregisterOwner(this, owners_[0]);
   }
 }
 
 void CleanupNotifier::RegisterOwner(CleanupNotifier *notifier, void *owner) {
-  MutexLock lock(cleanup_notifiers_by_owner_mutex_);
+  MutexLock lock(*cleanup_notifiers_by_owner_mutex_);
   assert(cleanup_notifiers_by_owner_);
   auto it = cleanup_notifiers_by_owner_->find(owner);
   if (it != cleanup_notifiers_by_owner_->end()) UnregisterOwner(it);
@@ -94,15 +94,15 @@ void CleanupNotifier::RegisterOwner(CleanupNotifier *notifier, void *owner) {
 }
 
 void CleanupNotifier::UnregisterOwner(CleanupNotifier *notifier, void *owner) {
-  MutexLock lock(cleanup_notifiers_by_owner_mutex_);
+  MutexLock lock(*cleanup_notifiers_by_owner_mutex_);
   assert(cleanup_notifiers_by_owner_);
   auto it = cleanup_notifiers_by_owner_->find(owner);
   if (it != cleanup_notifiers_by_owner_->end()) UnregisterOwner(it);
 }
 
 void CleanupNotifier::UnregisterOwner(
     std::map<void *, CleanupNotifier *>::iterator it) {
-  MutexLock lock(cleanup_notifiers_by_owner_mutex_);
+  MutexLock lock(*cleanup_notifiers_by_owner_mutex_);
   assert(cleanup_notifiers_by_owner_);
   void *owner = it->first;
   CleanupNotifier *notifier = it->second;
@@ -114,7 +114,7 @@ void CleanupNotifier::UnregisterOwner(
 }
 
 CleanupNotifier *CleanupNotifier::FindByOwner(void *owner) {
-  MutexLock lock(cleanup_notifiers_by_owner_mutex_);
+  MutexLock lock(*cleanup_notifiers_by_owner_mutex_);
   if (!cleanup_notifiers_by_owner_) return nullptr;
   auto it = cleanup_notifiers_by_owner_->find(owner);
   return it != cleanup_notifiers_by_owner_->end() ? it->second : nullptr;

app/src/cleanup_notifier.h

@@ -102,7 +102,7 @@ class CleanupNotifier {
   std::vector<void *> owners_;
 
   // Guards owners_ and cleanup_notifiers_by_owner_.
-  static Mutex cleanup_notifiers_by_owner_mutex_;
+  static Mutex* cleanup_notifiers_by_owner_mutex_;
   // Global map of cleanup notifiers bucketed by owner object.
   static std::map<void *, CleanupNotifier *> *cleanup_notifiers_by_owner_;
 };

app/src/secure/user_secure_manager.cc

@@ -50,7 +50,7 @@ namespace secure {
 
 using callback::NewCallback;
 
-Mutex UserSecureManager::s_scheduler_mutex_;  // NOLINT
+Mutex* UserSecureManager::s_scheduler_mutex_ = new Mutex();
 scheduler::Scheduler* UserSecureManager::s_scheduler_;
 int32_t UserSecureManager::s_scheduler_ref_count_;
 
@@ -184,7 +184,7 @@ Future<void> UserSecureManager::DeleteAllData() {
 }
 
 void UserSecureManager::CreateScheduler() {
-  MutexLock lock(s_scheduler_mutex_);
+  MutexLock lock(*s_scheduler_mutex_);
   if (s_scheduler_ == nullptr) {
     s_scheduler_ = new scheduler::Scheduler();
     // reset count
@@ -194,7 +194,7 @@ void UserSecureManager::CreateScheduler() {
 }
 
 void UserSecureManager::DestroyScheduler() {
-  MutexLock lock(s_scheduler_mutex_);
+  MutexLock lock(*s_scheduler_mutex_);
   if (s_scheduler_ == nullptr) {
     // reset count
     s_scheduler_ref_count_ = 0;

app/src/secure/user_secure_manager.h

@@ -74,7 +74,7 @@ class UserSecureManager {
   static void DestroyScheduler();
 
   // Guards static scheduler pointer
-  static Mutex s_scheduler_mutex_;  // NOLINT
+  static Mutex* s_scheduler_mutex_;
   static scheduler::Scheduler* s_scheduler_;
   static int32_t s_scheduler_ref_count_;