firebase / firebase-cpp-sdk

Firebase C++ SDK
http://firebase.google.com
Apache License 2.0
271 stars 110 forks source link

[Bug] Nested `Future::OnCompletion` appears to deadlock #1621

Open paulpv opened 3 days ago

paulpv commented 3 days ago

[REQUIRED] Please fill in the following fields:

[REQUIRED] Please describe the issue here:

As the title says, nested Future::OnCompletion appears to deadlock.

I have created a sample project reproducing this problem at: https://github.com/DistroAV/firebase-functions-future-async-nested

I have a very simple firebase function running in an emulator (the same problem happens when published to project too):

exports.foo = onCall((data, context) => {
        return 200;
    });

I then have C++ code to call this:

firebase::Future<firebase::functions::HttpsCallableResult> CallAsync(const char *name, const firebase::Variant &data,
        std::function<void(const firebase::Variant &data, int error,
                   const std::string &)>
            completion_callback)
{
    cout << "tid=" << pthread_self() << " +Cloud::Call(`" << name << "`, ...)" << endl;
    firebase::Future<firebase::functions::HttpsCallableResult> future;
    if (functions_) {
        auto httpcallref = functions_->GetHttpsCallable(name);
        cout << "tid=" << pthread_self() << " Call: `" << name << "` +Call(...)" << endl;
        future = httpcallref.Call(data);
        cout << "tid=" << pthread_self() << " Call: `" << name << "` -Call(...)" << endl;
        future.OnCompletion(
            [completion_callback](
                const firebase::Future<
                    firebase::functions::
                        HttpsCallableResult>
                    &future) {
                OnCallCompleted(future,
                        completion_callback);
            });
    }
    cout << "tid=" << pthread_self() << " -Cloud::Call(`" << name << "`, ...)" << endl;
    return future;
}

firebase::Variant OnCallCompleted(
    const firebase::Future<firebase::functions::HttpsCallableResult> &future,
    std::function<void(const firebase::Variant &data, int error,
               const std::string &)>
        completion_callback)
{
    cout << "tid=" << pthread_self() << " +OnCallCompleted(...)" << endl;
    firebase::Variant data;
    auto status = future.status();
    cout << "tid=" << pthread_self() << " OnCallCompleted: status=" << status << endl;
    if (status == firebase::kFutureStatusComplete) {
        auto httpcallresult = future.result();
        auto error = future.error();
        auto error_message = future.error_message();
        if (error == 0) {
            data = httpcallresult->data();
        }
        if (completion_callback) {
            completion_callback(data, error, error_message);
        }
    }
    cout << "tid=" << pthread_self() << " -OnCallCompleted(...)" << endl;
    return data;
}

firebase::Future<firebase::functions::HttpsCallableResult> FooAsync(std::function<void(int)> completion_callback)
{
    cout << "tid=" << pthread_self() << " +FooAsync(...)" << endl;
    auto future = CallAsync("foo", firebase::Variant(),
         [completion_callback](const firebase::Variant &data,
                     int error, const std::string &) {
             int value = (error == 0) ? data.int64_value() : -1;
             if (completion_callback) {
                 completion_callback(value);
             }
         });
    cout << "tid=" << pthread_self() << " -FooAsync(...)" << endl;
    return future;
}

int main() {
    ...
    future = FooAsync([](int value) {
        cout << "tid=" << pthread_self() << " FooAsync #3: value=" << value << endl;
        FooAsync([](int value) {
            cout << "tid=" << pthread_self() << " FooAsync #4: value=" << value << endl;
        });
    });
    WaitForCompletion(future);
    return 0;
}

When I run the code I get the following:

% ./FirebaseExample 
WARNING: Database URL not set in the Firebase config.
DEBUG: Creating Firebase App __FIRAPP_DEFAULT for Firebase C++ 12.1.0
DEBUG: Validating semaphore creation.
DEBUG: Added app name=__FIRAPP_DEFAULT: options, api_key=..., app_id=..., database_url=, messaging_sender_id=..., storage_bucket=..., project_id=... (0x...)
Using Functions Emulator at 127.0.0.1:5001
...
tid=0x1fefccc00 +FooAsync(...)
tid=0x1fefccc00 +Cloud::Call(`foo`, ...)
tid=0x1fefccc00 Call: `foo` +Call(...)
DEBUG: Calling Cloud Function with url: 127.0.0.1:5001/functions-futures-async-nested/us-central1/foo
data: {"data":null}
tid=0x1fefccc00 Call: `foo` -Call(...)
DEBUG: Cloud Function response body = {"result":200}
tid=0x16b7fb000 +OnCallCompleted(...)
tid=0x16b7fb000 FooAsync #3: value=200
tid=0x16b7fb000 +FooAsync(...)
tid=0x16b7fb000 +Cloud::Call(`foo`, ...)
^C

The app appears to deadlock and never complete the future and print the expected FooAsync #4: value=200. I have to Ctrl-C to end the app.

The above is simplified code for this Issue. In my actual code I also show initializing the project app and successful multiple non-async and non-nested async calls.

Steps to reproduce:

100% repro with the code/steps listed above

Relevant Code:

https://github.com/DistroAV/firebase-functions-future-async-nested

paulpv commented 3 days ago

Confirmed that if I force the nested FooAsync to be enqueued and then process on the main thread the code does not deadlock.

I don't have sample code prepared to show this yet, but hopefully you can imagine what I mean! :)