Add Thread tests; get argument transfer working for all cases

src/mbgl/util/run_loop.hpp

@@ -6,6 +6,7 @@
 #include <mbgl/util/uv_detail.hpp>
 
 #include <functional>
+#include <utility>
 #include <queue>
 #include <mutex>
 
@@ -19,37 +20,38 @@ public:
 
     void stop();
 
-    // Invoke fn() in the runloop thread.
-    template <class Fn>
-    void invoke(Fn&& fn) {
-        auto invokable = util::make_unique<Invoker<Fn>>(std::move(fn));
+    // Invoke fn(args...) on this RunLoop.
+    template <class Fn, class... Args>
+    void invoke(Fn&& fn, Args&&... args) {
+        auto tuple = std::make_tuple(std::move(args)...);
+        auto invokable = util::make_unique<Invoker<Fn, decltype(tuple), Args...>>(std::move(fn), std::move(tuple));
         withMutex([&] { queue.push(std::move(invokable)); });
         async.send();
     }
 
-    // Invoke fn() in the runloop thread, then invoke callback(result) in the current thread.
-    template <class Fn, class R>
-    void invokeWithResult(Fn&& fn, std::function<void (R)>&& callback) {
-        RunLoop* outer = current.get();
-        assert(outer);
-
-        invoke([fn = std::move(fn), callback = std::move(callback), outer] () mutable {
-            outer->invoke([callback = std::move(callback), result = std::move(fn())] () mutable {
-                callback(std::move(result));
-            });
-        });
+    // Return a function that invokes the given function on this RunLoop.
+    template <class... Args>
+    auto bind(std::function<void (Args...)> fn) {
+        return [this, fn = std::move(fn)] (Args&&... args) {
+            invoke(std::move(fn), std::move(args)...);
+        };
     }
 
-    // Invoke fn() in the runloop thread, then invoke callback() in the current thread.
-    template <class Fn>
-    void invokeWithResult(Fn&& fn, std::function<void ()>&& callback) {
-        RunLoop* outer = current.get();
-        assert(outer);
+    // Invoke fn(args...) on this RunLoop, then invoke callback(result) on the current RunLoop.
+    template <class R, class Fn, class... Args>
+    void invokeWithResult(Fn&& fn, std::function<void (R)> callback, Args&&... args) {
+        invoke([fn = std::move(fn), callback = current.get()->bind(callback)] (Args&&... a) mutable {
+            callback(fn(std::forward<Args>(a)...));
+        }, std::forward<Args>(args)...);
+    }
 
-        invoke([fn = std::move(fn), callback = std::move(callback), outer] () mutable {
-            fn();
-            outer->invoke(std::move(callback));
-        });
+    // Invoke fn(args...) on this RunLoop, then invoke callback() on the current RunLoop.
+    template <class Fn, class... Args>
+    void invokeWithResult(Fn&& fn, std::function<void ()> callback, Args&&... args) {
+        invoke([fn = std::move(fn), callback = current.get()->bind(callback)] (Args&&... a) mutable {
+            fn(std::forward<Args>(a)...);
+            callback();
+        }, std::forward<Args>(args)...);
     }
 
     uv_loop_t* get() { return async.get()->loop; }
@@ -65,11 +67,24 @@ private:
         virtual ~Message() = default;
     };
 
-    template <class F>
+    template <class F, class P, class... Args>
     struct Invoker : Message {
-        Invoker(F&& f) : func(std::move(f)) {}
-        void operator()() override { func(); }
+        Invoker(F&& f, P&& p)
+          : func(std::move(f)),
+            params(std::move(p)) {
+        }
+
+        void operator()() override {
+            invoke(std::index_sequence_for<Args...>{});
+        }
+
+        template <std::size_t... I>
+        void invoke(std::index_sequence<I...>) {
+             func(std::forward<Args>(std::get<I>(params))...);
+        }
+
         F func;
+        P params;
     };
 
     using Queue = std::queue<std::unique_ptr<Message>>;

src/mbgl/util/thread.hpp

@@ -4,26 +4,12 @@
 #include <future>
 #include <thread>
 #include <atomic>
+#include <utility>
 #include <functional>
 
 #include <mbgl/util/run_loop.hpp>
 #include <mbgl/platform/platform.hpp>
 
-namespace {
-
-template <::std::size_t...>
-struct index_sequence {};
-
-template <::std::size_t N, ::std::size_t... I>
-struct integer_sequence : integer_sequence<N - 1, N - 1, I...> {};
-
-template <::std::size_t... I>
-struct integer_sequence<0, I...> {
-    using type = index_sequence<I...>;
-};
-
-}
-
 namespace mbgl {
 namespace util {
 
@@ -50,19 +36,19 @@ public:
     // Invoke object->fn(args...) in the runloop thread.
     template <typename Fn, class... Args>
     void invoke(Fn fn, Args&&... args) {
-        loop->invoke(std::bind(fn, object, args...));
+        loop->invoke(bind<Fn, Args...>(fn), std::forward<Args>(args)...);
     }
 
     // Invoke object->fn(args...) in the runloop thread, then invoke callback(result) in the current thread.
-    template <typename Fn, class R, class... Args>
-    void invokeWithResult(Fn fn, std::function<void (R)>&& callback, Args&&... args) {
-        loop->invokeWithResult(std::bind(fn, object, std::move(args)...), std::move(callback));
+    template <class R, typename Fn, class... Args>
+    void invokeWithResult(Fn fn, std::function<void (R)> callback, Args&&... args) {
+        loop->invokeWithResult(bind<Fn, Args...>(fn), callback, std::forward<Args>(args)...);
     }
 
     // Invoke object->fn(args...) in the runloop thread, then invoke callback() in the current thread.
     template <typename Fn, class... Args>
-    void invokeWithResult(Fn fn, std::function<void ()>&& callback, Args&&... args) {
-        loop->invokeWithResult(std::bind(fn, object, std::move(args)...), std::move(callback));
+    void invokeWithResult(Fn fn, std::function<void ()> callback, Args&&... args) {
+        loop->invokeWithResult(bind<Fn, Args...>(fn), callback, std::forward<Args>(args)...);
     }
 
     // Invoke object->fn(args...) in the runloop thread, and wait for the result.
@@ -89,8 +75,13 @@ private:
     Thread& operator=(const Thread&) = delete;
     Thread& operator=(Thread&&) = delete;
 
+    template <typename Fn, class... Args>
+    auto bind(Fn fn) {
+        return [fn, this] (Args&&... a) { return (object->*fn)(std::forward<Args>(a)...); };
+    }
+
     template <typename P, std::size_t... I>
-    void run(P&& params, index_sequence<I...>);
+    void run(P&& params, std::index_sequence<I...>);
 
     std::promise<void> running;
     std::promise<void> joinable;
@@ -119,8 +110,7 @@ Thread<Object>::Thread(const std::string& name, ThreadPriority priority, Args&&.
             platform::makeThreadLowPriority();
         }
 
-        constexpr auto seq = typename integer_sequence<sizeof...(Args)>::type();
-        run(std::move(params), seq);
+        run(std::move(params), std::index_sequence_for<Args...>{});
     });
 
     running.get_future().get();
@@ -128,7 +118,7 @@ Thread<Object>::Thread(const std::string& name, ThreadPriority priority, Args&&.
 
 template <class Object>
 template <typename P, std::size_t... I>
-void Thread<Object>::run(P&& params, index_sequence<I...>) {
+void Thread<Object>::run(P&& params, std::index_sequence<I...>) {
     uv::loop l;
 
     {

test/miscellaneous/thread.cpp

@@ -0,0 +1,102 @@
+#include <mbgl/util/thread.hpp>
+#include <mbgl/util/run_loop.hpp>
+
+#include "../fixtures/util.hpp"
+
+using namespace mbgl::util;
+
+class TestObject {
+public:
+    TestObject(uv_loop_t*, std::thread::id otherTid)
+        : tid(std::this_thread::get_id()) {
+        EXPECT_NE(tid, otherTid);
+    }
+
+    void fn1(int val) {
+        EXPECT_EQ(tid, std::this_thread::get_id());
+        EXPECT_EQ(val, 1);
+    }
+
+    int fn2() {
+        EXPECT_EQ(tid, std::this_thread::get_id());
+        return 1;
+    }
+
+    void transferIn(std::unique_ptr<int> val) {
+        EXPECT_EQ(tid, std::this_thread::get_id());
+        EXPECT_EQ(*val, 1);
+    }
+
+    std::unique_ptr<int> transferOut() {
+        EXPECT_EQ(tid, std::this_thread::get_id());
+        return std::make_unique<int>(1);
+    }
+
+    std::unique_ptr<int> transferInOut(std::unique_ptr<int> val) {
+        EXPECT_EQ(tid, std::this_thread::get_id());
+        EXPECT_EQ(*val, 1);
+        return std::move(val);
+    }
+
+    void transferInShared(std::shared_ptr<int> val) {
+        EXPECT_EQ(tid, std::this_thread::get_id());
+        EXPECT_EQ(*val, 1);
+    }
+
+    std::shared_ptr<int> transferOutShared() {
+        EXPECT_EQ(tid, std::this_thread::get_id());
+        return std::make_shared<int>(1);
+    }
+
+    std::string transferString(const std::string& string) {
+        EXPECT_EQ(tid, std::this_thread::get_id());
+        EXPECT_EQ(string, "test");
+        return string;
+    }
+
+    const std::thread::id tid;
+};
+
+TEST(Thread, invoke) {
+    const std::thread::id tid = std::this_thread::get_id();
+
+    RunLoop loop(uv_default_loop());
+
+    loop.invoke([&] {
+        EXPECT_EQ(tid, std::this_thread::get_id());
+        Thread<TestObject> thread("Test", ThreadPriority::Regular, tid);
+
+        thread.invoke(&TestObject::fn1, 1);
+        thread.invokeWithResult<int>(&TestObject::fn2, [&] (int result) {
+            EXPECT_EQ(tid, std::this_thread::get_id());
+            EXPECT_EQ(result, 1);
+        });
+
+        thread.invoke(&TestObject::transferIn, std::make_unique<int>(1));
+        thread.invokeWithResult<std::unique_ptr<int>>(&TestObject::transferOut, [&] (std::unique_ptr<int> result) {
+            EXPECT_EQ(tid, std::this_thread::get_id());
+            EXPECT_EQ(*result, 1);
+        });
+
+        thread.invokeWithResult<std::unique_ptr<int>>(&TestObject::transferInOut, [&] (std::unique_ptr<int> result) {
+            EXPECT_EQ(tid, std::this_thread::get_id());
+            EXPECT_EQ(*result, 1);
+        }, std::make_unique<int>(1));
+
+        thread.invoke(&TestObject::transferInShared, std::make_shared<int>(1));
+        thread.invokeWithResult<std::shared_ptr<int>>(&TestObject::transferOutShared, [&] (std::shared_ptr<int> result) {
+            EXPECT_EQ(tid, std::this_thread::get_id());
+            EXPECT_EQ(*result, 1);
+        });
+
+        std::string test("test");
+        thread.invokeWithResult<std::string>(&TestObject::transferString, [&] (std::string result){
+            EXPECT_EQ(tid, std::this_thread::get_id());
+            EXPECT_EQ(result, "test");
+            loop.stop();
+        }, test);
+        test.clear();
+    });
+
+    uv_run(uv_default_loop(), UV_RUN_DEFAULT);
+}

test/test.gypi

@@ -53,8 +53,9 @@
         'miscellaneous/rotation_range.cpp',
         'miscellaneous/style_parser.cpp',
         'miscellaneous/text_conversions.cpp',
-        'miscellaneous/transform.cpp',
+        'miscellaneous/thread.cpp',
         'miscellaneous/tile.cpp',
+        'miscellaneous/transform.cpp',
         'miscellaneous/variant.cpp',
         'miscellaneous/worker.cpp',