Merge lp:~marcustomlinson/unity-scope-click/lp-1591422 into lp:unity-scope-click

[Marcus Tomlinson]

Ignore promise_already_satisfied exceptions thrown from getCredentials() signal handlers

[Marcus Tomlinson (marcustomlinson)]

Looking at:

    https://errors.ubuntu.com/problem/f271ff40a203e6d22ab0bf36beb8f6f6adddb90a,

we can see that the crash originates from:

    ubuntuone_credentials.cpp:63 -> "promise.set_value(token);"

Digging a little deeper into the future source, _M_set_result() seems to only throw std::future_error for the promise_already_satisfied condition. This unhandled exception is tearing down the scope.

We have 2 places in code where getCredentials() is called:
    click::CredentialsService::getToken()
    & InstalledPreview::get_consumer_key()

I suspect that during "QCoreApplication::processEvents();" in click::CredentialsService::getToken(), we may risk processing 2 getCredentials() calls in parallel, causing multiple callbacks into the signal handler.

This change adds a safety net which may or may not make the issue go away completely, but it certainly shouldn't make things any worse.

[dobey (dobey)]

The basics of this look sound, but would be nice if the statements were structured consistent with the rest of the code, like so:

try {
    promise.set_value();
} catch (const std::future_error&) {
    // Ignore promise_already_satisfied
}

review: Needs Fixing

[lp:~marcustomlinson/unity-scope-click/lp-1591422]

463. By Marcus Tomlinson on 2016-06-14

Fixed formatting

[Marcus Tomlinson (marcustomlinson)]

> The basics of this look sound, but would be nice if the statements were
> structured consistent with the rest of the code, like so:
>
> try {
> promise.set_value();
> } catch (const std::future_error&) {
> // Ignore promise_already_satisfied
> }

Sure, done.

[dobey (dobey)]

review: Approve

=== modified file 'libclickscope/click/preview.cpp'
--- libclickscope/click/preview.cpp	2016-06-01 13:36:55 +0000
+++ libclickscope/click/preview.cpp	2016-06-14 17:24:57 +0000
@@ -833,13 +833,21 @@
                     [&promise, &sso](const UbuntuOne::Token& token) {
                     qDebug() << "Credentials found";
                     sso.clear();
-                    promise.set_value(token.consumerKey().toStdString());
+                    try {
+                        promise.set_value(token.consumerKey().toStdString());
+                    } catch (const std::future_error&) {
+                        // Ignore promise_already_satisfied
+                    }
                 });
             QObject::connect(sso.data(), &click::CredentialsService::credentialsNotFound,
                     [&promise, &sso]() {
                     qDebug() << "No credentials found";
                     sso.clear();
-                    promise.set_value("");
+                    try {
+                        promise.set_value("");
+                    } catch (const std::future_error&) {
+                        // Ignore promise_already_satisfied
+                    }
                     });
 
             sso->getCredentials();
 
=== modified file 'libclickscope/click/ubuntuone_credentials.cpp'
--- libclickscope/click/ubuntuone_credentials.cpp	2016-05-10 13:42:12 +0000
+++ libclickscope/click/ubuntuone_credentials.cpp	2016-06-14 17:24:57 +0000
@@ -60,14 +60,22 @@
                                         &u1::SSOService::credentialsFound,
                                         [this, &promise](const u1::Token& token) {
                                             emit credentialsFound(_token);
-                                            promise.set_value(token);
+                                            try {
+                                                promise.set_value(token);
+                                            } catch (const std::future_error&) {
+                                                // Ignore promise_already_satisfied
+                                            }
                                         });
         auto notfound = QObject::connect(ssoService.data(),
                                          &u1::SSOService::credentialsNotFound,
                                          [this, &promise]() {
                                              qWarning() << "No Ubuntu One token found.";
                                              emit credentialsNotFound();
-                                             promise.set_value(u1::Token());
+                                             try {
+                                                 promise.set_value(u1::Token());
+                                             } catch (const std::future_error&) {
+                                                 // Ignore promise_already_satisfied
+                                             }
                                          });
 
         getCredentials();