From b608d85db78cb672da76cb4531438ba32843c6fb Mon Sep 17 00:00:00 2001 From: Ivo van Dongen Date: Thu, 25 May 2017 09:42:48 +0300 Subject: [core] allow self closing mailbox/actor --- include/mbgl/actor/mailbox.hpp | 2 +- src/mbgl/actor/mailbox.cpp | 5 +++-- test/actor/actor.test.cpp | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/include/mbgl/actor/mailbox.hpp b/include/mbgl/actor/mailbox.hpp index 1327e8e6fe..8ecf91701a 100644 --- a/include/mbgl/actor/mailbox.hpp +++ b/include/mbgl/actor/mailbox.hpp @@ -23,7 +23,7 @@ public: private: Scheduler& scheduler; - std::mutex receivingMutex; + std::recursive_mutex receivingMutex; std::mutex pushingMutex; bool closed { false }; diff --git a/src/mbgl/actor/mailbox.cpp b/src/mbgl/actor/mailbox.cpp index 947f6f9028..373c24275f 100644 --- a/src/mbgl/actor/mailbox.cpp +++ b/src/mbgl/actor/mailbox.cpp @@ -15,7 +15,8 @@ void Mailbox::close() { // must not block send(). Of the two, the receiving mutex must be acquired first, because that is // the order that an actor will obtain them when it self-sends a message, and consistent lock // acquisition order prevents deadlocks. - std::lock_guard receivingLock(receivingMutex); + // The receiving mutex is recursive to allow a mailbox (and thus the actor) to close itself. + std::lock_guard receivingLock(receivingMutex); std::lock_guard pushingLock(pushingMutex); closed = true; @@ -37,7 +38,7 @@ void Mailbox::push(std::unique_ptr message) { } void Mailbox::receive() { - std::lock_guard receivingLock(receivingMutex); + std::lock_guard receivingLock(receivingMutex); if (closed) { return; diff --git a/test/actor/actor.test.cpp b/test/actor/actor.test.cpp index 9db6882889..638c24ed7d 100644 --- a/test/actor/actor.test.cpp +++ b/test/actor/actor.test.cpp @@ -6,6 +6,7 @@ #include #include #include +#include using namespace mbgl; using namespace std::chrono_literals; @@ -123,6 +124,37 @@ TEST(Actor, DestructionBlocksOnSend) { thread.join(); } +TEST(Actor, DestructionAllowedInReceiveOnSameThread) { + // Destruction doesn't block if occurring on the same + // thread as receive(). This prevents deadlocks and + // allows for self-closing actors + + struct Test { + + Test(ActorRef){}; + + void callMeBack(std::function callback) { + callback(); + } + }; + + ThreadPool pool { 1 }; + + std::promise callbackFiredPromise; + + auto test = std::make_unique>(pool); + + // Callback (triggered while mutex is locked in Mailbox::receive()) + test->invoke(&Test::callMeBack, [&]() { + // Destroy the Actor/Mailbox in the same thread + test.reset(); + callbackFiredPromise.set_value(); + }); + + auto status = callbackFiredPromise.get_future().wait_for(std::chrono::seconds(1)); + ASSERT_EQ(std::future_status::ready, status); +} + TEST(Actor, OrderedMailbox) { // Messages are processed in order. -- cgit v1.2.1