Firebase Callbacks Should Run on Main Thread

[dginovker]

Hello Firebase!

Our team has been using Firebase with Unity for over 2 years now to enhance our mobile games, and we've built great infrastructure around it. Unfortunately, that also means we've had our fair share of difficult-to-diagnose libc and segabrt crashes, whose symptoms usually lead to closed Github issues - A repeating point of frustration. Doing a retrospect on many of our crashes in 2021 and 2022 found that many of the issues we faced internally, as well as many issues we see on this repo, actually come from the fact the Unity API is not thread-safe and Firebase callbacks usually happen off the main thread.

Let's look at some examples:

Firebase.FirebaseApp.CheckDependenciesAsync().ContinueWith(checkTask => {
  // Peek at the status and see if we need to try to fix dependencies.
  Firebase.DependencyStatus status = checkTask.Result;
  if (status != Firebase.DependencyStatus.Available) {
    return Firebase.FirebaseApp.FixDependenciesAsync().ContinueWith(t => {
      return Firebase.FirebaseApp.CheckDependenciesAsync();
    }).Unwrap();
  } else {
    return checkTask;
  }
}).Unwrap().ContinueWith(task => {
  dependencyStatus = task.Result;
  if (dependencyStatus == Firebase.DependencyStatus.Available) {
    // TODO: Continue with Firebase initialization.
  } else {
    Debug.LogError(
      "Could not resolve all Firebase dependencies: " + dependencyStatus);
  }
});

This code is taken directly from the docs people use to get started. TheDebug.LogError call might lead a developer into thinking all is normal, but Debug is one of the only thread-safe Unity APIs out there. When they start to make non-thread safe callbacks and initializations, they invite all sorts of race conditions or even crashes that go undetected while in the Editor.

public void Start() {
  Firebase.Messaging.FirebaseMessaging.TokenReceived += OnTokenReceived;
  Firebase.Messaging.FirebaseMessaging.MessageReceived += OnMessageReceived;
}

public void OnTokenReceived(object sender, Firebase.Messaging.TokenReceivedEventArgs token) {
  UnityEngine.Debug.Log("Received Registration Token: " + token.Token);
}

public void OnMessageReceived(object sender, Firebase.Messaging.MessageReceivedEventArgs e) {
  UnityEngine.Debug.Log("Received a new message from: " + e.Message.From);
}

This is an example from the Firebase Messaging docs, and it's even more sinister, because there is no indication OnMessageReceived is being run off the main thread. As users of the SDK, oftentimes our code starts out simple, with the original developer blissfully unaware of the impending doom when a colleague modifies a random function that's part of the callback. With Messaging in particular (which doesn't work on Editor), the don't even know they've created a (SIGABRT) crash unless they happen to test this code path on device!

Multithreaded code is great for performance, but it should be an opt-in experience. Experienced developers know the risks and design decisions that come with incorporating multithreaded code, but juniors developers can struggle when thrown in with very little guidance.

There are 3 main problems that arise from off main thread callbacks:

• There's a fundamental best-practices juxtaposition where Firebase for Unity SDK gives the user non-main thread callbacks, but Unity isn't thread safe
  • i.e. Unhandled exceptions in threads do not get caught and displayed by the Unity Editor, an issue further amplified by how unhandled exceptions work differently on Android and iOS
• The written documentation and Unity for Firebase video guides very sparsely note the multithreaded callbacks
• Firebase Unity issues are constantly filed, but go stale due to the minimal viable reproduction cases not crashing when non-thread safe proprietary methods aren't called

There's a lot of great features that Firebase for Unity provides, and we continuously look forwards to expanding our usage of the SDK. We hope this feedback can help provide some insight into your users and their points of frustration, or perhaps even help a passer-by realize why their code has gone wrong 😄. While these issues are mostly behind our team for now, we hope they can be resolved upstream to prevent future users from going through the same pain.

[google-oss-bot]

This issue does not seem to follow the issue template. Make sure you provide all the required information.

[google-oss-bot]

I found a few problems with this issue:

• I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
• This issue does not seem to follow the issue template. Make sure you provide all the required information.

[a-maurice]

Hi @dginovker

Thanks for the detailed writeup. We do generally try to get the callbacks to happen on the Unity main thread, but sometimes it isn't guaranteed, as you have found. Part of that knowledge though comes from before when we supported older versions of Unity that used older dotnets, so it is possible that there are some improvements we could make, so we will look into that.

We do have an extension that we include, ContinueWithOnMainThread, which will make sure that the ContinueWith function is called back on a main thread, during Unity's Update loop. Generally we try to make sure the documentation uses that instead of regular ContinueWith, but clearly a couple have snuck in, as you found. We will do a pass on those, to see if there are any others as well.

We will leave this feature request open for now, to track any work on making the continuation happen on the main thread regularly.

[Thaina]

Alternatively Firebase could adopt IObservable pattern for Listener API and add extension method for ObserveOnMainThread and SubscribeOnMainThread like UniRX. This would also make firebase compatible with reactive extension paradigm

[StephenHodgson]

I've found it much more reliable to just use async/await as you normally would. Just be sure to wrap any calls in try/catch. Also use ConfigureAwait(true).