Skip to content

Commit f7e67fc

Browse files
cynthiajianga-maurice
authored andcommitted
[Auth] desktop load will update auth_data->persistent_cache_load_pending
+ trigger both auth state listener and token listener if load is done and no valid user loaded. If load a valid user, set that as the current user will automatically trigger these listeners. + rename the flag to more properly reflect the operation + Add linux internal failure log. PiperOrigin-RevId: 250543873
1 parent 08cced4 commit f7e67fc

File tree

6 files changed

+44
-18
lines changed

6 files changed

+44
-18
lines changed

auth/src/auth.cc

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,7 @@ static bool PushBackIfMissing(const T& entry, std::vector<T>* v) {
195195
// Return whether the listener is added.
196196
template <typename T>
197197
static bool AddListener(T listener, std::vector<T>* listener_vector, Auth* auth,
198-
std::vector<Auth*>* auth_vector, Mutex* mutex) {
199-
MutexLock lock(*mutex);
198+
std::vector<Auth*>* auth_vector) {
200199
// Add to array of listeners if not already there.
201200
const bool listener_added = PushBackIfMissing(listener, listener_vector);
202201
// Add auth to array of Auths if not already there.
@@ -212,28 +211,35 @@ static bool AddListener(T listener, std::vector<T>* listener_vector, Auth* auth,
212211

213212
void Auth::AddAuthStateListener(AuthStateListener* listener) {
214213
if (!auth_data_) return;
215-
bool added = AddListener(listener, &auth_data_->listeners, this,
216-
&listener->auths_, &auth_data_->listeners_mutex);
214+
// Would have to lock mutex till the method ends to protect on race
215+
// conditions.
216+
MutexLock lock(auth_data_->listeners_mutex);
217+
bool added =
218+
AddListener(listener, &auth_data_->listeners, this, &listener->auths_);
219+
217220
// If the listener is registered successfully and persistent cache has been
218221
// loaded, trigger OnAuthStateChanged() immediately. Otherwise, wait until
219222
// the cache is loaded, through AuthStateListener event
220-
if (added && auth_data_->persistent_cache_loaded) {
223+
if (added && !auth_data_->persistent_cache_load_pending) {
221224
listener->OnAuthStateChanged(this);
222225
}
223226
}
224227

225228
void Auth::AddIdTokenListener(IdTokenListener* listener) {
226229
if (!auth_data_) return;
230+
// Would have to lock mutex till the method ends to protect on race
231+
// conditions.
232+
MutexLock lock(auth_data_->listeners_mutex);
227233
bool added = AddListener(listener, &auth_data_->id_token_listeners, this,
228-
&listener->auths_, &auth_data_->listeners_mutex);
234+
&listener->auths_);
229235
// AddListener is valid even if the listener is already registered.
230236
// This makes sure that we only increase the reference count if a listener
231237
// was actually added.
232238
if (added) {
233239
// If the listener is registered successfully and persistent cache has been
234240
// loaded, trigger OnAuthStateChanged() immediately. Otherwise, wait until
235241
// the cache is loaded, through AuthStateListener event
236-
if (auth_data_->persistent_cache_loaded) {
242+
if (!auth_data_->persistent_cache_load_pending) {
237243
listener->OnIdTokenChanged(this);
238244
}
239245
EnableTokenAutoRefresh(auth_data_);
@@ -337,7 +343,7 @@ static inline bool VectorContains(const T& entry, const std::vector<T>& v) {
337343
\
338344
/* Auth should have loaded persistent cache if exists when the listener */ \
339345
/* event is triggered for the first time. */ \
340-
auth_data->persistent_cache_loaded = true; \
346+
auth_data->persistent_cache_load_pending = false; \
341347
}
342348

343349
AUTH_NOTIFY_LISTENERS(NotifyAuthStateListeners, "Auth state", listeners,

auth/src/data.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ struct AuthData {
7979
listener_impl(nullptr),
8080
id_token_listener_impl(nullptr),
8181
expect_id_token_listener_callback(false),
82-
persistent_cache_loaded(false) {}
82+
persistent_cache_load_pending(true) {}
8383

8484
~AuthData() {
8585
ClearUserInfos(this);
@@ -155,6 +155,7 @@ struct AuthData {
155155
/// Mutex protecting the `listeners`, `id_token_listeners`, and
156156
/// `phone_auth_provider.data_->listeners vectors`.
157157
/// 'listeners' are added and removed and accessed on different threads.
158+
/// This is also protecting persistent_cache_load_pending flag.
158159
Mutex listeners_mutex;
159160

160161
// Mutex for changes to the internal token listener state.
@@ -163,8 +164,8 @@ struct AuthData {
163164
// Tracks if the Id Token listener is expecting a callback to occur.
164165
bool expect_id_token_listener_callback;
165166

166-
// Tracks if the persistent cache has been loaded.
167-
bool persistent_cache_loaded;
167+
// Tracks if the persistent cache load is pending.
168+
bool persistent_cache_load_pending;
168169

169170
// Mutex protecting `expect_id_token_listener_callback`
170171
Mutex expect_id_token_mutex;

auth/src/desktop/auth_desktop.cc

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,10 +184,10 @@ void Auth::InitPlatformAuth(AuthData* const auth_data) {
184184
auth_data->app->function_registry()->RegisterFunction(
185185
internal::FnAuthStopTokenListener,
186186
Auth::StopTokenRefreshThreadForRegistry);
187-
#ifdef FIREBASE_EARLY_ACCESS_PREVIEW
187+
188188
// Load existing UserData
189189
InitializeUserDataPersist(auth_data);
190-
#endif // FIREBASE_EARLY_ACCESS_PREVIEW
190+
191191
InitializeTokenRefresher(auth_data);
192192
}
193193

@@ -211,9 +211,7 @@ void Auth::DestroyPlatformAuth(AuthData* const auth_data) {
211211
auth_data->id_token_listeners.clear();
212212
}
213213

214-
#ifdef FIREBASE_EARLY_ACCESS_PREVIEW
215214
DestroyUserDataPersist(auth_data);
216-
#endif // FIREBASE_EARLY_ACCESS_PREVIEW
217215

218216
UserView::ClearUser(auth_data);
219217

@@ -418,6 +416,15 @@ void DestroyUserDataPersist(AuthData* auth_data) {
418416
auth_data->auth->RemoveAuthStateListener(auth_impl->user_data_persist.get());
419417
}
420418

419+
void LoadFinishTriggerListeners(AuthData* auth_data) {
420+
// We would have to block other listener changes to protect race condition
421+
// on how many times a listener should be triggered. We would rely on first
422+
// listener trigger to flip the persistence loading bit.
423+
MutexLock lock(auth_data->listeners_mutex);
424+
NotifyAuthStateListeners(auth_data);
425+
NotifyIdTokenListeners(auth_data);
426+
}
427+
421428
void IdTokenRefreshThread::WakeThread() { wakeup_sem_.Post(); }
422429

423430
IdTokenRefreshThread::IdTokenRefreshThread()

auth/src/desktop/auth_desktop.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ const int kMsPerTokenRefresh =
127127

128128
void InitializeUserDataPersist(AuthData* auth_data);
129129
void DestroyUserDataPersist(AuthData* auth_data);
130+
void LoadFinishTriggerListeners(AuthData* auth_data);
130131

131132
} // namespace auth
132133
} // namespace firebase

auth/src/desktop/user_desktop.cc

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,10 @@ void UserDataPersist::OnAuthStateChanged(Auth* auth) { // NOLINT
412412
}
413413
}
414414

415-
void AssignLoadedData(const Future<std::string>& future, void* auth_data) {
415+
void AssignLoadedData(const Future<std::string>& future, AuthData* auth_data) {
416+
// This function will change the persistent_cache_load_pending, which is
417+
// a critical flag to decide listener trigger event, so lock listener_mutex
418+
// to protect it.
416419
if (future.error() == firebase::app::secure::kNoEntry) {
417420
LogDebug(future.error_message());
418421
return;
@@ -463,7 +466,13 @@ void AssignLoadedData(const Future<std::string>& future, void* auth_data) {
463466
loaded_user.last_sign_in_timestamp = userData->last_sign_in_timestamp();
464467
loaded_user.creation_timestamp = userData->creation_timestamp();
465468

466-
UserView::ResetUser(static_cast<AuthData*>(auth_data), loaded_user);
469+
UserView::ResetUser(auth_data, loaded_user);
470+
}
471+
472+
void HandleLoadedData(const Future<std::string>& future, void* auth_data) {
473+
auto cast_auth_data = static_cast<AuthData*>(auth_data);
474+
AssignLoadedData(future, cast_auth_data);
475+
LoadFinishTriggerListeners(cast_auth_data);
467476
}
468477

469478
Future<std::string> UserDataPersist::LoadUserData(AuthData* auth_data) {
@@ -473,7 +482,7 @@ Future<std::string> UserDataPersist::LoadUserData(AuthData* auth_data) {
473482

474483
Future<std::string> future =
475484
user_secure_manager_->LoadUserData(auth_data->app->name());
476-
future.OnCompletion(AssignLoadedData, auth_data);
485+
future.OnCompletion(HandleLoadedData, auth_data);
477486
return future;
478487
}
479488

auth/src/include/firebase/auth.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,8 @@ class Auth {
487487
friend class IdTokenRefreshListener;
488488
friend class IdTokenRefreshThread;
489489
friend class UserDataPersist;
490+
friend class UserDesktopTest;
491+
friend class AuthDesktopTest;
490492

491493
friend void EnableTokenAutoRefresh(AuthData* authData);
492494
friend void DisableTokenAutoRefresh(AuthData* authData);

0 commit comments

Comments
 (0)