GetTokenAsync race condition

[mayhammf]

Bug Report

In some cases the task returned by CrossFirebasePushNotification.Current.GetTokenAsync() never gets resolved. When that happens no exception is thrown nor CrossFirebasePushNotification.Current.OnNotificationError is invoked. This is caused by the way the GetTokenAsync method is implemented

Expected behavior

await GetTokenAsync() must always return token or null and should invoke OnNotificationError in that case.

Reproduction steps

string token = await CrossFirebasePushNotification.Current.GetTokenAsync();
System.Diagnostics.Debug.WriteLine(CrossFirebasePushNotification.Current.Token);

around the same time of calling FirebasePushNotificationManager.Initialize(). You'll notice the WriteLine method is never called.

Configuration

Version: 3.16

Platform:

• Android

Proposed solution

This issue can be resolved by encapsulating the TaskCompletionSource (which is used later by OnComplete) rather than using an unsafe reference which will be replaced every time GetTokenAsync is called (even if the previous task has not been resolved yet.)

Example below:

public async Task<string> GetTokenAsync()
{
    using var tokenTaskHandler = new GmsTaskCompletionHanlder();
    Firebase.Iid.FirebaseInstanceId.Instance.GetInstanceId().AddOnCompleteListener(tokenTaskHandler);

    string retVal = null;

    try
    {
        retVal = await tokenTaskHandler.Task;
    }
    catch (Exception ex)
    {
        _onNotificationError?.Invoke(CrossFirebasePushNotification.Current, new FirebasePushNotificationErrorEventArgs(FirebasePushNotificationErrorType.RegistrationFailed, $"{ex}"));
    }

    return retVal;
}

public class GmsTaskCompletionHanlder : Java.Lang.Object, Android.Gms.Tasks.IOnCompleteListener
{
    private readonly TaskCompletionSource<string> taskCompletionSource = new TaskCompletionSource<string>();
    public Task<string> Task => taskCompletionSource.Task;
    public void OnComplete(Android.Gms.Tasks.Task task)
    {
        try
        {
            if (task.IsSuccessful)
            {
                var token = task.Result.JavaCast<Firebase.Iid.IInstanceIdResult>().Token;
                this.taskCompletionSource?.TrySetResult(token);
            }
            else
            {
                throw task.Exception;
            }
        }
        catch (Exception ex)
        {
            this.taskCompletionSource?.TrySetException(ex);
        }
    }
}

[thomasgalliker]

GetTokenAsync has a static taskcompletionsource as field behind. This is a very dangerous design. When multiple calls to GetTokenAsync happen, some taskcompletionsources may wait forever.

This bug is open for 3 years now. Is anyone taking care of this plugin anymore?