Android: SignInSilently freezes Unity

[Deathwing]

I have a simple Unity UI Button, which has a handler like this:

        public async void OnGoogleLoginClicked()
        {
            var user = await GoogleSignIn.DefaultInstance.SignInSilently(); // we use the silent sign in somewhere else, but that is the easiest to reproduce the issue
        }

I also enabled logging for the GoogleSignIn plugin and I am getting:

2023/11/28 15:05:37.918 13513 13625 Debug SignInFragment AuthHelperFragment.signinSilently called!
2023/11/28 15:05:37.918 13513 13625 Debug SignInFragment Using DEFAULT_SIGN_IN
2023/11/28 15:05:37.919 13513 13625 Debug SignInFragment Requesting IDToken  client: ***-***.apps.googleusercontent.com
2023/11/28 15:05:37.927 13513 13625 Debug SignInFragment  Has connected == false
2023/11/28 15:05:37.932 13513 13625 Debug SignInFragment Done with processRequest!
2023/11/28 15:05:37.963 13513 13513 Error SignInFragment Error with silentSignIn: Status{statusCode=4: , resolution=null}
2023/11/28 15:05:37.984 13513 13513 Info Unity googlesignin.IListener : 
2023/11/28 15:05:37.984 13513 13513 Info Unity Google.Impl.SignInListener:OnResult(Int32, AndroidJavaObject) (at ./Library/PackageCache/com.google.signin@dbf79fa766/GoogleSignIn/Impl/GoogleSignInImpl.cs:139)
2023/11/28 15:05:37.990 13513 13513 Info Unity ID : 
2023/11/28 15:05:37.990 13513 13513 Info Unity Google.Impl.SignInListener:OnResult(Int32, AndroidJavaObject) (at ./Library/PackageCache/com.google.signin@dbf79fa766/GoogleSignIn/Impl/GoogleSignInImpl.cs:140)

(the error is expected, also the id and listener logs are from my modification with the allowance of acct == null)

But that's it... afterwards, the whole application is unresponsive :)

The issue doesn't exist in the editor, nor on iOS builds, only Android seems affected.

[Deathwing]

I think I found the reason, but I am not yet sure how to fix it correctly: The native result correctly has the status and acct set (status = 4 (required signing), acct = null). which is also forwarded correctly to the SignInListener within the plugin. But when the future itself checks it here:

internal IEnumerator WaitForResult(TaskCompletionSource<T> tcs) {
      yield return new WaitUntil(() => !Pending);
      if (Status == GoogleSignInStatusCode.CANCELED) {
        tcs.SetCanceled();
      } else if (Status == GoogleSignInStatusCode.SUCCESS ||
            Status == GoogleSignInStatusCode.SUCCESS_CACHE) {
        tcs.SetResult(Result);
      } else {
        tcs.SetException(new GoogleSignIn.SignInException(Status));
      }
    }

Status is GoogleSignInStatusCode.SUCCESS, and trying to access Result throws a NullReferenceException exception in this line:

internal static IntPtr GoogleSignIn_Result(HandleRef self) => self.ToAndroidJavaObject().Call<AndroidJavaObject>("getAccount").GetRawObject();

[Thaina]

Thanks for the report. I will try to fix this soon

Not sure if we could just change this code to

internal static IntPtr GoogleSignIn_Result(HandleRef self) => self.ToAndroidJavaObject()?.Call<AndroidJavaObject>("getAccount")?.GetRawObject() ?? IntPtr.Zero;

Would just fix the problem

[Deathwing]

Made a couple of adjustments to it, and it seems to work fine for us, you can check it here: https://github.com/calumma-robert/google-signin-unity.git

[Thaina]

Thank you, I have been busy working on other project unrelated to this as of late. Would you like to make a pull request?

[calumma-robert]

I also noticed, cancelling a request, and trying to make a new one is not working, as it will instantly return the old result, (on the next request it will return the new one, etc.) the problem is because tokenRequest cant be cleared (as otherwise it would be unusable again, as only configure which can be called once, creates a tokenRequest.

Edit: That is why I tried to wait an additional frame before or after the pending check, but that makes the HandleRef to go stale and therefore crashes the application :)

[calumma-robert]

I removed the cached tokenResult, as it maskes underlying errors. I also tried to change isPending to also take Busy state into account (that is the reason why it is not correct) but again...results in a crash because of the HandleRef becoming stale...