Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editor get stuck on Application.Reload when using Listen() on a collection or document reference #845

Closed
Sov3rain opened this issue Oct 28, 2020 · 16 comments

Comments

@Sov3rain
Copy link

[REQUIRED] Please fill in the following fields:

  • Unity editor version: 2020.1.8f1
  • Firebase Unity SDK version: 6.16.0
  • Source you installed the SDK: Unity Package Manager
  • Problematic Firebase Component: Firestore
  • Other Firebase Components in use: Auth, Functions
  • Additional SDKs you are using: none
  • Platform you are using the Unity editor on: Windows
  • Platform you are targeting: Android, iOS
  • Scripting Runtime: IL2CPP

[REQUIRED] Please describe the issue here:

When listening for changes on a collection or a document in any part of a script, Editor get stuck on Application.Reload state after recompiling or when entering play mode.

Steps to reproduce:

  • Add a document or collection listener with ref.Listen() in any part of a monobehaviour script.
  • Return to the Editor and wait for compilation
  • Try to enter play mode
  • Editor get stuck on Application.Reload state and is unusable.

image

Appens 100% of the time. If not listener is set, everything is ok.

Relevant Code:

Example class:

using Firebase.Firestore;
using UnityEngine;

public class TestListenFirebase : MonoBehaviour
{
    void Start()
    {
        var db = FirebaseFirestore.DefaultInstance;
        CollectionReference col = db.Collection("users");

        col.Listen(snapshot =>
        {
            Debug.Log("Listening...");
        });
    }
}
@Sov3rain Sov3rain added the new New issue. label Oct 28, 2020
@dconeybe dconeybe self-assigned this Oct 28, 2020
@dconeybe
Copy link

Hi @Sov3rain. Thank you for reporting this issue. I had coincidentally ran into a very similar issue recently and logged it internally as b/170201731. I hope to look into it shortly. I'll try to reproduce with your code sample.

In the meantime, could you reproduce this hang and see if you can capture the following information:

  1. The stack traces of all threads in the Unity Editor (perhaps you can attach a debugger and dump the thread states).
  2. The contents of the Unity Editor log, especially the last 100 lines or so.
@dconeybe
Copy link

@Sov3rain I noticed that you've reported this against

Firebase Unity SDK version: 6.16.0

Could you try upgrading to the latest version (6.16.1 at the time of writing) and see if the issue still occurs? I'm actually having difficulty reproducing.

@Sov3rain
Copy link
Author

Sov3rain commented Oct 29, 2020

@Sov3rain I noticed that you've reported this against

Firebase Unity SDK version: 6.16.0

Could you try upgrading to the latest version (6.16.1 at the time of writing) and see if the issue still occurs? I'm actually having difficulty reproducing.

After updating to 6.16.1 I can still reproduce 100% of the time. I'll try to dump my editor log and paste it here ASAP.

@Sov3rain
Copy link
Author

@dconeybe Attached is my Editor log file.

I must add to this that I read somewhere that using FirebaseApp.CheckAndFixDependenciesAsync() can cause this sort of behaviour. I didn't look at this though.

Editor.log

@DellaBitta DellaBitta added needs-attention Need Googler's attention and removed new New issue. labels Oct 29, 2020
@Sov3rain
Copy link
Author

Sov3rain commented Nov 4, 2020

Hey @dconeybe ,

I managed to get some time to test and try to reproduce on a empty project. I can reproduce 100% of time in that project.

Unity Version : 2020.1.8f1
Firebase SDK Version : 6.16.1

Here is the project itselft:
debugging-firestore-listeners.zip

I really need this functionnality to work properly, or a least a workaround or something because we need to release an app in the next 12 days, and before implementing the call manually, I need to know if you have somewhat of a idea of what's happening.

Thanks a lot

@dconeybe
Copy link

dconeybe commented Nov 4, 2020

Oh great! I'll try it out and let you know what I find.

@dconeybe
Copy link

dconeybe commented Nov 4, 2020

Although I was not able to reproduce the issue on my Mac, I was able to easily reproduce it on a colleague's Windows laptop. This is great news because it means that investigation can start. The root cause is still unknown but I will continue to investigate. So unfortunately, at this time I do not have an ETA for a fix nor a suggestion for a workaround.

@Sov3rain
Copy link
Author

Sov3rain commented Nov 5, 2020

Good news then! If you find something, please let me know. If you need me to perform some more tests, don't hesitate to ask.

If it can be helpful, here is my windows version : Version 1909 (build 18363.1139)

@dconeybe
Copy link

dconeybe commented Nov 6, 2020

