Skip to content

Commit 217f40c

Browse files
authored
Remove the Auth currentUser cache (#739)
* Remove the Auth currentUser cache * Update readme.md
1 parent 2437d11 commit 217f40c

File tree

2 files changed

+15
-36
lines changed

2 files changed

+15
-36
lines changed

auth/src/swig/auth.i

Lines changed: 13 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -616,8 +616,6 @@ static CppInstanceManager<Auth> g_auth_instances;
616616
private System.IntPtr authStateListener;
617617
// Pointer to IdTokenListenerImpl.
618618
private System.IntPtr idTokenListener;
619-
// Proxy for the current user object.
620-
private FirebaseUser currentUser;
621619

622620
// Retrieve a reference to the auth object associated with the specified app.
623621
private static FirebaseAuth ProxyFromAppCPtr(System.IntPtr appCPtr) {
@@ -690,16 +688,6 @@ static CppInstanceManager<Auth> g_auth_instances;
690688
appProxy = null;
691689
appCPtr = System.IntPtr.Zero;
692690

693-
// Detatch the user proxy from the C++ object as it will no longer
694-
// be valid. FirebaseUser never owns the C++ object and so will never
695-
// delete it.
696-
// NOTE: This uses the cached currentUser as the auth object may be
697-
// destroyed at this point.
698-
if (currentUser != null) {
699-
currentUser.Dispose();
700-
currentUser = null;
701-
}
702-
703691
// Destroy token and auth state listeners.
704692
if (authStateListener != System.IntPtr.Zero) {
705693
AuthUtil.DestroyAuthStateListener(this, authStateListener);
@@ -734,9 +722,6 @@ static CppInstanceManager<Auth> g_auth_instances;
734722
ForwardStateChange(appCPtr, (auth) => {
735723
// If any state changed events are registered, signal them.
736724
if (auth.stateChangedImpl != null) {
737-
lock (appCPtrToAuth) {
738-
auth.UpdateCurrentUser(auth.CurrentUserInternal);
739-
}
740725
auth.stateChangedImpl(auth, System.EventArgs.Empty);
741726
}
742727

@@ -877,25 +862,17 @@ static CppInstanceManager<Auth> g_auth_instances;
877862
return taskCompletionSource.Task;
878863
}
879864

880-
// Update the cached user proxy for this object.
881-
private FirebaseUser UpdateCurrentUser(FirebaseUser proxy) {
882-
lock (appCPtrToAuth) {
883-
if (proxy == null || !proxy.IsValid()) {
884-
// If there is no current user, remove the cached proxy.
885-
currentUser = null;
886-
} else if (currentUser == null || !currentUser.IsValid()) {
887-
// If no proxy is cached, cache the current proxy.
888-
currentUser = proxy;
889-
} else {
890-
// If the user changed, update the cached proxy.
891-
if (!currentUser.EqualToInternal(proxy)) {
892-
currentUser.Dispose();
893-
currentUser = proxy;
894-
}
865+
// Does additional work to set up the FirebaseUser.
866+
private FirebaseUser SetupUser(FirebaseUser user) {
867+
if (user != null) {
868+
// If the user isn't valid, switch to use null instead
869+
if (!user.IsValid()) {
870+
return null;
895871
}
896-
if (currentUser != null) currentUser.authProxy = this;
872+
// Set the Auth object in the user
873+
user.authProxy = this;
897874
}
898-
return currentUser;
875+
return user;
899876
}
900877

901878
/// @brief Synchronously gets the cached current user, or null if there is none.
@@ -905,7 +882,7 @@ static CppInstanceManager<Auth> g_auth_instances;
905882
public FirebaseUser CurrentUser {
906883
get {
907884
var user = swigCPtr.Handle != System.IntPtr.Zero ? CurrentUserInternal : null;
908-
return UpdateCurrentUser(user);
885+
return SetupUser(user);
909886
}
910887
}
911888

@@ -1160,7 +1137,7 @@ static CppInstanceManager<Auth> g_auth_instances;
11601137
Firebase.Internal.TaskCompletionSourceCompat<FirebaseUser>.SetException(
11611138
taskCompletionSource, task.Exception);
11621139
} else {
1163-
taskCompletionSource.SetResult(UpdateCurrentUser(task.Result));
1140+
taskCompletionSource.SetResult(SetupUser(task.Result));
11641141
}
11651142
}
11661143

@@ -1195,7 +1172,7 @@ static CppInstanceManager<Auth> g_auth_instances;
11951172
AuthResult result = task.Result;
11961173
// This assume all the users from AuthResult points to current users.
11971174
// TODO(AuthRewrite): Update this logic when we can have multile FirebaseUser.
1198-
result.UserInternal = UpdateCurrentUser(result.User);
1175+
result.UserInternal = SetupUser(result.User);
11991176
taskCompletionSource.SetResult(result);
12001177
}
12011178
}
@@ -1314,7 +1291,7 @@ static CppInstanceManager<Auth> g_auth_instances;
13141291

13151292
/// The currently signed-in FirebaseUser, or null if there isn't any (i.e.
13161293
/// the user is signed out).
1317-
public FirebaseUser User { get { return authProxy.CurrentUser; } }
1294+
public FirebaseUser User { get { return authProxy != null ? authProxy.CurrentUser : null; } }
13181295
%}
13191296

13201297
// AuthResult

docs/readme.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ Release Notes
7373
-------------
7474
# Upcoming release
7575
- Changes
76+
- Auth: Fix a [crash](https://github.com/firebase/firebase-unity-sdk/issues/733)
77+
that could occur when referencing CurrentUser.
7678
- Auth: Remove internal methods.
7779
- General: Fix an [issue](https://github.com/firebase/firebase-unity-sdk/issues/726)
7880
where AppCheck bundles were unintentionally included in App in the tgz.

0 commit comments

Comments
 (0)