[core] Avoid blocking in Thread<Object> constructor (#12151)

* Introduce AspiringActor, EstablishedActor

This pair of objects represents the two-phase (parent-thread /
child-thread) construction that's needed to support constructing
Thread<Object> without blocking until the child thread is up and
running.

An `AspiringActor<O>` is responsible for:
 - ownership of the actor's `Mailbox`
 - allocating the memory for (but *not* constructing) the target object `O`

Using these two pieces--the mailbox and a stable address for `O`--an
`AspiringActor<O>` can accept messages for the target object, or provide
`ActorRef<O>`s that do so, before the object has actually been
constructed by the corresponding `EstablishedActor<O>`.  (Such messages
are queued in the mailbox until after the object is constructed.)

This allows for an `AspiringActor<O>` to be created and safely used by a
thread other than the one on which the target object will (eventually)
live.

An `EstablishedActor<O>` is responsible for managing the lifetime of the
target object `O` and the open/closed state of the parent's `mailbox`.

The `O` object's lifetime is contained by that of its owning
`EstablishedActor<O>`: the `EstablishedActor` constructor executes the
`O` constructor via "placement new", constructing it at the address
provided by the parent `AspiringActor`, and the `~EstablishedActor`
destructor similarly executes the `~O` destructor (after closing the
mailbox). `EstablishedActor` should therefore live entirely on the
thread intended to own `O`.

* Remove Actor#{invoke,ask}

6 files changed, 212 insertions, 70 deletions

diff --git a/include/mbgl/actor/actor.hpp b/include/mbgl/actor/actor.hpp
index a0df19208e..0052fad242 100644
--- a/include/mbgl/actor/actor.hpp
+++ b/include/mbgl/actor/actor.hpp
@@ -1,5 +1,7 @@
 #pragma once
 
+#include <mbgl/actor/aspiring_actor.hpp>
+#include <mbgl/actor/established_actor.hpp>
 #include <mbgl/actor/mailbox.hpp>
 #include <mbgl/actor/message.hpp>
 #include <mbgl/actor/actor_ref.hpp>
@@ -8,6 +10,7 @@
 #include <memory>
 #include <future>
 #include <type_traits>
+#include <cassert>
 
 namespace mbgl {
 
@@ -34,65 +37,33 @@ namespace mbgl {
     the lifetime of the owning Actor, and sending a message to a `Ref` whose `Actor` has died is
     a no-op. (In the future, a dead-letters queue or log may be implemented.)
 
-    Construction and destruction of an actor is currently synchronous: the corresponding `O`
+    Construction and destruction of an Actor is synchronous: the corresponding `O`
     object is constructed synchronously by the `Actor` constructor, and destructed synchronously
     by the `~Actor` destructor, after ensuring that the `O` is not currently receiving an
-    asynchronous message. (Construction and destruction may change to be asynchronous in the
-    future.) The constructor of `O` is passed an `ActorRef<O>` referring to itself (which it
-    can use to self-send messages), followed by the forwarded arguments passed to `Actor<O>`.
+    asynchronous message. The constructor of `O` is passed an `ActorRef<O>` referring to itself
+    (which it can use to self-send messages), followed by the forwarded arguments passed to
+    `Actor<O>`.  Asynchronous object construction can be accomplished by directly using the
+    lower-level types, `AspiringActor<O>` and `EstablishedActor<O>`.
 
     Please don't send messages that contain shared pointers or references. That subverts the
     purpose of the actor model: prohibiting direct concurrent access to shared state.
 */
-
 template <class Object>
-class Actor : public util::noncopyable {
+class Actor {
 public:
+    template <class... Args>
+    Actor(Scheduler& scheduler, Args&&... args)
+        : target(scheduler, parent, std::forward<Args>(args)...) {}
 
-    // Enabled for Objects with a constructor taking ActorRef<Object> as the first parameter
-    template <typename U = Object, class... Args, typename std::enable_if<std::is_constructible<U, ActorRef<U>, Args...>::value>::type * = nullptr>
-    Actor(Scheduler& scheduler, Args&&... args_)
-            : mailbox(std::make_shared<Mailbox>(scheduler)),
-              object(self(), std::forward<Args>(args_)...) {
-    }
-
-    // Enabled for plain Objects
-    template<typename U = Object, class... Args, typename std::enable_if<!std::is_constructible<U, ActorRef<U>, Args...>::value>::type * = nullptr>
-    Actor(Scheduler& scheduler, Args&& ... args_)
-            : mailbox(std::make_shared<Mailbox>(scheduler)), object(std::forward<Args>(args_)...) {
-    }
-
-    ~Actor() {
-        mailbox->close();
-    }
-
-    template <typename Fn, class... Args>
-    void invoke(Fn fn, Args&&... args) {
-        mailbox->push(actor::makeMessage(object, fn, std::forward<Args>(args)...));
-    }
-
-    template <typename Fn, class... Args>
-    auto ask(Fn fn, Args&&... args) {
-        // Result type is deduced from the function's return type
-        using ResultType = typename std::result_of<decltype(fn)(Object, Args...)>::type;
-
-        std::promise<ResultType> promise;
-        auto future = promise.get_future();
-        mailbox->push(actor::makeMessage(std::move(promise), object, fn, std::forward<Args>(args)...));
-        return future;
-    }
+    Actor(const Actor&) = delete;
 
     ActorRef<std::decay_t<Object>> self() {
-        return ActorRef<std::decay_t<Object>>(object, mailbox);
-    }
-
-    operator ActorRef<std::decay_t<Object>>() {
-        return self();
+        return parent.self();
     }
 
 private:
-    std::shared_ptr<Mailbox> mailbox;
-    Object object;
+    AspiringActor<Object> parent;
+    EstablishedActor<Object> target;
 };
 
 } // namespace mbgl
diff --git a/include/mbgl/actor/aspiring_actor.hpp b/include/mbgl/actor/aspiring_actor.hpp
new file mode 100644
index 0000000000..6c410cdfca
--- /dev/null
+++ b/include/mbgl/actor/aspiring_actor.hpp
@@ -0,0 +1,60 @@
+#pragma once
+
+#include <mbgl/actor/mailbox.hpp>
+#include <mbgl/actor/message.hpp>
+#include <mbgl/actor/actor_ref.hpp>
+
+#include <memory>
+#include <future>
+#include <type_traits>
+#include <cassert>
+
+namespace mbgl {
+
+template <class Object>
+class EstablishedActor;
+
+template <class Object>
+class Actor;
+
+/*
+    An `AspiringActor<O>` is one half of the pair of types that comprise an actor (see `Actor<O>`),
+    the other half being `EstablishedActor<O>`.  It is responsible for:
+     - ownership of the actor's `Mailbox`
+     - allocating the memory for (but *not* constructing) the target object `O`
+ 
+    Using these two pieces--the mailbox and a stable address for `O`--an `AspiringActor<O>` can
+    accept messages for the target object, or provide `ActorRef<O>`s that do so, before the object
+    has actually been constructed by the corresponding `EstablishedActor<O>`.  (Such messages are
+    queued in the mailbox until after the object is constructed.)
+ 
+    This allows for an `AspiringActor<O>` to be created and safely used by a thread other than the
+    one on which the target object will (eventually) live.
+*/
+template <class Object>
+class AspiringActor {
+public:
+    AspiringActor() : mailbox(std::make_shared<Mailbox>()) {
+        // mailbox starts closed because the `Object` hasn't yet been constructed
+        assert(!mailbox->isOpen());
+    }
+    
+    AspiringActor(const AspiringActor&) = delete;
+    
+    ActorRef<std::decay_t<Object>> self() {
+        return ActorRef<std::decay_t<Object>>(object(), mailbox);
+    }
+    
+private:
+    std::shared_ptr<Mailbox> mailbox;
+    std::aligned_storage_t<sizeof(Object)> objectStorage;
+    
+    Object& object() {
+        return *reinterpret_cast<Object *>(&objectStorage);
+    }
+    
+    friend class EstablishedActor<Object>;
+    friend class Actor<Object>;
+};
+
+} // namespace mbgl
diff --git a/include/mbgl/actor/established_actor.hpp b/include/mbgl/actor/established_actor.hpp
new file mode 100644
index 0000000000..da0d8ac705
--- /dev/null
+++ b/include/mbgl/actor/established_actor.hpp
@@ -0,0 +1,80 @@
+#pragma once
+
+#include <mbgl/actor/aspiring_actor.hpp>
+#include <mbgl/actor/mailbox.hpp>
+#include <mbgl/actor/message.hpp>
+#include <mbgl/actor/actor_ref.hpp>
+
+#include <memory>
+#include <future>
+#include <type_traits>
+#include <cassert>
+
+namespace mbgl {
+
+/*
+    An `EstablishedActor<O>` is one half of the pair of types that comprise an actor (see `Actor<O>`),
+    the other half being `AspiringActor<O>`.  It is responsible for managing the lifetime of the
+    target object `O` and the open/closed state of the parent's `mailbox`.
+ 
+    The `O` object's lifetime is contained by that of its owning `EstablishedActor<O>`: the 
+    `EstablishedActor` constructor executes the `O` constructor via "placement new", constructing
+    it at the address provided by the parent `AspiringActor`, and the `~EstablishedActor` destructor
+    similarly executes the `~O` destructor (after closing the mailbox). `EstablishedActor` should
+    therefore live entirely on the thread intended to own `O`.
+*/
+
+template <class Object>
+class EstablishedActor {
+public:
+    // Construct the Object from a parameter pack `args` (i.e. `Object(args...)`)
+    template <typename U = Object, class... Args, typename std::enable_if<
+        std::is_constructible<U, Args...>::value ||
+        std::is_constructible<U, ActorRef<U>, Args...>::value
+    >::type * = nullptr>
+    EstablishedActor(Scheduler& scheduler, AspiringActor<Object>& parent_, Args&& ... args)
+    :   parent(parent_) {
+        emplaceObject(std::forward<Args>(args)...);
+        parent.mailbox->open(scheduler);
+    }
+    
+    // Construct the `Object` from a tuple containing the constructor arguments (i.e.
+    // `Object(std::get<0>(args), std::get<1>(args), ...)`)
+    template <class ArgsTuple, std::size_t ArgCount = std::tuple_size<std::decay_t<ArgsTuple>>::value>
+    EstablishedActor(Scheduler& scheduler, AspiringActor<Object>& parent_, ArgsTuple&& args)
+    :   parent(parent_) {
+        emplaceObject(std::forward<ArgsTuple>(args), std::make_index_sequence<ArgCount>{});
+        parent.mailbox->open(scheduler);
+    }
+    
+    EstablishedActor(const EstablishedActor&) = delete;
+
+    ~EstablishedActor() {
+        parent.mailbox->close();
+        parent.object().~Object();
+    }
+    
+private:
+    // Enabled for Objects with a constructor taking ActorRef<Object> as the first parameter
+    template <typename U = Object, class... Args, typename std::enable_if<std::is_constructible<U, ActorRef<U>, Args...>::value>::type * = nullptr>
+    void emplaceObject(Args&&... args_) {
+        new (&parent.objectStorage) Object(parent.self(), std::forward<Args>(args_)...);
+    }
+
+    // Enabled for plain Objects
+    template <typename U = Object, class... Args, typename std::enable_if<std::is_constructible<U, Args...>::value>::type * = nullptr>
+    void emplaceObject(Args&&... args_) {
+        new (&parent.objectStorage) Object(std::forward<Args>(args_)...);
+    }
+    
+    // Used to expand a tuple holding the constructor arguments
+    template <class ArgsTuple, std::size_t... I>
+    void emplaceObject(ArgsTuple&& args, std::index_sequence<I...>) {
+        emplaceObject(std::move(std::get<I>(std::forward<ArgsTuple>(args)))...);
+        (void) args; // mark args as used: if it's empty tuple, it's not actually used above.
+    }
+
+    AspiringActor<Object>& parent;
+};
+
+} // namespace mbgl
diff --git a/include/mbgl/actor/mailbox.hpp b/include/mbgl/actor/mailbox.hpp
index 8ecf91701a..23c579917a 100644
--- a/include/mbgl/actor/mailbox.hpp
+++ b/include/mbgl/actor/mailbox.hpp
@@ -1,5 +1,7 @@
 #pragma once
 
+#include <mbgl/util/optional.hpp>
+
 #include <memory>
 #include <mutex>
 #include <queue>
@@ -11,17 +13,30 @@ class Message;
 
 class Mailbox : public std::enable_shared_from_this<Mailbox> {
 public:
+   
+    // Create a "holding" mailbox, messages to which will remain queued,
+    // unconsumed, until the mailbox is associated with a Scheduler using
+    // start(). This allows a Mailbox object to be created on one thread and
+    // later transferred to a different target thread that may not yet exist.
+    Mailbox();
+    
     Mailbox(Scheduler&);
 
-    void push(std::unique_ptr<Message>);
-
+    // Attach the given scheduler to this mailbox and begin processing messages
+    // sent to it. The mailbox must be a "holding" mailbox, as created by the
+    // default constructor Mailbox().
+    void open(Scheduler& scheduler_);
     void close();
+
+    bool isOpen() const;
+
+    void push(std::unique_ptr<Message>);
     void receive();
 
     static void maybeReceive(std::weak_ptr<Mailbox>);
 
 private:
-    Scheduler& scheduler;
+    optional<Scheduler*> scheduler;
 
     std::recursive_mutex receivingMutex;
     std::mutex pushingMutex;
diff --git a/include/mbgl/actor/scheduler.hpp b/include/mbgl/actor/scheduler.hpp
index d8a26ebeab..75ead29f0a 100644
--- a/include/mbgl/actor/scheduler.hpp
+++ b/include/mbgl/actor/scheduler.hpp
@@ -31,6 +31,11 @@ class Mailbox;
 class Scheduler {
 public:
     virtual ~Scheduler() = default;
+    
+    // Used by a Mailbox when it has a message in its queue to request that it
+    // be scheduled. Specifically, the scheduler is expected to asynchronously
+    // call `receive() on the given mailbox, provided it still exists at that
+    // time.
     virtual void schedule(std::weak_ptr<Mailbox>) = 0;
 
     // Set/Get the current Scheduler for this thread
diff --git a/include/mbgl/util/thread.hpp b/include/mbgl/util/thread.hpp
index 74e722b02d..bc58427349 100644
--- a/include/mbgl/util/thread.hpp
+++ b/include/mbgl/util/thread.hpp
@@ -37,45 +37,55 @@ namespace util {
 // - `Object` can use `Timer` and do asynchronous I/O, like wait for sockets events.
 //
 template<class Object>
-class Thread : public Scheduler {
+class Thread {
 public:
     template <class... Args>
     Thread(const std::string& name, Args&&... args) {
-        std::promise<void> running;
 
-        thread = std::thread([&] {
+        std::promise<void> running_;
+        running = running_.get_future();
+
+        auto capturedArgs = std::make_tuple(std::forward<Args>(args)...);
+
+        thread = std::thread([
+            this,
+            name,
+            capturedArgs = std::move(capturedArgs),
+            runningPromise = std::move(running_)
+        ] () mutable {
             platform::setCurrentThreadName(name);
             platform::makeThreadLowPriority();
 
             util::RunLoop loop_(util::RunLoop::Type::New);
             loop = &loop_;
+            EstablishedActor<Object> establishedActor(loop_, object, std::move(capturedArgs));
 
-            object = std::make_unique<Actor<Object>>(*this, std::forward<Args>(args)...);
-            running.set_value();
-
+            runningPromise.set_value();
+            
             loop->run();
+            
+            (void) establishedActor;
+            
             loop = nullptr;
         });
-
-        running.get_future().get();
     }
 
-    ~Thread() override {
+    ~Thread() {
         if (paused) {
             resume();
         }
 
-        std::promise<void> joinable;
+        std::promise<void> stoppable;
+        
+        running.wait();
 
-        // Kill the actor, so we don't get more
-        // messages posted on this scheduler after
-        // we delete the RunLoop.
+        // Invoke a noop task on the run loop to ensure that we're executing
+        // run() before we call stop()
         loop->invoke([&] {
-            object.reset();
-            joinable.set_value();
+            stoppable.set_value();
         });
 
-        joinable.get_future().get();
+        stoppable.get_future().get();
 
         loop->stop();
         thread.join();
@@ -85,8 +95,8 @@ public:
     // can be used to send messages to `Object`. It is safe
     // to the non-owning reference to outlive this object
     // and be used after the `Thread<>` gets destroyed.
-    ActorRef<std::decay_t<Object>> actor() const {
-        return object->self();
+    ActorRef<std::decay_t<Object>> actor() {
+        return object.self();
     }
 
     // Pauses the `Object` thread. It will prevent the object to wake
@@ -103,6 +113,8 @@ public:
 
         auto pausing = paused->get_future();
 
+        running.wait();
+
         loop->invoke(RunLoop::Priority::High, [this] {
             auto resuming = resumed->get_future();
             paused->set_value();
@@ -127,13 +139,12 @@ public:
 private:
     MBGL_STORE_THREAD(tid);
 
-    void schedule(std::weak_ptr<Mailbox> mailbox) override {
-        loop->schedule(mailbox);
-    }
+    AspiringActor<Object> object;
 
     std::thread thread;
-    std::unique_ptr<Actor<Object>> object;
 
+    std::future<void> running;
+    
     std::unique_ptr<std::promise<void>> paused;
     std::unique_ptr<std::promise<void>> resumed;