Update: I have found the root cause of the issue; however, I don't have an ETA for the fix. I have a suggested workaround that you could try but it's far from ideal.

The issue is that the thread that makes the listener callbacks is a C++ thread and is part of a thread pool. When the callback into C# happens then C# becomes aware of the thread's existence and begins to track it. When your app is restarted in the Unity Editor it calls AppDomain.Unload() to tear things down. The first thing that it does is send a signal to all threads that it is tracking (both C# threads and C++ threads) and waits for them all to terminate; however, the threads in our thread pool do not terminate in response to signals and therefore the unload operation waits forever for the listener callback thread to exit.

The ugly fix that I created for testing purposes created a brand new thread for every listener callback so that the thread pool thread would never cross the C#/C++ boundary and therefore C# would never learn of its existence. However, it's clearly not a sustainable solution so I'll need to implement something more efficient. This exact same issue came up while I was investigating the fix for #783, which is basically the same issue except for transactions.

The workaround that I can think of (but have not yet tested) is to call FirebaseApp.Dispose() prior to restarting the app in the Unity Editor. This might work because it should shut down all threads in the thread pool. You could possibly add a button into your app somewhere that would call FirebaseApp.Dispose() and just make sure to remove the button in the production version of your app.

I'm sorry I don't have better news and that this issue is hampering your development. I'll continue to post progress updates for the fix to this issue.

@dconeybe dconeybe added type: bug and removed needs-attention Need Googler's attention labels Nov 6, 2020
@dconeybe
Copy link

dconeybe commented Nov 6, 2020

Note to Googlers: This issue is logged internally as b/172566004.

@dconeybe
Copy link

dconeybe commented Nov 6, 2020

@Sov3rain One of my colleagues suggested an even better workaround: Call FirebaseApp.Dispose() in the OnApplicationQuit() method of your MonoBehaviour.

For example, in the code sample from the first comment of this issue, the bug can be worked around by adding an OnApplicationQuit() method as follows:

using Firebase.Firestore;
using UnityEngine;

public class TestListenFirebase : MonoBehaviour
{

    private FirebaseFirestore _firestore;

    void Start()
    {
        _firestore = FirebaseFirestore.DefaultInstance;
        CollectionReference col = _firestore.Collection("users");

        col.Listen(snapshot =>
        {
            Debug.Log("Listening...");
        });
    }

    void OnApplicationQuit()
    {
        Debug.Log("OnApplicationQuit()");
        _firestore.App.Dispose();
    }
}
@Sov3rain
Copy link
Author

Sov3rain commented Nov 7, 2020

Hey @dconeybe, thanks for your help! I can confirm that the above workaround with Dispose() in OnApplicationQuit() callback is working.

I've got a warning right after stopping the editor though:

Auth object 0xa7649da0 should be deleted before the App 0x0dd24080 it depends upon.

I'm assuming that it's safe to ignore since the session is terminated, but I would like confirmation.

In addition, is it okay to keep the workaround logic in the build (Android and iOS alike) or is it safer to run it only in editor?

@dconeybe
Copy link

dconeybe commented Nov 7, 2020

@Sov3rain You can safely ignore that warning about the Auth object not being deleted before the App.

As for whether or not to keep the workaround in Android & iOS build, I'm afraid I'm not familiar enough with Unity development to make a confident recommendation. I'm relatively new to Unity and have been working only on the Firestore SDK which interacts very minimally with the Unity lifecycle. Given that disclaimer, if I were to recommend anything it would be to remove the workaround from your production build since I'm not sure under which conditions OnApplicationQuit() is invoked in those scenarios.

@Sov3rain
Copy link
Author

Sov3rain commented Nov 7, 2020

@dconeybe Fine, we will make some tests for sure, but for now I'm thinking of surrounding the OnApplicationQuit() callback with a #if directive like this:

#if UNITY_EDITOR
    void OnApplicationQuit()
    {
        _firestore.App.Dispose();
    }
#endif

It should do the trick without breaking anything in the build.

@dconeybe
Copy link

Good news! I have submitted a fix for this bug today. It should be included in the next release, for which, unfortunately, I do not have an ETA. Until then, please employ one of the workarounds described in earlier comments in this issue. I'm going to close this issue as "fixed" since there is no further work to do from our end; however, feel free to re-open if the next release still contains this bug.

@dconeybe
Copy link

dconeybe commented Dec 3, 2020

FYI The Firebase Unity SDK v7.0.0 was just released and contains the fix for this issue.

@firebase firebase locked and limited conversation to collaborators Dec 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.