From 90c90caf5df6b33c61fc4abb9aaa2812e40808fc Mon Sep 17 00:00:00 2001 From: Paul T Date: Tue, 27 Oct 2020 22:11:21 -0400 Subject: [PATCH 1/5] Make Registration Handle RAII and Allow for Multi-thread Read Mutiple threads can now read the event registration list simulatneously and the handler registration is now an RAII class that de-registers the handler upon destruction. --- eventbus/include/eventbus/event_bus.hpp | 157 ++++++++++++++++++------ 1 file changed, 119 insertions(+), 38 deletions(-) diff --git a/eventbus/include/eventbus/event_bus.hpp b/eventbus/include/eventbus/event_bus.hpp index d84500a..e1d2357 100644 --- a/eventbus/include/eventbus/event_bus.hpp +++ b/eventbus/include/eventbus/event_bus.hpp @@ -6,15 +6,31 @@ #include #include #include -#include +#include #include #include +#include namespace dp { - struct handler_registration + class event_bus; + + class handler_registration { - const void* handle{ nullptr }; + const void* handle_{ nullptr }; + dp::event_bus* event_bus_{ nullptr }; + public: + handler_registration(const handler_registration& other) = delete; + handler_registration(handler_registration&& other) noexcept; + handler_registration& operator=(const handler_registration& other) = delete; + handler_registration& operator=(handler_registration&& other) noexcept; + ~handler_registration(); + + [[nodiscard]] const void* handle() const; + void unregister() const noexcept; + protected: + handler_registration(const void* handle, dp::event_bus* bus); + friend class event_bus; }; class event_bus @@ -27,28 +43,28 @@ namespace dp { using traits = detail::function_traits; const auto type_idx = std::type_index(typeid(EventType)); - handler_registration registration; + const void* handle; if constexpr (traits::arity == 0) { - safe_registrations_access([&]() { + safe_unique_registrations_access([&]() { auto it = handler_registrations_.emplace(type_idx, [handler = std::forward(handler)](auto) { handler(); }); - registration.handle = static_cast(&(it->second)); + handle = static_cast(&(it->second)); }); } else { - safe_registrations_access([&]() { + safe_unique_registrations_access([&]() { auto it = handler_registrations_.emplace(type_idx, [func = std::forward(handler)](auto value) { func(std::any_cast(value)); }); - registration.handle = static_cast(&(it->second)); + handle = static_cast(&(it->second)); }); } - return registration; + return { handle, this }; } template @@ -58,35 +74,35 @@ namespace dp static_assert(std::is_same_v>, "Member function pointer must match instance type."); const auto type_idx = std::type_index(typeid(EventType)); - handler_registration registration{}; + const void* handle; if constexpr (traits::arity == 0) { - safe_registrations_access([&]() { + safe_unique_registrations_access([&]() { auto it = handler_registrations_.emplace(type_idx, [class_instance, function](auto) { (class_instance->*function)(); }); - registration.handle = static_cast(&(it->second)); + handle = static_cast(&(it->second)); }); } else { - safe_registrations_access([&]() { + safe_unique_registrations_access([&]() { auto it = handler_registrations_.emplace(type_idx, [class_instance, function](auto value) { (class_instance->*function)(std::any_cast(value)); }); - registration.handle = static_cast(&(it->second)); + handle = static_cast(&(it->second)); }); } - return registration; + return { handle, this }; } template void fire_event(const EventType& evt) noexcept { - safe_registrations_access([&]() { + safe_shared_registrations_access([&]() { // only call the functions we need to auto [begin_evt_id, end_evt_id] = handler_registrations_.equal_range(std::type_index(typeid(EventType))); for (; begin_evt_id != end_evt_id; ++begin_evt_id) @@ -105,31 +121,41 @@ namespace dp bool remove_handler(const handler_registration& registration) noexcept { - if (!registration.handle) { return false; } + if (!registration.handle()) { return false; } - std::lock_guard lock(registration_mutex_); - for (auto it = handler_registrations_.begin(); it != handler_registrations_.end(); ++it) - { - if (static_cast(&(it->second)) == registration.handle) + auto result = false; + safe_unique_registrations_access([this, &result, ®istration]() { - handler_registrations_.erase(it); - return true; - } - } - - return false; + for (auto it = handler_registrations_.begin(); it != handler_registrations_.end(); ++it) + { + if (static_cast(&(it->second)) == registration.handle()) + { + handler_registrations_.erase(it); + result = true; + break; + } + } + }); + return result; } void remove_handlers() noexcept { - std::lock_guard lock(registration_mutex_); - handler_registrations_.clear(); + safe_unique_registrations_access([this]() + { + handler_registrations_.clear(); + }); } [[nodiscard]] std::size_t handler_count() noexcept { - std::lock_guard lock(registration_mutex_); - return handler_registrations_.size(); + std::shared_lock lock(registration_mutex_); + std::size_t count{}; + safe_shared_registrations_access([this, &count]() + { + count = handler_registrations_.size(); + }); + return count; } private: @@ -157,21 +183,76 @@ namespace dp std::atomic lock_holder_{}; }; - using mutex_type = mutex; - mutex_type registration_mutex_; + using mutex_type = std::shared_mutex; + mutable mutex_type registration_mutex_; std::unordered_multimap> handler_registrations_; template - void safe_registrations_access(Callable&& callable) { - try { - if(registration_mutex_.locked_by_caller()) return; + void safe_shared_registrations_access(Callable&& callable) + { + try + { + std::shared_lock lock(registration_mutex_); + callable(); + } + catch (std::system_error&) + { + + } + } + template + void safe_unique_registrations_access(Callable&& callable) + { + try + { + // if(registration_mutex_.locked_by_caller()) return; // if this fails, an exception may be thrown. - std::lock_guard lock(registration_mutex_); + std::unique_lock lock(registration_mutex_); callable(); } - catch (std::system_error&) { + catch (std::system_error&) + { // do nothing } } }; + + + inline const void* handler_registration::handle() const + { + return handle_; + } + + inline void handler_registration::unregister() const noexcept + { + if (event_bus_ && handle_) + { + event_bus_->remove_handler(*this); + } + } + + inline handler_registration::handler_registration(const void* handle, dp::event_bus* bus) + : handle_(handle), event_bus_(bus) + { + } + + inline handler_registration::handler_registration(handler_registration&& other) noexcept + : handle_(std::exchange(other.handle_, nullptr)), event_bus_(std::exchange(other.event_bus_, nullptr)) + { + } + + inline handler_registration& handler_registration::operator=(handler_registration&& other) noexcept + { + handle_ = std::exchange(other.handle_, nullptr); + event_bus_ = std::exchange(other.event_bus_, nullptr); + return *this; + } + + inline handler_registration::~handler_registration() + { + if (event_bus_ && handle_) + { + event_bus_->remove_handler(*this); + } + } } From c5539360371b996e5aedd603d0751d5402bda1b7 Mon Sep 17 00:00:00 2001 From: Paul T Date: Tue, 27 Oct 2020 22:12:17 -0400 Subject: [PATCH 2/5] Update Unit Tests Updated tests to reflect changes in classes. No longer support registering or de-registering inside event callbacks. --- eventbus/test/event_bus_tests.cpp | 83 ++++++++++++++----------------- 1 file changed, 37 insertions(+), 46 deletions(-) diff --git a/eventbus/test/event_bus_tests.cpp b/eventbus/test/event_bus_tests.cpp index 1a19441..da0f13e 100644 --- a/eventbus/test/event_bus_tests.cpp +++ b/eventbus/test/event_bus_tests.cpp @@ -80,43 +80,6 @@ TEST(EventBus, LambdaRegistrationAndDeregistration) EXPECT_EQ(evt_bus.handler_count(), 1); } -TEST(EventBus, RegisterWhileDispatching) -{ - struct nefarious_event_listener - { - dp::event_bus* evt_bus; - void on_event(test_event_type) const - { - nefarious_event_listener listener{}; - listener.evt_bus = evt_bus; - if (evt_bus) - { - auto _ = evt_bus->register_handler(&listener, &nefarious_event_listener::on_event); - } - } - }; - - dp::event_bus evt_bus; - event_handler_counter counter; - const auto registration = evt_bus.register_handler(&counter, &event_handler_counter::on_test_event); - - nefarious_event_listener listener{}; - listener.evt_bus = &evt_bus; - const dp::handler_registration nef_registration = evt_bus.register_handler( - &listener, &nefarious_event_listener::on_event); - - for (auto i = 0; i < 40; ++i) { - evt_bus.fire_event(test_event_type{ 2, "test event", 1.3 }); - std::cout << "Loop " << i << " Handler count: " << evt_bus.handler_count() << "\n"; - // count should be 2 because we registered the first object and the - // test fixture class is registered as well (the counter). - ASSERT_EQ(evt_bus.handler_count(), 2); - } - - ASSERT_TRUE(evt_bus.remove_handler(registration)); - ASSERT_TRUE(evt_bus.remove_handler(nef_registration)); -} - TEST(EventBus, DeregisterWhileDispatching) { dp::event_bus evt_bus; @@ -129,7 +92,7 @@ TEST(EventBus, DeregisterWhileDispatching) void on_event(test_event_type) { if (evt_bus && registrations) { - std::for_each(registrations->begin(), registrations->end(), [&](auto reg) { + std::for_each(registrations->begin(), registrations->end(), [&](auto& reg) { evt_bus->remove_handler(reg); }); } @@ -142,7 +105,7 @@ TEST(EventBus, DeregisterWhileDispatching) deregister_while_dispatch_listener listener; auto reg = evt_bus.register_handler(&listener, &deregister_while_dispatch_listener::on_event); listeners.emplace_back(listener); - registrations.emplace_back(reg); + registrations.emplace_back(std::move(reg)); } listeners[0].evt_bus = &evt_bus; @@ -155,7 +118,7 @@ TEST(EventBus, DeregisterWhileDispatching) } // remove all the registrations - for (auto reg : registrations) { + for (auto& reg : registrations) { EXPECT_TRUE(evt_bus.remove_handler(reg)); } @@ -171,7 +134,7 @@ TEST(EventBus, MultiThreaded) { } void on_event(const test_event_type& evt) { std::cout << "simple event: " << index_ << " " << evt.event_message << "\n"; - std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); } }; @@ -179,18 +142,46 @@ TEST(EventBus, MultiThreaded) { simple_listener listener_one(1); simple_listener listener_two(2); + auto reg_one = evt_bus.register_handler(&listener_one, &simple_listener::on_event); + auto reg_two = evt_bus.register_handler(&listener_two, &simple_listener::on_event); + + event_handler_counter event_counter; + auto event_handler_reg = evt_bus.register_handler(&event_counter, &event_handler_counter::on_test_event); + auto thread_one = std::thread([&evt_bus, &listener_one]() { - evt_bus.fire_event(test_event_type{ 3, "thread_one", 1.0 }); - auto _ = evt_bus.register_handler(&listener_one, &simple_listener::on_event); + for (auto i = 0; i < 5; ++i) + { + evt_bus.fire_event(test_event_type{ 3, "thread_one", 1.0 }); + } }); auto thread_two = std::thread([&evt_bus, &listener_two]() { - auto _ = evt_bus.register_handler(&listener_two, &simple_listener::on_event); - evt_bus.fire_event(test_event_type{ 3, "thread_two", 2.0 }); + for (auto i = 0; i < 5; ++i) + { + evt_bus.fire_event(test_event_type{ 3, "thread_two", 2.0 }); + } }); thread_one.join(); thread_two.join(); - EXPECT_EQ(evt_bus.handler_count(), 2); + // include the event counter + EXPECT_EQ(evt_bus.handler_count(), 3); + + EXPECT_EQ(event_counter.get_count(), 10); +} + +TEST(EventBus, AutoDeregisterInDtor) +{ + dp::event_bus evt_bus; + event_handler_counter counter; + { + auto registration = evt_bus.register_handler(&counter, &event_handler_counter::on_test_event); + } + + EXPECT_EQ(evt_bus.handler_count(), 0); + evt_bus.fire_event(test_event_type{}); + evt_bus.fire_event(test_event_type{}); + evt_bus.fire_event(test_event_type{}); + EXPECT_EQ(counter.get_count(), 0); } \ No newline at end of file From 7e07e1a02f9a81ff959bbd02f2b83196c3dc1942 Mon Sep 17 00:00:00 2001 From: Paul T Date: Tue, 27 Oct 2020 22:12:31 -0400 Subject: [PATCH 3/5] Update Naming in Demo --- demo/main.cpp | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/demo/main.cpp b/demo/main.cpp index 1f52c09..85d14df 100644 --- a/demo/main.cpp +++ b/demo/main.cpp @@ -1,16 +1,15 @@ #include -#include #include const char new_line = '\n'; -struct dummy_event +struct first_event { std::string message; }; -struct other_event +struct second_event { int id; std::string message; @@ -21,7 +20,7 @@ struct third_event double value{ 234.00 }; }; -void foo(const dummy_event& evt) +void foo(const first_event& evt) { std::cout << "event: " << evt.message << std::endl; } @@ -30,7 +29,7 @@ class my_callback_object { public: my_callback_object() = default; - void on_event_fired(const dummy_event&) { event_count_++; } + void on_event_fired(const first_event&) { event_count_++; } void on_third_event() {}; [[nodiscard]] int get_event_count() const { return event_count_; } private: @@ -49,7 +48,7 @@ int main() event_bus evt_bus; my_callback_object callback_obj; - const auto reg = evt_bus.register_handler(&foo); + const auto reg = evt_bus.register_handler(&foo); const auto third_event_reg = evt_bus.register_handler([](const third_event& evt) { std::cout << "my third event handler: " << evt.value << new_line; @@ -57,25 +56,25 @@ int main() const auto empty_event_handler = evt_bus.register_handler([]() {std::cout << "I just do stuff when a third_event is fired." << new_line; }); - dummy_event evt{ "hello from dummy event" }; + first_event evt{ "hello from dummy event" }; evt_bus.fire_event(&evt); evt_bus.fire_event(third_event{ 13.0 }); evt_bus.remove_handler(third_event_reg); evt_bus.fire_event(third_event{ 13.0 }); - const auto other_event_reg = evt_bus.register_handler([](const other_event& other_evt) {std::cout << "first other event handler says: " << other_evt.message << std::endl; }); - const auto other_event_second_reg = evt_bus.register_handler([](const other_event& other_evt){std::cout << "second other event handler says: " << other_evt.id << " " << other_evt.message << std::endl;}); - const auto dmy_evt_first_reg = evt_bus.register_handler([](const dummy_event& dmy_evt) {std::cout << "third event handler says: " << dmy_evt.message << std::endl;}); - const auto dmy_evt_pmr_reg = evt_bus.register_handler(&callback_obj , &my_callback_object::on_event_fired); + const auto other_event_reg = evt_bus.register_handler([](const second_event& other_evt) {std::cout << "first other event handler says: " << other_evt.message << std::endl; }); + const auto other_event_second_reg = evt_bus.register_handler([](const second_event& other_evt){std::cout << "second other event handler says: " << other_evt.id << " " << other_evt.message << std::endl;}); + const auto dmy_evt_first_reg = evt_bus.register_handler([](const first_event& dmy_evt) {std::cout << "third event handler says: " << dmy_evt.message << std::endl;}); + const auto dmy_evt_pmr_reg = evt_bus.register_handler(&callback_obj , &my_callback_object::on_event_fired); const auto thrid_event_reg_pmr = evt_bus.register_handler(&callback_obj, &my_callback_object::on_third_event); // the following does not compile // third_event_object teo; // const auto rg = evt_bus.register_handler(&teo, &my_callback_object::on_third_event); - other_event other_evt{ 2, "hello there" }; - dummy_event dmy_event{ "oh boy..." }; + second_event other_evt{ 2, "hello there" }; + first_event dmy_event{ "oh boy..." }; evt_bus.fire_event(dmy_event); From cda1b6924d721ab7a01985b491aa2f96902bddaf Mon Sep 17 00:00:00 2001 From: Paul T Date: Tue, 27 Oct 2020 22:12:40 -0400 Subject: [PATCH 4/5] Switch to Ninja on Linux --- CMakeSettings.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeSettings.json b/CMakeSettings.json index aed7785..9fc4de1 100644 --- a/CMakeSettings.json +++ b/CMakeSettings.json @@ -20,7 +20,7 @@ }, { "name": "WSL-Clang-Debug", - "generator": "Unix Makefiles", + "generator": "Ninja", "configurationType": "Debug", "buildRoot": "${projectDir}\\out\\build\\${name}", "installRoot": "${projectDir}\\out\\install\\${name}", @@ -41,7 +41,7 @@ }, { "name": "WSL-GCC-Debug", - "generator": "Unix Makefiles", + "generator": "Ninja", "configurationType": "Debug", "buildRoot": "${projectDir}\\out\\build\\${name}", "installRoot": "${projectDir}\\out\\install\\${name}", From 2ad8a77908ccd82b1952e6857ab38b7c34dcd39d Mon Sep 17 00:00:00 2001 From: Paul T Date: Fri, 30 Oct 2020 23:53:19 -0400 Subject: [PATCH 5/5] Update readme for library changes. --- README.md | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 5d6e739..20658ed 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,8 @@ eventbus `eventbus` is a simple, header only C++17 event bus library that doesn't require you to inherit from any sort of `event` class. -- [Design Goals](#design-goals) +- [Overview](#overview) +- [Features](#features) - [Integration](#integration) - [CMake](#cmake) - [vcpkg](#vcpkg) @@ -48,16 +49,20 @@ eventbus - [Author](#author) - [Contributors](#contributors) -## Design Goals +## Overview -`eventbus` implements the "Mediator" pattern. This pattern is useful when you want components to communicate to each other without necessarily "knowing" about each other. This can be useful in *some* situations but should be used with caution (there are alternative design patterns to consider). +`eventbus` implements the "Mediator" pattern. This pattern is useful when you want components to communicate to each other without necessarily "knowing" about each other. Effectively, this is a thread safe event dispatcher with a list of callbacks. -- **Do not require event object inheritance** I wanted to implement an event bus system that doesn't require users to inherit from some base `Event` class in order to use the event class. -- **Flexible Callback Types** It's important that the library supports a variety different types of callbacks including: +## Features + +- **Does not require event object inheritance** A base `Event` class is not requied for use with `dp::event_bus`. Any class/struct can be used as an event object. +- **Flexible Callback Types** `eventbus` supports a variety different types of callbacks including: - Lambdas - Class member functions - Free functions -- **Flexible Callbacks** Callbacks should be able to take no input parameters, the event type by `const&` or by value. +- **Flexible Callbacks** No parameter callbacks are also supported as well as taking the event type by value or by `const &`. +- **RAII de-registrations** The handler registration objects automatically de-register the handler upon destruction. +- **Thread safety** Multiple threads can fire events at once to the same `event_bus`. Handlers can also be registered from different threads. ## Integration @@ -109,14 +114,14 @@ void event_callback(event_type evt) } dp::event_bus evt_bus; -evt_bus.register_handler(&event_callback) +const auto registration_handler = evt_bus.register_handler(&event_callback) ```` #### Lambda ````cpp dp::event_bus evt_bus; -evt_bus.register_handler([](const event_type& evt) +const auto registration_handler = evt_bus.register_handler([](const event_type& evt) { // logic code... }); @@ -137,7 +142,7 @@ class event_handler // other code dp::event_bus evt_bus; event_handler handler; -evt_bus.register_handler(&handler, &event_handler::on_event); +const auto registration_handler = evt_bus.register_handler(&handler, &event_handler::on_event); ```` **Note:** You can't mix a class instance of type `T` with the member function of another class (i.e. `&U::function_name`). @@ -159,6 +164,11 @@ A complete example can be seen in the [demo](https://github.com/DeveloperPaul123 In general, all callback functions **must** return `void`. Currently, `eventbus` only supports single argument functions as callbacks. +The following use cases are not supported: + +- Registering a callback inside an event callback. +- De-registering a callback inside an event callback. + ## Contributing If you find an issue with this library please file an [issue](https://github.com/DeveloperPaul123/eventbus/issues). Pull requests are also welcome! Please see the [contribution guidelines](CONTRIBUTING.md) for more information.