Fix use-after-free when Firebase instances are destroyed during static destruction by AustinBenoit · Pull Request #1949 · firebase/firebase-cpp-sdk

AustinBenoit · 2026-10-26T15:38:14Z

Description

Provide details of the change, and generalize the change in the PR title above.

Fixes #110.

Destroying a static firebase::App or product wrapper (e.g. static std::unique_ptr<Auth>) during process exit can run after the C++ runtime has already destroyed the SDK's own static maps, mutexes, or loggers, causing use-after-free or destroyed-mutex UB.

Adds firebase::NoDestructor<T> (app/src/no_destructor.h) for inline, trivially destructible function-local statics that initialize thread-safely on first use and never run destructors at exit.
Migrates process-wide registries, mutexes, loggers, callback/cleanup state, and static constants across firebase_app, app/rest, and all product libraries (Analytics, AppCheck, Auth, Database, Firestore, Functions, Installations, Messaging, RemoteConfig, Storage, UMP) onto NoDestructor<T>.
Fixes Installations out-of-order App teardown map retention, failed-init leaks in Installations and RemoteConfig, and O(log N) map removal in Auth::DeleteInternal().
Enables -Werror=exit-time-destructors on Clang/AppleClang across all 12 static library targets.

Testing

Describe how you've tested these changes. Link any manually triggered Integration tests or CPP binary SDK Packaging Github Action workflows, if applicable.

Verified 0 __cxa_atexit references across all 12 built desktop static archives (208 object files) and clean clang++ -Werror=exit-time-destructors compilation.
Added firebase_app_static_destruction_test (post-main() static teardown and repeated App reinitialization), expanded firebase_app_no_destructor_test, and added InstallationsTest.DestroyAppBeforeInstallationsClearsCachedInstance.
Verified static App / Auth / RemoteConfig teardown under AddressSanitizer (exit 0, no UAF).

Type of Change

Place an x the applicable box:

Bug fix. Add the issue # below if applicable.
New feature. A non-breaking change which adds functionality.
Other, such as a build process or documentation change.

Fixes #110

Notes

Bug fixes and feature changes require an update to the Release Notes section of release_build_files/readme.md.
Read the contribution guidelines CONTRIBUTING.md.
Changes to the public API require an internal API review. If you'd like to help us make Firebase APIs better, please propose your change in a feature request so that we can discuss it together.

Each product keeps a process-wide map of App -> instance plus a mutex, and both are touched from the instance's destructor via DeleteInternal(). When a user holds an instance in a static (e.g. a static std::unique_ptr<Auth>) that is destroyed after the SDK's own globals, the destructor accesses an already-destroyed map or mutex. Auth and Remote Config stored the map as a static std::map, which produced a heap-use-after-free in ~Auth()/~RemoteConfig(). Database, Storage, Functions, Installations and App Check already heap-allocated the map but still used a static Mutex, which is destroyed at exit before the lock in DeleteInternal(). Bring every product in line with the layout App and Firestore already use: the map is lazily allocated and freed when the last instance goes away, and the mutex is allocated once and never freed. Fixes #110

gemini-code-assist

Code Review

This pull request addresses a potential crash at process exit by ensuring that global mutexes and maps for various Firebase SDK components (Auth, RemoteConfig, Database, Storage, Functions, Installations, and AppCheck) are handled safely. The changes involve dynamically allocating these objects to prevent destruction order issues during C++ runtime shutdown. The reviewer correctly identified that the current implementation of global mutex initialization is susceptible to the Static Initialization Order Fiasco (SIOF) and suggested using a function-local static pointer ('construct on first use' idiom) to mitigate this risk.

Replace the namespace-scope `Mutex* g_x = new Mutex();` globals with accessor functions that own a function-local static. Dynamic initialization of a namespace-scope pointer is subject to static initialization order, so a static initializer in another translation unit that called GetInstance() would have found a null mutex. A function-local static is initialized on first use, thread-safely, regardless of translation unit order, and the pointer it holds is still never freed. Auth and Remote Config now also keep their map behind the same kind of accessor instead of lazily allocating and freeing a raw pointer, which removes the null checks and manual lifetime management from the GetInstance/Find/DeleteInternal paths. Callers only ever see references.

Introduce firebase::NoDestructor<T>, a small wrapper (modelled on absl::NoDestructor) that constructs a T in place and never runs its destructor. Use it for the registry mutexes of every product and the registry maps of Auth and Remote Config in place of `static T* const t = new T;`. The objects are now stored inline in the function-local static rather than on the heap, there is no `new` or raw pointer anywhere in the registry code, and leak checkers have nothing to report since nothing is allocated. The semantics are otherwise unchanged: constructed on first use, thread-safely, regardless of translation unit order, and never destroyed. Includes a unit test for NoDestructor.

