From e88fb8522d335b9a7fb723ae5656dc9b2e7850c7 Mon Sep 17 00:00:00 2001
From: José Bollo <jose.bollo@iot.bzh>
Date: Sat, 5 Jan 2019 21:22:39 +0100
Subject: Improve integration of callsync
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Avoid concurrency with the thread managing
the event loop by using condition variables.

Also, improve safety of link to event loop:
instead of using the default one that depends on the
current thread and that might be shared with other
items, use an explicit new one running in a dedicated
thread.

Change-Id: I3d6fd2fd52a97dae42228657a09b8b0a15b96edf
Signed-off-by: José Bollo <jose.bollo@iot.bzh>
---
 src/libwindowmanager.cpp | 83 +++++++++++++++++++++---------------------------
 1 file changed, 37 insertions(+), 46 deletions(-)

diff --git a/src/libwindowmanager.cpp b/src/libwindowmanager.cpp
index 515fea1..22bec4f 100644
--- a/src/libwindowmanager.cpp
+++ b/src/libwindowmanager.cpp
@@ -206,7 +206,7 @@ int LibWindowmanager::Impl::init(int port, char const *token) {
     }
 
     /* get the default event loop */
-    rc = sd_event_default(&this->loop);
+    rc = sd_event_new(&this->loop);
     if (rc < 0) {
         HMI_ERROR("libwm", "Connection to default event loop failed: %s",
                 strerror(-rc));
@@ -621,6 +621,41 @@ std::pair<bool, LibWindowmanager::EventType> make_event_type(char const *et) {
 }
 }  // namespace
 
+#include <condition_variable>
+#include <mutex>
+class callsync {
+private:
+    const std::function<void(bool, json_object *)> &onReply_;
+    std::condition_variable cond_;
+    std::mutex mutex_;
+    bool ok_;
+private:
+    static void on_reply(void *closure, afb_wsj1_msg *msg) {
+        callsync *call = reinterpret_cast<callsync*>(closure);
+        call->onReply_(call->ok_ = afb_wsj1_msg_is_reply_ok(msg), afb_wsj1_msg_object_j(msg));
+        call->cond_.notify_one();
+        afb_wsj1_msg_unref(msg);
+    }
+public:
+    callsync(
+        struct afb_wsj1 *wsj1,
+        const char *api,
+        const char *verb,
+        json_object *args,
+        const std::function<void(bool, json_object *)> &onReply
+    )
+        : onReply_(onReply), cond_(), mutex_(), ok_(false) {
+        std::unique_lock<std::mutex> ulock(mutex_);
+        int rc = afb_wsj1_call_j(wsj1, api, verb, args, on_reply, this);
+        if (rc < 0)
+            onReply(false, nullptr);
+        else
+            cond_.wait(ulock);
+    }
+    operator bool() const { return ok_; }
+    operator int() const { return int(ok_) - 1; }
+};
+
 /// object will be json_object_put
 int LibWindowmanager::Impl::api_call(
     const char *verb, json_object *object,
@@ -631,51 +666,7 @@ int LibWindowmanager::Impl::api_call(
     if ((0 == strcmp("RequestSurface", verb)) ||
         (0 == strcmp("GetDisplayInfo", verb)) ||
         (0 == strcmp("GetAreaInfo", verb))) {
-        // We need to wrap the actual onReply call once in order to
-        // *look* like a normal functions pointer (std::functions<>
-        // with captures cannot convert to function pointers).
-        // Alternatively we could setup a local struct and use it as
-        // closure, but I think it is cleaner this way.
-        int call_rc = 0;
-        std::atomic<bool> returned{};
-        returned.store(false, std::memory_order_relaxed);
-        std::function<void(bool, json_object *)> wrappedOnReply =
-            [&returned, &call_rc, &onReply](bool ok, json_object *j) {
-                TRACEN(wrappedOnReply);
-                call_rc = ok ? 0 : -EINVAL;
-                // We know it failed, but there may be an explanation in the
-                // json object.
-                {
-                    TRACEN(onReply);
-                    onReply(ok, j);
-                }
-                returned.store(true, std::memory_order_release);
-            };
-
-        // make the actual call, use wrappedOnReply as closure
-        rc = afb_wsj1_call_j(
-            this->wsj1, wmAPI, verb, object,
-            [](void *closure, afb_wsj1_msg *msg) {
-                TRACEN(callClosure);
-                auto *onReply =
-                    reinterpret_cast<std::function<void(bool, json_object *)> *>(
-                        closure);
-                (*onReply)(!(afb_wsj1_msg_is_reply_ok(msg) == 0),
-                           afb_wsj1_msg_object_j(msg));
-            },
-            &wrappedOnReply);
-
-        if (0 == rc) {
-            // We need to wait until "returned" got set, this is necessary
-            // if events get triggered by the call (and would be dispatched before
-            // the actual call-reply).
-            while (!returned.load(std::memory_order_consume)) {
-                sd_event_run(loop, 16);
-            }
-
-            // return the actual API call result
-            rc = call_rc;
-        }
+        rc = callsync(this->wsj1, wmAPI, verb, object, onReply);
     }
     else {
         rc = afb_wsj1_call_j(this->wsj1, wmAPI, verb, object, _on_reply_static, this);
-- 
cgit 1.2.3-korg