Skip to content

[firestore][android] fix instance cache key for Firestore initialization #8982

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
michaelpomogajko wants to merge 1 commit into
base: main
Choose a base branch
from
+1 −1
Open

[firestore][android] fix instance cache key for Firestore initialization #8982

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...droid/src/main/java/io/invertase/firebase/firestore/UniversalFirebaseFirestoreCommon.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ static FirebaseFirestore getFirestoreForApp(String appName, String databaseId) {

setFirestoreSettings(instance, firestoreKey);

instanceCache.put(appName, new WeakReference<FirebaseFirestore>(instance));
instanceCache.put(firestoreKey, new WeakReference<FirebaseFirestore>(instance));
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

While this change correctly fixes the key mismatch between lookup and storage, the underlying cache implementation remains problematic: (1) Key Reachability: WeakHashMap removes entries when the key object is no longer strongly reachable. Since firestoreKey is a local string created via concatenation, it will be garbage collected quickly, making the cache ineffective and leading to the repeated setFirestoreSettings calls this PR aims to avoid. (2) Thread Safety: WeakHashMap is not thread-safe. Concurrent access to this static map from multiple threads can cause race conditions or internal corruption. (3) WeakReference Handling: The retrieval logic at line 43 (visible in context) returns cachedInstance.get() without checking for null. If the WeakReference is cleared, the method returns null, which may cause a NullPointerException in the caller. Consider replacing instanceCache with a ConcurrentHashMap to ensure persistence, thread safety, and reliability.


return instance;
}
Expand Down
Loading