AustinBenoit · 2026-10-29T14:43:05Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a NoDestructor utility class to wrap function-local statics, such as global maps and mutexes, ensuring they are constructed on first use and never destroyed. This change prevents crashes at process exit when Firebase SDK instances (such as Auth, RemoteConfig, Database, Storage, Functions, Installations, or AppCheck) are owned by static objects and destroyed after the SDK's globals. The review feedback highlights several critical issues regarding redundant recursive locking or potential deadlocks in Auth, RemoteConfig, and Installations where locked functions call other functions that attempt to acquire the same mutex. Additionally, minor improvements are suggested for NoDestructor to use decayed array pointers instead of pointer-to-array types for cleaner and more portable casts.

- In NoDestructor, cast the decayed array pointer `storage_` rather than `&storage_` in placement new and `get()`. - In Auth, RemoteConfig, and Installations, factor out `Find*Locked(App*)` helpers (and inline the internal cleanup in `~Installations()`) so callers that already hold the registry mutex do not recursively re-lock it.

AustinBenoit · 2026-10-29T21:18:25Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces the NoDestructor template class to wrap function-local statics, preventing process-exit crashes when Firebase SDK instances (such as Auth, RemoteConfig, Database, Storage, Functions, Installations, and AppCheck) are owned by static objects and destroyed after the SDK's globals. The feedback suggests optimizing the Auth map deletion by performing a direct key lookup instead of iterating through the map, and refactoring the Installations cleanup logic into a helper function to eliminate code duplication.

…odules - Harden NoDestructor<T> with SFINAE forwarding-ctor guard, brace-init overloads, and std::launder access - Migrate firebase_app, app/rest, and all product libraries (Analytics, AppCheck, Auth, Database, Firestore, Functions, Installations, Messaging, RemoteConfig, Storage, UMP) across Desktop, Android, and iOS onto function-local NoDestructor accessors - Fix Installations out-of-order App deletion map retention, ABA safety, failed-init cleanup in Installations/RemoteConfig, and O(log N) Auth map removal - Enable -Werror=exit-time-destructors on Clang/AppleClang across all 12 static library targets - Add firebase_app_static_destruction_test, expanded NoDestructor unit tests, and Installations out-of-order teardown regression test

github-actions · 2026-10-31T02:18:33Z

  if (!cleaned_up_) {
    while (callbacks_.begin() != callbacks_.end()) {
-      std::pair<void *, CleanupCallback> object_and_callback =
+      std::pair<void*, CleanupCallback> object_and_callback =


⚠️ Lint warning: Add #include <utility> for pair<>

github-actions · 2026-10-31T02:18:33Z

@@ -47,9 +48,10 @@ static_assert(sizeof(FutureBase) == sizeof(Future<int>),
 typedef void DataDeleteFn(void* data_to_delete);
 typedef std::pair<FutureHandleId, FutureBackingData*> BackingPair;


⚠️ Lint warning: Add #include <utility> for pair<>

github-actions · 2026-10-31T02:18:33Z

+
+const std::pair<Variant, Variant>& QueryParamsComparator::MaxNode() {
+  static const NoDestructor<std::pair<Variant, Variant>> max_node(
+      std::make_pair(QueryParamsComparator::kMaxKey, MaxVariant()));


⚠️ Lint warning: Add #include <utility> for make_pair

github-actions · 2026-10-31T02:18:34Z

 namespace internal {
-extern ::firebase::Mutex g_cached_ump_embedded_files_mutex;
+::firebase::Mutex& CachedUmpEmbeddedFilesMutex();
 extern std::vector<::firebase::internal::EmbeddedFile>*


⚠️ Lint warning: Add #include <vector> for vector<>

gemini-code-assist Bot reviewed Oct 26, 2026

View reviewed changes

AustinBenoit marked this pull request as draft October 26, 2026 15:42

AustinBenoit added 2 commits October 26, 2026 15:46

Merge branch 'main' into fix/static-destruction-order

e17d554

gemini-code-assist Bot reviewed Oct 29, 2026

View reviewed changes

Comment thread auth/src/auth.cc Resolved

Comment thread installations/src/installations.cc Resolved

Comment thread installations/src/installations.cc Resolved

AustinBenoit added the tests-requested: full Trigger a FULL set of integration tests (uses expanded test matrix). label Oct 30, 2026

Merge branch 'main' into fix/static-destruction-order

7d6b83b

github-actions Bot reviewed Oct 31, 2026

View reviewed changes

		@@ -47,9 +48,10 @@ static_assert(sizeof(FutureBase) == sizeof(Future<int>),
		typedef void DataDeleteFn(void* data_to_delete);
		typedef std::pair<FutureHandleId, FutureBackingData*> BackingPair;

Conversation

AustinBenoit commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Type of Change

Notes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AustinBenoit commented Oct 29, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AustinBenoit commented Oct 29, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot Oct 31, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 31, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 31, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

AustinBenoit commented Oct 26, 2026 •

edited

Loading