Skip to content

Commit cec32ee

Browse files
chkuang-ga-maurice
authored andcommitted
Make current_user() a block call waiting for persistent cache to load on desktop.
* Each platform has its own implementation of current_user() * Avoid calling current_user() internally, except during listener event, since it can potentially be a block call. * Flip persistent_cache_load_pending in the beginning of AUTH_NOTIFY_LISTENERS() macro so that the user can call current_user() during OnAuthStateChanged() or OnIdTokenChanged(). Integration test: cl/247698227 PiperOrigin-RevId: 251526674
1 parent ef6b53c commit cec32ee

File tree

5 files changed

+86
-26
lines changed

5 files changed

+86
-26
lines changed

auth/src/android/auth_android.cc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,24 @@ Future<User*> Auth::CreateUserWithEmailAndPassword(const char* email,
524524
return MakeFuture(&futures, handle);
525525
}
526526

527+
// It's safe to return a direct pointer to `current_user` because that class
528+
// holds nothing but a pointer to AuthData, which never changes.
529+
// All User functions that require synchronization go through AuthData's mutex.
530+
User* Auth::current_user() {
531+
if (!auth_data_) return nullptr;
532+
MutexLock lock(auth_data_->future_impl.mutex());
533+
534+
// auth_data_->current_user should be available after Auth is created because
535+
// persistent is loaded during the constructor of Android FirebaseAuth.
536+
// This may change to make FirebaseAuth.getCurrentUser() to block and wait for
537+
// persistent loading. However, it is safe to access auth_data_->current_user
538+
// here since FirebaseAuth.getCurrentUser() (Android) is called in
539+
// InitPlatformAuth().
540+
User* user =
541+
auth_data_->user_impl == nullptr ? nullptr : &auth_data_->current_user;
542+
return user;
543+
}
544+
527545
void Auth::SignOut() {
528546
JNIEnv* env = Env(auth_data_);
529547
env->CallVoidMethod(AuthImpl(auth_data_), auth::GetMethodId(auth::kSignOut));

auth/src/auth.cc

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -163,17 +163,6 @@ void Auth::DeleteInternal() {
163163

164164
Auth::~Auth() { DeleteInternal(); }
165165

166-
// It's safe to return a direct pointer to `current_user` because that class
167-
// holds nothing but a pointer to AuthData, which never changes.
168-
// All User functions that require synchronization go through AuthData's mutex.
169-
User* Auth::current_user() {
170-
if (!auth_data_) return nullptr;
171-
MutexLock lock(auth_data_->future_impl.mutex());
172-
User* user =
173-
auth_data_->user_impl == nullptr ? nullptr : &auth_data_->current_user;
174-
return user;
175-
}
176-
177166
// Always non-nullptr since set in constructor.
178167
App& Auth::app() {
179168
FIREBASE_ASSERT(auth_data_ != nullptr);
@@ -219,7 +208,9 @@ void Auth::AddAuthStateListener(AuthStateListener* listener) {
219208

220209
// If the listener is registered successfully and persistent cache has been
221210
// loaded, trigger OnAuthStateChanged() immediately. Otherwise, wait until
222-
// the cache is loaded, through AuthStateListener event
211+
// the cache is loaded, through AuthStateListener event.
212+
// NOTE: This should be called synchronously or current_user() for desktop
213+
// implementation may not work.
223214
if (added && !auth_data_->persistent_cache_load_pending) {
224215
listener->OnAuthStateChanged(this);
225216
}
@@ -321,6 +312,10 @@ static inline bool VectorContains(const T& entry, const std::vector<T>& v) {
321312
void notify_method_name(AuthData* auth_data) { \
322313
MutexLock lock(auth_data->listeners_mutex); \
323314
\
315+
/* Auth should have loaded persistent cache if exists when the listener */ \
316+
/* event is triggered for the first time. */ \
317+
auth_data->persistent_cache_load_pending = false; \
318+
\
324319
/* Make a copy of the listener list in case it gets modified during */ \
325320
/* notification_method(). Note that modification is possible because */ \
326321
/* the same thread is allowed to reaquire the `listeners_mutex` */ \
@@ -340,10 +335,6 @@ static inline bool VectorContains(const T& entry, const std::vector<T>& v) {
340335
/* Notify the listener. */ \
341336
listener->notification_method(auth_data->auth); \
342337
} \
343-
\
344-
/* Auth should have loaded persistent cache if exists when the listener */ \
345-
/* event is triggered for the first time. */ \
346-
auth_data->persistent_cache_load_pending = false; \
347338
}
348339

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

auth/src/desktop/auth_desktop.cc

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,44 @@ void Auth::SignOut() {
371371
AuthenticationResult::SignOut(auth_data_);
372372
}
373373

374+
// AuthStateListener to wait for current_user() until persistent cache load is
375+
// finished.
376+
class CurrentUserBlockListener : public firebase::auth::AuthStateListener {
377+
public:
378+
explicit CurrentUserBlockListener() : semaphore_(0) {}
379+
~CurrentUserBlockListener() override {}
380+
381+
void OnAuthStateChanged(Auth* auth) override { semaphore_.Post(); }
382+
383+
void WaitForEvent() { semaphore_.Wait(); }
384+
385+
private:
386+
Semaphore semaphore_;
387+
};
388+
389+
// It's safe to return a direct pointer to `current_user` because that class
390+
// holds nothing but a pointer to AuthData, which never changes.
391+
// All User functions that require synchronization go through AuthData's mutex.
392+
User* Auth::current_user() {
393+
if (!auth_data_) return nullptr;
394+
395+
// Add a listener and wait for the first trigger.
396+
CurrentUserBlockListener listener;
397+
AddAuthStateListener(&listener);
398+
// If the persistent cache has not be loaded, this would wait until the
399+
// loading is finished and OnAuthStateChanged() is triggered.
400+
// If it has been loaded, OnAuthStateChanged() should be triggered recursively
401+
// and synchronously when AddAuthStateListener() is called.
402+
listener.WaitForEvent();
403+
RemoveAuthStateListener(&listener);
404+
405+
{
406+
MutexLock lock(auth_data_->future_impl.mutex());
407+
return auth_data_->user_impl == nullptr ? nullptr
408+
: &auth_data_->current_user;
409+
}
410+
}
411+
374412
void InitializeTokenRefresher(AuthData* auth_data) {
375413
auto auth_impl = static_cast<AuthImpl*>(auth_data->auth_impl);
376414
auth_impl->token_refresh_thread.Initialize(auth_data);
@@ -448,7 +486,7 @@ void IdTokenRefreshThread::Initialize(AuthData* auth_data) {
448486
// lock, to prevent deadlocks!
449487
refresh_thread->ref_count_mutex_.Acquire();
450488
auth->auth_data_->future_impl.mutex().Acquire();
451-
if (auth->current_user() && refresh_thread->ref_count_ > 0) {
489+
if (auth->auth_data_->user_impl && refresh_thread->ref_count_ > 0) {
452490
// The internal identifier kInternalFn_GetTokenForRefresher,
453491
// ensures that we won't mess with the LastResult for the
454492
// user-facing one.
@@ -459,8 +497,8 @@ void IdTokenRefreshThread::Initialize(AuthData* auth_data) {
459497

460498
if (ms_since_last_refresh >= kMsPerTokenRefresh) {
461499
Future<std::string> future =
462-
refresh_thread->auth->current_user()->GetTokenInternal(
463-
true, kInternalFn_GetTokenForRefresher);
500+
refresh_thread->auth->auth_data_->current_user
501+
.GetTokenInternal(true, kInternalFn_GetTokenForRefresher);
464502
auth->auth_data_->future_impl.mutex().Release();
465503
refresh_thread->ref_count_mutex_.Release();
466504

auth/src/include/firebase/auth.h

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,17 +137,16 @@ class Auth {
137137
~Auth();
138138

139139
/// Synchronously gets the cached current user, or nullptr if there is none.
140-
/// Current user might not be available immediately after Auth is created, if
141-
/// a user has signed in before. It is recommended to use AuthStateListener as
142-
/// the source of truth whether a user has logged in or not.
140+
/// @note This function may block and wait until the Auth instance finishes
141+
/// loading the saved user's state. This should only happen for a short
142+
/// period of time after the Auth instance is created.
143143
/// <SWIG>
144144
/// @xmlonly
145145
/// <csproperty name="CurrentUser">
146146
/// Synchronously gets the cached current user, or null if there is none.
147-
/// CurrentUser might not be available immediately after FirebaseAuth is
148-
/// created, if a user has signed in before. It is recommended to use
149-
/// StateChanged delegate as the source of truth whether a user has logged in
150-
/// or not.
147+
/// @note This function may block and wait until the Auth instance finishes
148+
/// loading the saved user's state. This should only happen for a short
149+
/// period of time after the Auth instance is created.
151150
/// </csproperty>
152151
/// @endxmlonly
153152
/// </SWIG>

auth/src/ios/auth_ios.mm

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,20 @@ void UpdateCurrentUser(AuthData* auth_data) {
227227
return MakeFuture(&futures, handle);
228228
}
229229

230+
// It's safe to return a direct pointer to `current_user` because that class
231+
// holds nothing but a pointer to AuthData, which never changes.
232+
// All User functions that require synchronization go through AuthData's mutex.
233+
User *Auth::current_user() {
234+
if (!auth_data_) return nullptr;
235+
MutexLock lock(auth_data_->future_impl.mutex());
236+
237+
// auth_data_->current_user should be available after Auth is created because
238+
// [AuthImpl(auth_data) currentUser] is called during Auth::InitPlatformAuth()
239+
// and it would block until persistence is loaded.
240+
User *user = auth_data_->user_impl == nullptr ? nullptr : &auth_data_->current_user;
241+
return user;
242+
}
243+
230244
static User* AssignUser(FIRUser *_Nullable user, AuthData *auth_data) {
231245
// Update our pointer to the iOS user that we're wrapping.
232246
MutexLock lock(auth_data->future_impl.mutex());

0 commit comments

Comments
 (0)