Skip to content

Commit 1d40a13

Browse files
wilhuffpaulb777
authored andcommitted
Fix potential race during client initialization (#4091) (#4102)
1 parent 6af1b91 commit 1d40a13

File tree

3 files changed

+17
-18
lines changed

3 files changed

+17
-18
lines changed

Firestore/CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
# Unreleased
22

33
# v1.6.1
4-
- [changed] Internal improvements.
4+
- [fixed] Fix a race condition that could cause a segmentation fault during
5+
client initialization.
56

67
# v1.6.0
78
- [feature] Added an `addSnapshotsInSyncListener()` method to

Firestore/core/src/firebase/firestore/core/firestore_client.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ class FirestoreClient : public std::enable_shared_from_this<FirestoreClient> {
207207
std::chrono::milliseconds initial_gc_delay_ = std::chrono::minutes(1);
208208
std::chrono::milliseconds regular_gc_delay_ = std::chrono::minutes(5);
209209
bool gc_has_run_ = false;
210+
bool credentials_initialized_ = false;
210211
local::LruDelegate* _Nullable lru_delegate_;
211212
util::DelayedOperation lru_callback_;
212213
};

Firestore/core/src/firebase/firestore/core/firestore_client.mm

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -94,18 +94,21 @@
9494
new FirestoreClient(database_info, std::move(credentials_provider),
9595
std::move(user_executor), std::move(worker_queue)));
9696

97-
auto user_promise = std::make_shared<std::promise<User>>();
98-
bool credentials_initialized = false;
99-
10097
std::weak_ptr<FirestoreClient> weak_client(shared_client);
101-
auto credential_change_listener = [credentials_initialized, user_promise,
102-
weak_client](User user) mutable {
98+
auto credential_change_listener = [weak_client, settings](User user) mutable {
10399
auto shared_client = weak_client.lock();
104100
if (!shared_client) return;
105101

106-
if (!credentials_initialized) {
107-
credentials_initialized = true;
108-
user_promise->set_value(user);
102+
if (!shared_client->credentials_initialized_) {
103+
shared_client->credentials_initialized_ = true;
104+
105+
// When we register the credentials listener for the first time,
106+
// it is invoked synchronously on the calling thread. This ensures that
107+
// the first item enqueued on the worker queue is
108+
// `FirestoreClient::Initialize()`.
109+
shared_client->worker_queue()->Enqueue([shared_client, user, settings] {
110+
shared_client->Initialize(user, settings);
111+
});
109112
} else {
110113
shared_client->worker_queue()->Enqueue([shared_client, user] {
111114
shared_client->worker_queue()->VerifyIsCurrentQueue();
@@ -119,15 +122,9 @@ new FirestoreClient(database_info, std::move(credentials_provider),
119122
shared_client->credentials_provider_->SetCredentialChangeListener(
120123
credential_change_listener);
121124

122-
// Defer initialization until we get the current user from the
123-
// credential_change_listener. This is guaranteed to be synchronously
124-
// dispatched onto our worker queue, so we will be initialized before any
125-
// subsequently queued work runs.
126-
shared_client->worker_queue()->Enqueue(
127-
[shared_client, user_promise, settings] {
128-
User user = user_promise->get_future().get();
129-
shared_client->Initialize(user, settings);
130-
});
125+
HARD_ASSERT(
126+
shared_client->credentials_initialized_,
127+
"CredentialChangeListener not invoked during client initialization");
131128

132129
return shared_client;
133130
}

0 commit comments

Comments
 (0)