Unverified Commit b055e589 authored by Dominik Charousset's avatar Dominik Charousset Committed by GitHub

Merge pull request #759

 Fix potential deadlock in the thread-safe clock.
Close #755.
parents 902ad7e9 e2821a42
......@@ -19,13 +19,14 @@
#pragma once
#include <array>
#include <mutex>
#include <atomic>
#include <string>
#include <memory>
#include <condition_variable>
#include <cstddef>
#include <functional>
#include <condition_variable>
#include <memory>
#include <mutex>
#include <string>
#include <typeinfo>
#include "caf/abstract_actor.hpp"
#include "caf/actor_cast.hpp"
......@@ -164,6 +165,9 @@ public:
virtual ~module();
/// Returns the human-redable name of the module.
const char* name() const noexcept;
/// Starts any background threads needed by the module.
virtual void start() = 0;
......
......@@ -58,8 +58,8 @@ public:
void cancel_dispatch_loop();
private:
std::mutex mx_;
std::condition_variable cv_;
std::recursive_mutex mx_;
std::condition_variable_any cv_;
std::atomic<bool> done_;
};
......
......@@ -118,10 +118,34 @@ using variant_visit_result_t =
template <class... Ts>
class variant {
public:
// -- member types -----------------------------------------------------------
using types = detail::type_list<Ts...>;
using type0 = typename detail::tl_at<types, 0>::type;
// -- constants --------------------------------------------------------------
/// Stores the ID for the last type.
static constexpr int max_type_id = sizeof...(Ts) - 1;
/// Stores whether all types are nothrow constructible.
static constexpr bool nothrow_move_construct =
detail::conjunction<
std::is_nothrow_move_constructible<Ts>::value...
>::value;
/// Stores whether all types are nothrow assignable *and* constructible. We
/// need to check both, since assigning to a variant results in a
/// move-contruct unless the before and after types are the same.
static constexpr bool nothrow_move_assign =
nothrow_move_construct
&& detail::conjunction<
std::is_nothrow_move_assignable<Ts>::value...
>::value;
// -- sanity checks ----------------------------------------------------------
static_assert(sizeof...(Ts) <= 20, "Too many template arguments given.");
static_assert(sizeof...(Ts) > 0, "No template argument given.");
......@@ -129,19 +153,7 @@ public:
static_assert(!detail::tl_exists<types, std::is_reference>::value,
"Cannot create a variant of references");
using type0 = typename detail::tl_at<types, 0>::type;
variant& operator=(const variant& other) {
variant_assign_helper<variant> helper{*this};
other.template apply<void>(helper);
return *this;
}
variant& operator=(variant&& other) {
variant_move_helper<variant> helper{*this};
other.template apply<void>(helper);
return *this;
}
// -- constructors, destructors, and assignment operators --------------------
variant() : type_(variant_npos) {
// Never empty ...
......@@ -151,14 +163,16 @@ public:
}
template <class U>
variant(U&& arg) : type_(variant_npos) {
variant(U&& arg)
noexcept(std::is_rvalue_reference<U&&>::value && nothrow_move_assign)
: type_(variant_npos) {
set(std::forward<U>(arg));
}
template <class U>
variant& operator=(U&& arg) {
set(std::forward<U>(arg));
return *this;
variant(variant&& other) noexcept(nothrow_move_construct)
: type_(variant_npos) {
variant_move_helper<variant> helper{*this};
other.template apply<void>(helper);
}
variant(const variant& other) : type_(variant_npos) {
......@@ -166,15 +180,30 @@ public:
other.template apply<void>(helper);
}
variant(variant&& other) : type_(variant_npos) {
variant& operator=(const variant& other) {
variant_assign_helper<variant> helper{*this};
other.template apply<void>(helper);
return *this;
}
variant& operator=(variant&& other) noexcept(nothrow_move_assign) {
variant_move_helper<variant> helper{*this};
other.template apply<void>(helper);
return *this;
}
template <class U>
variant& operator=(U&& arg) noexcept(nothrow_move_assign) {
set(std::forward<U>(arg));
return *this;
}
~variant() {
destroy_data();
}
// -- properties -------------------------------------------------------------
constexpr size_t index() const {
return static_cast<size_t>(type_);
}
......
......@@ -203,6 +203,21 @@ actor_system::module::~module() {
// nop
}
const char* actor_system::module::name() const noexcept {
switch (id()) {
case scheduler:
return "Scheduler";
case middleman:
return "Middleman";
case opencl_manager:
return "OpenCL Manager";
case openssl_manager:
return "OpenSSL Manager";
default:
return "???";
}
}
actor_system::actor_system(actor_system_config& cfg)
: ids_(0),
types_(*this),
......@@ -310,9 +325,13 @@ actor_system::~actor_system() {
// group module is the first one, relies on MM
groups_.stop();
// stop modules in reverse order
for (auto i = modules_.rbegin(); i != modules_.rend(); ++i)
if (*i)
(*i)->stop();
for (auto i = modules_.rbegin(); i != modules_.rend(); ++i) {
auto& ptr = *i;
if (ptr != nullptr) {
CAF_LOG_DEBUG("stop module" << ptr->name());
ptr->stop();
}
}
await_detached_threads();
registry_.stop();
}
......
......@@ -23,7 +23,7 @@ namespace detail {
namespace {
using guard_type = std::unique_lock<std::mutex>;
using guard_type = std::unique_lock<std::recursive_mutex>;
} // namespace <anonymous>
......@@ -110,6 +110,9 @@ void thread_safe_actor_clock::run_dispatch_loop() {
guard_type guard{mx_};
while (done_ == false) {
// Wait for non-empty schedule.
// Note: The thread calling run_dispatch_loop() is guaranteed not to lock
// the mutex recursively. Otherwise, cv_.wait() or cv_.wait_until()
// would be unsafe, because wait operations call unlock() only once.
if (schedule_.empty()) {
cv_.wait(guard);
} else {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment