From ae246a2db7125ac3e8af6ec5d9bf6aa4cf236163 Mon Sep 17 00:00:00 2001 From: Jessica James Date: Sat, 2 Jun 2018 21:16:03 -0500 Subject: [PATCH] General `timer` refactoring; still needs documentation --- CMakeLists.txt | 4 +- src/common/CMakeLists.txt | 2 +- src/common/thread_pool.cpp | 75 +++++----- src/common/timer.cpp | 171 ----------------------- src/common/timer/cancel_token.cpp | 75 ++++++++++ src/common/timer/synchronized_timer.cpp | 78 +++++++++++ src/common/timer/timer.cpp | 164 ++++++++++++++++++++++ src/common/timer/timer_context.cpp | 119 ++++++++++++++++ src/common/{ => timer}/timer_manager.cpp | 65 +++++---- src/include/assert.hpp | 68 +++++++++ src/include/impl/timer_context.hpp | 75 ++++++++++ src/include/impl/timer_manager.hpp | 25 ++-- src/include/thread_pool.hpp | 6 +- src/include/timer.hpp | 88 ++++++++---- src/test/test.hpp | 3 +- src/test/thread_pool.cpp | 38 ++--- src/test/timer.cpp | 120 ++++++++++++++-- 17 files changed, 874 insertions(+), 302 deletions(-) delete mode 100644 src/common/timer.cpp create mode 100644 src/common/timer/cancel_token.cpp create mode 100644 src/common/timer/synchronized_timer.cpp create mode 100644 src/common/timer/timer.cpp create mode 100644 src/common/timer/timer_context.cpp rename src/common/{ => timer}/timer_manager.cpp (50%) create mode 100644 src/include/assert.hpp create mode 100644 src/include/impl/timer_context.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 40aa5c5..1ade743 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,9 @@ cmake_minimum_required(VERSION 3.8) project(jessilib) -#set(CMAKE_CXX_STANDARD 14) +set(CMAKE_CXX_STANDARD 17) + +include(build/CMakeLists.txt) # Setup source files add_subdirectory(src) diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 6edfa5d..37573d3 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -3,6 +3,6 @@ cmake_minimum_required(VERSION 3.8) # Setup source files include_directories(../include) set(SOURCE_FILES - timer.cpp timer_manager.cpp thread_pool.cpp) + timer/timer.cpp timer/timer_manager.cpp thread_pool.cpp timer/timer_context.cpp timer/cancel_token.cpp timer/synchronized_timer.cpp) add_library(jessilib ${SOURCE_FILES}) diff --git a/src/common/thread_pool.cpp b/src/common/thread_pool.cpp index a05e14b..d9cb9dc 100644 --- a/src/common/thread_pool.cpp +++ b/src/common/thread_pool.cpp @@ -18,13 +18,20 @@ #include "thread_pool.hpp" #include +#include namespace jessilib { // thread_pool +unsigned int thread_pool::default_threads() { + static constexpr unsigned int MIN_THREADS{ 1 }; + static const unsigned int DEFAULT_THREADS{ std::max(std::thread::hardware_concurrency(), MIN_THREADS) }; + return DEFAULT_THREADS; +}; + thread_pool::thread_pool() - : thread_pool{ std::thread::hardware_concurrency() } { + : thread_pool{ default_threads() } { } thread_pool::thread_pool(size_t in_threads) @@ -32,42 +39,40 @@ thread_pool::thread_pool(size_t in_threads) assert(in_threads != 0); while (in_threads != 0) { - thread& self = m_threads[--in_threads]; - self.m_thread = std::thread([this, &self]() { + thread& worker = m_threads[--in_threads]; + worker.m_thread = std::thread([this, &worker]() { while (true) { // Run next pending task, if there is any - self.m_task = pop_task(); - if (self.m_task != nullptr) { - self.run_task(); + worker.m_task = pop_task(); + if (worker.m_task != nullptr) { + worker.run_task(); continue; } - if (self.m_shutdown) { - break; - } - - // Push inactive thread { - std::lock_guard guard(m_inactive_threads_mutex); - m_inactive_threads.push(&self); + // Check if we're shutting down + std::unique_lock notifier_guard(worker.m_notifier_mutex); + if (worker.m_shutdown) { + break; + } + + // Push inactive thread + { + std::lock_guard inactive_threads_guard(m_inactive_threads_mutex); + m_inactive_threads.push(&worker); + } + + // Wait for notification + worker.m_notifier.wait(notifier_guard); } - // Wait for notification - self.wait(); - // Run task - self.run_task(); + worker.run_task(); } }); } } -template -T lock_helper(T&& in_obj, std::mutex& in_mutex) { - std::lock_guard guard(in_mutex); - return std::move(in_obj); -} - thread_pool::~thread_pool() { join(); } @@ -88,14 +93,21 @@ void thread_pool::push(task_t in_task) { void thread_pool::join() { std::lock_guard guard(m_threads_mutex); - // Join threads - for (thread& thread : m_threads) { + // Shutdown threads + for (thread& worker : m_threads) { // Mark thread for shutdown - thread.m_shutdown = true; - thread.m_notifier.notify_one(); + { + std::unique_lock guard(worker.m_notifier_mutex); + worker.m_shutdown = true; + } + + // Notify thread to shutdown + worker.m_notifier.notify_one(); + } - // Wait for thread to complete - thread.m_thread.join(); + // Join threads + for (thread& worker : m_threads) { + worker.m_thread.join(); } // Cleanup threads @@ -151,9 +163,4 @@ void thread_pool::thread::run_task() { } } -void thread_pool::thread::wait() { - std::unique_lock guard(m_notifier_mutex); - m_notifier.wait(guard); -} - } // namespace jessilib diff --git a/src/common/timer.cpp b/src/common/timer.cpp deleted file mode 100644 index 55332ea..0000000 --- a/src/common/timer.cpp +++ /dev/null @@ -1,171 +0,0 @@ -/** - * Copyright (C) 2017 Jessica James. - * - * Permission to use, copy, modify, and/or distribute this software for any - * purpose with or without fee is hereby granted, provided that the above - * copyright notice and this permission notice appear in all copies. - * - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY - * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION - * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN - * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. - * - * Written by Jessica James - */ - -#include "timer.hpp" -#include -#include -#include "impl/timer_manager.hpp" - -namespace jessilib { - -/** helper */ -timer::function_t callback_with_iterations(timer::iterations_t iterations, timer::function_t callback) { - assert(iterations > 0); - - return [iterations, callback](timer& timer) mutable { - callback(timer); - - if (--iterations == 0) { - timer.cancel(); - } - }; -} - -/** timer */ - -timer::timer() - : m_callback{ nullptr }, - m_self{ impl::timer_manager::instance().m_detached_timers.end() } { - // Empty ctor body -} - -timer::timer(timer&& in_timer) - : m_period{ in_timer.m_period }, - m_next{ in_timer.m_next }, - m_callback{ std::move(in_timer.m_callback) }, - m_self{ impl::timer_manager::instance().m_detached_timers.end() } { - - // Cancel in_timer - in_timer.m_callback = nullptr; - - // Replace active timer - impl::timer_manager& manager = impl::timer_manager::instance(); - { - std::lock_guard lock(manager.m_mutex); - manager.m_active_timers.erase(&in_timer); // remove in_timer - manager.m_active_timers.insert(this); // insert this - } - - // Notify - manager.m_cvar.notify_one(); -} - -timer::timer(duration_t in_period, function_t in_callback) - : m_period{ in_period }, - m_next{ calc_next() }, - m_callback{ in_callback }, - m_self{ impl::timer_manager::instance().m_detached_timers.end() } { - // PROBLEM: timer may be executing while moving data???? - - // Assertion checks - assert(m_callback != nullptr); - assert(m_period.count() != 0); - - // Add timer - impl::timer_manager& manager = impl::timer_manager::instance(); - { - std::lock_guard lock(manager.m_mutex); - manager.m_active_timers.insert(this); - } - - // Notify - manager.m_cvar.notify_one(); -} - -timer::timer(duration_t in_period, iterations_t in_iterations, function_t in_callback) - : timer{ in_period, callback_with_iterations(in_iterations, in_callback) } { - // Empty ctor body -} - -timer& timer::operator=(timer&& in_timer) { - impl::timer_manager& manager = impl::timer_manager::instance(); - - m_period = in_timer.m_period; - m_next = in_timer.m_next; - m_callback = std::move(in_timer.m_callback); - m_self = in_timer.m_self; -} - -timer::~timer() { - // If it's null, then it was either never added (default constructed) or already removed (as part of move) - if (!null()) { - cancel(); - } -} - -timer::time_point_t timer::next() const { - return m_next; -} - -timer::duration_t timer::period() const { - return m_period; -} - -timer::function_t timer::function() const { - return m_callback; -} - -bool timer::null() const { - return m_callback == nullptr; -} - -bool timer::current() const { - impl::timer_manager& manager = impl::timer_manager::instance(); - - return manager.m_current_timer == this && manager.m_thread.get_id() == std::this_thread::get_id(); -} - -bool timer::detached() const { - impl::timer_manager& manager = impl::timer_manager::instance(); - - std::lock_guard lock(manager.m_detached_timers_mutex); - return m_self != manager.m_detached_timers.end(); -} - -void timer::detach() { - impl::timer_manager& manager = impl::timer_manager::instance(); - - assert(!current()); // you cannot detach a timer from within itself, because that would destroy the callback you're currently executing - assert(!detached()); // you cannot detach a timer that is already detached - std::lock_guard lock(manager.m_detached_timers_mutex); - manager.m_detached_timers.emplace_back(std::move(*this)); - --manager.m_detached_timers.back().m_self; // Is this a race condition? -} - -void timer::cancel() { - impl::timer_manager& manager = impl::timer_manager::instance(); - - if (current()) { - manager.m_active_timers.erase(this); - return; - } - - { - std::lock_guard lock(manager.m_mutex); - manager.m_active_timers.erase(this); - } - manager.m_cvar.notify_one(); -} - -// Internals - -timer::time_point_t timer::calc_next() { - return std::chrono::steady_clock::now() + m_period; -} - -} // namespace jessilib \ No newline at end of file diff --git a/src/common/timer/cancel_token.cpp b/src/common/timer/cancel_token.cpp new file mode 100644 index 0000000..af6f218 --- /dev/null +++ b/src/common/timer/cancel_token.cpp @@ -0,0 +1,75 @@ +/** + * Copyright (C) 2018 Jessica James. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY + * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION + * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + * + * Written by Jessica James + */ + +#include +#include "timer.hpp" + +namespace jessilib { + +/** cancel_token_context */ + +namespace impl { +struct cancel_token_context { + bool m_expired{ false }; + std::atomic m_weak_reference_count{ 0 }; +}; +} // namespace impl + +/** cancel_token */ + +cancel_token::cancel_token() + : m_context{ new impl::cancel_token_context } { + // Empty ctor body +}; + +cancel_token::cancel_token(cancel_token&& in_token) + : m_context{ in_token.m_context } { + in_token.m_context = nullptr; +} + +cancel_token::~cancel_token() { + if (m_context != nullptr) { + m_context->m_expired = true; + + if (m_context->m_weak_reference_count == 0) { + delete m_context; + } + } +} + +/** cancel_detector */ + +cancel_detector::cancel_detector(const cancel_token& in_token) + : m_context{ in_token.m_context } { + // Increment reference count + ++m_context->m_weak_reference_count; +} + +cancel_detector::~cancel_detector() { + // Decrement reference count + if (--m_context->m_weak_reference_count == 0 && m_context->m_expired) { + // No other references exist to the context block; delete it + delete m_context; + } +} + +bool cancel_detector::expired() const { + return m_context->m_expired; +}; + +} // namespace jessilib \ No newline at end of file diff --git a/src/common/timer/synchronized_timer.cpp b/src/common/timer/synchronized_timer.cpp new file mode 100644 index 0000000..ae4e18c --- /dev/null +++ b/src/common/timer/synchronized_timer.cpp @@ -0,0 +1,78 @@ +/** + * Copyright (C) 2018 Jessica James. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY + * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION + * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + * + * Written by Jessica James + */ + +#include +#include "timer.hpp" + +namespace jessilib { + +/** synchronized_timer */ + +class synchonrized_callback { +public: + synchonrized_callback(timer::function_t in_callback) + : m_callback{ std::move(in_callback) }, + m_calls{ 0 } { + // Empty ctor body + } + + synchonrized_callback(const synchonrized_callback& in_callback) + : m_callback{ in_callback.m_callback } { + // Empty ctor body + } + + synchonrized_callback(synchonrized_callback&& in_callback) + : m_callback{ std::move(in_callback.m_callback) } { + // Empty ctor body + } + + void operator()(timer& in_timer) { + cancel_detector detector = m_cancel_token; + // Iterate calls + if (++m_calls == 1) { + // No other calls were queued; this should be safe. + do { + m_callback(in_timer); + + // callback may have cancelled the timer (and thus destructed this callback); check + if (detector.expired()) { + // We cannot access any members of this struct outside of the stack; return immediately + return; + } + } + while (--m_calls != 0); + } + } + +private: + timer::function_t m_callback; + std::atomic m_calls{ 0 }; + cancel_token m_cancel_token; +}; + +syncrhonized_timer::syncrhonized_timer(duration_t in_period, function_t in_callback) + : timer{ in_period, synchonrized_callback{ std::move(in_callback) } } { + // Empty ctor body +} + +syncrhonized_timer::syncrhonized_timer(duration_t in_period, iterations_t in_iterations, function_t in_callback) + : timer{ in_period, in_iterations, synchonrized_callback{ std::move(in_callback) } } { + // Empty ctor body +} + +} // namespace jessilib \ No newline at end of file diff --git a/src/common/timer/timer.cpp b/src/common/timer/timer.cpp new file mode 100644 index 0000000..1ee73e8 --- /dev/null +++ b/src/common/timer/timer.cpp @@ -0,0 +1,164 @@ +/** + * Copyright (C) 2017-2018 Jessica James. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY + * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION + * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + * + * Written by Jessica James + */ + +#include "timer.hpp" +#include "assert.hpp" +#include "impl/timer_manager.hpp" +#include "impl/timer_context.hpp" + +namespace jessilib { + +/** callback_with_iterations */ + +class callback_with_iterations { +public: + callback_with_iterations(timer::iterations_t in_iterations, timer::function_t in_callback) + : m_callback{ std::move(in_callback) }, + m_iterations{ in_iterations } { + // Empty ctor body + } + + callback_with_iterations(const callback_with_iterations& in_callback) + : m_callback{ in_callback.m_callback }, + m_iterations{ in_callback.m_iterations } { + // Empty ctor body + } + + callback_with_iterations(callback_with_iterations&& in_callback) + : m_callback{ std::move(in_callback.m_callback) }, + m_iterations{ in_callback.m_iterations } { + // Empty ctor body + } + + void operator()(timer& in_timer) { + cancel_detector detector{ m_cancel_token }; + + { + std::unique_lock iterations_guard(m_iterations_mutex); + // Ensure timer is not already expired + if (m_iterations == 0) { + return; + } + + // Decrement iterations and cancel if necessary + --m_iterations; + } + + // Call callback + m_callback(in_timer); + + if (!detector.expired()) { + std::unique_lock iterations_guard(m_iterations_mutex); + if (m_iterations == 0) { + // Cancel the timer + in_timer.cancel(); + } + } + } + +private: + timer::function_t m_callback; + timer::iterations_t m_iterations; + std::mutex m_iterations_mutex; + cancel_token m_cancel_token; +}; + +/** timer */ + +timer::timer(duration_t in_period, function_t in_callback) + : m_context{ new impl::timer_context{ in_period, std::move(in_callback) } } { + // Add timer_context to timer_manager + // Note: this logic must be here (rather than timer_context) to ensure timer_context is wrapped in a shared_ptr + if jessilib_assert(in_period.count() != 0) + if jessilib_assert(!m_context->null()) { + // Add timer + impl::timer_manager& manager = impl::timer_manager::instance(); + { + std::lock_guard lock(manager.m_mutex); + manager.m_active_timers.insert(m_context.get()); + } + + // Notify + manager.m_cvar.notify_one(); + } +} + +timer::timer(duration_t in_period, iterations_t in_iterations, function_t in_callback) + : timer{ in_period, callback_with_iterations(in_iterations, std::move(in_callback)) } { + // Empty ctor body +} + +bool timer::operator==(const timer& rhs) const { + return m_context == rhs.m_context; +} + +bool timer::operator!=(const timer& rhs) const { + return !(rhs == *this); +} + +timer::time_point_t timer::next() const { + if (m_context != nullptr) { + return m_context->next(); + } + + return {}; +} + +timer::duration_t timer::period() const { + if (m_context != nullptr) { + return m_context->period(); + } + + return {}; +} + +const timer::function_t& timer::function() const { + if (m_context != nullptr) { + return m_context->function(); + } + + // No context; return nullptr + static timer::function_t s_null_function_t { nullptr }; + return s_null_function_t; +} + +bool timer::null() const { + return m_context == nullptr || m_context->null(); +} + +bool timer::detached() const { + if (m_context != nullptr) { + return m_context->detached(); + } + + return false; +} + +void timer::detach() { + if (m_context != nullptr) { + m_context->detach(); + } +} + +void timer::cancel() { + if (m_context != nullptr) { + m_context->cancel(); + } +} + +} // namespace jessilib \ No newline at end of file diff --git a/src/common/timer/timer_context.cpp b/src/common/timer/timer_context.cpp new file mode 100644 index 0000000..65247bf --- /dev/null +++ b/src/common/timer/timer_context.cpp @@ -0,0 +1,119 @@ +/** + * Copyright (C) 2018 Jessica James. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY + * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION + * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + * + * Written by Jessica James + */ + +#include "impl/timer_context.hpp" +#include "impl/timer_manager.hpp" +#include "assert.hpp" + +namespace jessilib { +namespace impl { + +timer_context::timer_context(duration_t in_period, function_t in_callback) + : m_period{ in_period }, + // m_next{ calc_next() }, + m_callback{ std::move(in_callback) }, + m_self{ timer_manager::instance().m_detached_timers.end() }, + m_last_fire_finish{ std::chrono::steady_clock::now() } { + // Empty ctor body +} + +timer_context::~timer_context() { + // All references to the context have been destroyed + if (!null()) { + cancel(); + } +} + +timer_context::time_point_t timer_context::next() const { + return m_next; +} + +timer_context::duration_t timer_context::period() const { + return m_period; +} + +const timer::function_t& timer_context::function() const { + return m_callback; +} + +bool timer_context::null() const { + return m_callback == nullptr; +} + +bool timer_context::detached() const { + impl::timer_manager& manager = impl::timer_manager::instance(); + return m_self != manager.m_detached_timers.end(); +} + +void timer_context::detach() { + if (!null()) { + impl::timer_manager& manager = impl::timer_manager::instance(); + std::list>& detached_timers = manager.m_detached_timers; + + jessilib_assert(!detached()); // you cannot detach a timer that is already detached + std::lock_guard lock(manager.m_detached_timers_mutex); + // We need to attach this new shared_ptr to the callback + m_self = detached_timers.emplace(detached_timers.end(), shared_from_this()); + } +} + +void timer_context::cancel() { + impl::timer_manager& manager = impl::timer_manager::instance(); + + // Unlock mutex if it's currently being held by this thread + bool needs_shared_lock = false; + if (manager.thread_callback_timer().m_context.get() == this) { + needs_shared_lock = true; + m_mutex.unlock_shared(); + } + + { + std::lock_guard manager_guard(manager.m_mutex); + std::lock_guard context_guard(m_mutex); + + if (!null()) { + // Remove from active timers + manager.m_active_timers.erase(this); + + // Nullify timer + m_callback = nullptr; + + // Remove from detached timers (if it's detached) + if (detached()) { + std::lock_guard detached_timers_lock(manager.m_detached_timers_mutex); + manager.m_detached_timers.erase(m_self); + } + } + + manager.is_timeout = false; + } + + // Re-lock mutex + if (needs_shared_lock) { + m_mutex.lock_shared(); + } + + manager.m_cvar.notify_one(); +} + +timer_context::time_point_t timer_context::calc_next() { + return std::chrono::steady_clock::now() + m_period; +} + +} // namespace impl +} // namespace jessilib \ No newline at end of file diff --git a/src/common/timer_manager.cpp b/src/common/timer/timer_manager.cpp similarity index 50% rename from src/common/timer_manager.cpp rename to src/common/timer/timer_manager.cpp index 5ec9dc2..16a9eeb 100644 --- a/src/common/timer_manager.cpp +++ b/src/common/timer/timer_manager.cpp @@ -1,5 +1,5 @@ /** - * Copyright (C) 2017 Jessica James. + * Copyright (C) 2017-2018 Jessica James. * * Permission to use, copy, modify, and/or distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -17,6 +17,7 @@ */ #include "impl/timer_manager.hpp" +#include "impl/timer_context.hpp" namespace jessilib { namespace impl { @@ -32,44 +33,53 @@ timer_manager::~timer_manager() { m_thread_active = false; m_cvar.notify_one(); m_thread.join(); + m_pool.join(); + m_detached_timers.clear(); } void timer_manager::loop() { // loop while (m_thread_active) { std::unique_lock lock(m_mutex); - auto itr{m_active_timers.begin()}; + auto itr{ m_active_timers.begin() }; if (itr != m_active_timers.end()) { // Wait until the next timer is ready to fire if (m_cvar.wait_until(lock, (*itr)->next()) == std::cv_status::timeout && itr != m_active_timers.end()) { - // Set active - timer* target = *itr; - m_current_timer = target; - bool detached = target->detached(); + // Due to a race condition, we may still receive timeout when another thread has notified m_cvar too late + // Notifying the thread before releasing the lock does not resolve this, because wait_until's return + // status may be based entirely on the time of return and the input time (as is the case in GCC 7.2) + if (!is_timeout) { + // itr may be invalidated; restart wait + is_timeout = true; + continue; + } - // Execute timer - target->m_callback(*target); + timer_context* context = *itr; - // Check if timer still exists - itr = m_active_timers.begin(); - if (itr != m_active_timers.end() && *itr == target) { - // Timer still exists; extract - m_active_timers.erase(itr); + // Reset timings + m_active_timers.erase(itr); + context->m_next = context->calc_next(); + m_active_timers.insert(context); - // reset timings - target->m_next = target->calc_next(); + // Push timer to execute + std::weak_ptr weak_context = context->weak_from_this(); + m_pool.push([weak_context]() { + if (auto context = weak_context.lock()) { + // Execute timer + // Locking will only fail when the timer is in the process of being cancelled + std::shared_lock context_guard{ context->m_mutex, std::try_to_lock }; + if (context_guard.owns_lock() && !context->null()) { + timer& callback_timer{thread_callback_timer()}; + callback_timer.m_context = context; - // push - m_active_timers.insert(target); - } - else if (detached) { - // Timer has been canceled; remove it from the detached timers list - std::lock_guard lock(m_detached_timers_mutex); - m_detached_timers.erase(target->m_self); - } + // Call callback + context->m_callback(callback_timer); - // Reset active - m_current_timer = nullptr; + // Release timer context + callback_timer.m_context = nullptr; + } + } + }); } // else // m_active_timers changed; itr may be invalid; itr may not be the next timer } @@ -80,5 +90,10 @@ void timer_manager::loop() { } } +timer& timer_manager::thread_callback_timer() { + static thread_local timer callback_timer; + return callback_timer; +} + } // namespace impl } // namespace jessilib diff --git a/src/include/assert.hpp b/src/include/assert.hpp new file mode 100644 index 0000000..14e6621 --- /dev/null +++ b/src/include/assert.hpp @@ -0,0 +1,68 @@ +/** + * Copyright (C) 2018 Jessica James. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY + * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION + * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + * + * Written by Jessica James + */ + +#pragma once + +#include + +/** Macros */ + +#define STRINGIFY(arg) \ + #arg +#define STRINGIFY_HELPER(line) \ + STRINGIFY(line) + +// Returns a boolean expression indicating assertion success/failure +#define jessilib_assert(expression) \ + (::jessilib::impl::_assert_helper( expression, "Failed assertion: '" #expression "' at " __FILE__ ":" STRINGIFY_HELPER(__LINE__) )) + +namespace jessilib { + +/** Exception type */ + class assertion_failed : public std::logic_error { + public: + inline explicit assertion_failed(const char* expression) + : std::logic_error{ expression } { + } + }; + +namespace impl { + +/** Macro helpers */ +inline bool _assert_helper(bool value, [[maybe_unused]] const char* message) { +#ifndef NDEBUG + if (!value) { + throw assertion_failed{message}; + } +#endif // NDEBUG + + return value; +} + +} // namespace impl +} // namespace jessilib + +// Provides for disabling of assertions; will likely produce warnings +#ifdef DISABLE_ASSERTIONS + +// Disable jessilib_assert +#undef jessilib_assert +#define jessilib_assert(expression) \ + (true) + +#endif // DISABLE_ASSERTIONS diff --git a/src/include/impl/timer_context.hpp b/src/include/impl/timer_context.hpp new file mode 100644 index 0000000..0821767 --- /dev/null +++ b/src/include/impl/timer_context.hpp @@ -0,0 +1,75 @@ +/** + * Copyright (C) 2018 Jessica James. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY + * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION + * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + * + * Written by Jessica James + */ + +#pragma once + +#include +#include +#include "timer.hpp" + +namespace jessilib { +namespace impl { + +class timer_context : public std::enable_shared_from_this { +public: + // Destructor (needs to be public for shared_ptr) + ~timer_context(); + +private: + // Types + using function_t = timer::function_t; /** Function type called by the timer */ + using time_point_t = timer::time_point_t; /** Type representing the point in time at which a timer will be called */ + using duration_t = timer::duration_t; /** Type representing the time between calls */ + + // Constructor + timer_context(duration_t in_period, function_t in_callback); + + // Explicitly delete all implicit constructors/assignment operators + timer_context() = delete; + timer_context(const timer_context&) = delete; + timer_context(timer_context&&) = delete; + timer_context& operator=(const timer_context&) = delete; + timer_context& operator=(timer_context&&) = delete; + + // Accessors + time_point_t next() const; + duration_t period() const; + const function_t& function() const; + bool null() const; + bool detached() const; + time_point_t calc_next(); + + // Mutators + void detach(); + void cancel(); + + // Members + duration_t m_period; + function_t m_callback; + time_point_t m_next; + std::list>::iterator m_self; // Necessary to cancel detached timers, and implement detached() + std::shared_mutex m_mutex; + std::chrono::steady_clock::time_point m_last_fire_finish; // Set and accessed only from timer_manager + + // Friends + friend timer; + friend timer_manager; +}; + +} +} \ No newline at end of file diff --git a/src/include/impl/timer_manager.hpp b/src/include/impl/timer_manager.hpp index 5b9b529..94e5d06 100644 --- a/src/include/impl/timer_manager.hpp +++ b/src/include/impl/timer_manager.hpp @@ -1,5 +1,5 @@ /** - * Copyright (C) 2017 Jessica James. + * Copyright (C) 2017-2018 Jessica James. * * Permission to use, copy, modify, and/or distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -25,6 +25,8 @@ #include #include #include "../timer.hpp" +#include "../thread_pool.hpp" +#include "timer_context.hpp" namespace jessilib { namespace impl { @@ -39,24 +41,30 @@ class timer_manager { ~timer_manager(); struct timer_sort { - bool operator()(const timer* lhs, const timer* rhs) const { + inline bool operator()(const timer_context* lhs, const timer_context* rhs) const { return lhs->next() < rhs->next(); } }; + class worker_thread { + public: + void operator()(); + }; + // Loop void loop(); - // helpers - bool is_current(const timer& in_timer); - bool is_detached(const timer& in_timer); + // Attempt to execute timer + //bool try_fire(timer* in_timer); + static timer& thread_callback_timer(); // Members - std::list m_detached_timers; - std::multiset m_active_timers; + thread_pool m_pool{ thread_pool::default_threads() * 2 }; + std::list> m_detached_timers; + std::multiset m_active_timers; // timer? weak_ptr? std::mutex m_mutex, m_detached_timers_mutex; std::condition_variable m_cvar; - std::atomic m_current_timer{nullptr}; + bool is_timeout{ true }; std::atomic m_thread_active{true}; std::thread m_thread{[this]() { loop(); @@ -64,6 +72,7 @@ class timer_manager { // Friends friend timer; + friend timer_context; }; // class timer_manager } // namespace impl diff --git a/src/include/thread_pool.hpp b/src/include/thread_pool.hpp index 541e87c..29fb90f 100644 --- a/src/include/thread_pool.hpp +++ b/src/include/thread_pool.hpp @@ -26,6 +26,7 @@ #include #include #include +#include namespace jessilib { @@ -45,13 +46,14 @@ public: size_t threads() const; // how many threads are in the pool size_t active() const; // how many threads are running tasks + static unsigned int default_threads(); + private: struct thread { void run_task(); - void wait(); std::atomic m_active{ false }; - std::atomic m_shutdown{ false }; + bool m_shutdown{ false }; task_t m_task; std::condition_variable m_notifier; std::mutex m_notifier_mutex; diff --git a/src/include/timer.hpp b/src/include/timer.hpp index 051dbd9..e33e0f1 100644 --- a/src/include/timer.hpp +++ b/src/include/timer.hpp @@ -1,5 +1,5 @@ /** - * Copyright (C) 2017 Jessica James. + * Copyright (C) 2017-2018 Jessica James. * * Permission to use, copy, modify, and/or distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -28,6 +28,8 @@ namespace jessilib { /** forward delcarations */ namespace impl { class timer_manager; + class timer_context; + struct cancel_token_context; } // namespace impl /** timer */ @@ -37,37 +39,29 @@ public: // Types using function_t = std::function; /** Function type called by the timer */ using time_point_t = std::chrono::steady_clock::time_point; /** Type representing the point in time at which a timer will be called */ - using duration_t = std::chrono::steady_clock::duration; - using iterations_t = size_t; - - enum class state { - active, // this timer is still active - processing, // this timer is processing right now - null // this timer is inactive - }; + using duration_t = std::chrono::steady_clock::duration; /** Type representing the time between calls */ + using iterations_t = size_t; /** Type representing iterations */ // Constructors - timer(); - timer(timer&& in_timer); + timer() = default; + timer(const timer& in_timer) = default; + timer(timer&& in_timer) = default; timer(duration_t in_period, function_t in_callback); timer(duration_t in_period, iterations_t in_iterations, function_t in_callback); - // Move operator - timer& operator=(timer&& in_timer); - - // Explicitly deleted copy constructor and assignment operator - timer(const timer&) = delete; - timer& operator=(const timer&) = delete; + // Assignment/move operators + timer& operator=(const timer&) = default; + timer& operator=(timer&& in_timer) = default; - // Destructor - ~timer(); + // Comparison operators + bool operator==(const timer& rhs) const; + bool operator!=(const timer& rhs) const; // Accessors time_point_t next() const; duration_t period() const; - function_t function() const; + const function_t& function() const; bool null() const; - bool current() const; bool detached() const; // Mutators @@ -75,19 +69,51 @@ public: void cancel(); private: - // possible states: null, active, processing, cancelled (still processing) - - // Internal helpers - time_point_t calc_next(); - - // Members - duration_t m_period; - function_t m_callback; - time_point_t m_next; - std::list::iterator m_self; + // Timer context block + std::shared_ptr m_context; // Friends friend impl::timer_manager; + friend impl::timer_context; }; // class timer +class syncrhonized_timer : public timer { +public: + // Constructors + syncrhonized_timer() = default; + syncrhonized_timer(const syncrhonized_timer& in_timer) = default; + syncrhonized_timer(syncrhonized_timer&& in_timer) = default; + syncrhonized_timer(duration_t in_period, function_t in_callback); + syncrhonized_timer(duration_t in_period, iterations_t in_iterations, function_t in_callback); + + // Assignment/move operators + syncrhonized_timer& operator=(const syncrhonized_timer&) = default; + syncrhonized_timer& operator=(syncrhonized_timer&& in_timer) = default; +}; + + +/** Useful when performing actions within a timer which may destroy the timer's callback */ + +class cancel_token { +public: + cancel_token(); + cancel_token(cancel_token&& in_token); + ~cancel_token(); + +private: + friend class cancel_detector; + impl::cancel_token_context* m_context; +}; + +class cancel_detector { +public: + cancel_detector(const cancel_token& in_token); + ~cancel_detector(); + bool expired() const; + +private: + impl::cancel_token_context* m_context; +}; + + } // namespace jessilib \ No newline at end of file diff --git a/src/test/test.hpp b/src/test/test.hpp index 9439ea4..f70fc2c 100644 --- a/src/test/test.hpp +++ b/src/test/test.hpp @@ -22,4 +22,5 @@ #include "gtest/gtest.h" // Helper macros -#define repeat( ITERATIONS ) for (size_t iteration__ = 0; iteration__ != (ITERATIONS); ++iteration__) +#define UNIQUE_LABEL( LABEL ) LABEL ## __LINE__ ## __ +#define repeat( ITERATIONS ) for (size_t UNIQUE_LABEL(iteration_) = 0; UNIQUE_LABEL(iteration_) != (ITERATIONS); ++ UNIQUE_LABEL(iteration_) ) diff --git a/src/test/thread_pool.cpp b/src/test/thread_pool.cpp index 4047481..7ed0f26 100644 --- a/src/test/thread_pool.cpp +++ b/src/test/thread_pool.cpp @@ -31,39 +31,41 @@ TEST(ThreadPoolTest, initialDefault) { EXPECT_EQ(pool.threads(), std::thread::hardware_concurrency()); std::this_thread::sleep_for(10ms); - EXPECT_EQ(pool.active(), 0); + EXPECT_EQ(pool.active(), 0U); pool.join(); - EXPECT_EQ(pool.active(), 0); - EXPECT_EQ(pool.threads(), 0); + EXPECT_EQ(pool.active(), 0U); + EXPECT_EQ(pool.threads(), 0U); } TEST(ThreadPoolTest, initialSizeDefined) { thread_pool pool{ 7 }; - EXPECT_EQ(pool.threads(), 7); + EXPECT_EQ(pool.threads(), 7U); std::this_thread::sleep_for(10ms); - EXPECT_EQ(pool.active(), 0); + EXPECT_EQ(pool.active(), 0U); pool.join(); - EXPECT_EQ(pool.active(), 0); - EXPECT_EQ(pool.threads(), 0); + EXPECT_EQ(pool.active(), 0U); + EXPECT_EQ(pool.threads(), 0U); } TEST(ThreadPoolTest, push) { - std::atomic iterations{ 0 }; - thread_pool pool; - repeat (total_iterations) { - pool.push([&iterations, &pool]() { - ++iterations; - }); - } + std::atomic iterations{0}; + thread_pool pool; - pool.join(); - EXPECT_EQ(iterations, total_iterations); + repeat (total_iterations) { + pool.push([&iterations, &pool]() { + ++iterations; + }); + } + + pool.join(); + EXPECT_EQ(iterations, total_iterations); + } } TEST(ThreadPoolTest, deadlockSingleThread) { @@ -75,8 +77,8 @@ TEST(ThreadPoolTest, deadlockSingleThread) { ++iterations; // Neither of the below should cause a deadlock - EXPECT_EQ(pool.threads(), 1); - EXPECT_EQ(pool.active(), 1); + EXPECT_EQ(pool.threads(), 1U); + EXPECT_EQ(pool.active(), 1U); }); } diff --git a/src/test/timer.cpp b/src/test/timer.cpp index 123d2be..9069c11 100644 --- a/src/test/timer.cpp +++ b/src/test/timer.cpp @@ -1,5 +1,5 @@ /** - * Copyright (C) 2017 Jessica James. + * Copyright (C) 2017-2018 Jessica James. * * Permission to use, copy, modify, and/or distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -27,18 +27,42 @@ constexpr size_t total_iterations{ 4 }; constexpr std::chrono::steady_clock::duration period = 1ms; constexpr std::chrono::steady_clock::duration timeout = period * total_iterations * 2 + 1s; +TEST(TimerTest, basic) { + timer timer_obj; + + // Check default state + EXPECT_EQ(timer_obj.next(), timer::time_point_t{}); + EXPECT_EQ(timer_obj.period(), timer::duration_t{ 0 }); + EXPECT_EQ(timer_obj.function(), nullptr); + EXPECT_TRUE(timer_obj.null()); + EXPECT_FALSE(timer_obj.detached()); + + // Should have no effect + timer_obj.detach(); + timer_obj.cancel(); + + // Verify state unchanged + EXPECT_EQ(timer_obj.next(), timer::time_point_t{}); + EXPECT_EQ(timer_obj.period(), timer::duration_t{ 0 }); + EXPECT_EQ(timer_obj.function(), nullptr); + EXPECT_TRUE(timer_obj.null()); + EXPECT_FALSE(timer_obj.detached()); +} + TEST(TimerTest, scoped) { size_t iterations{ 0 }; std::promise promise; - timer timer_obj{ period, [&iterations, &promise](timer& in_timer) { + timer timer_obj{ period, [&iterations, &promise, &timer_obj](timer& in_timer) { + EXPECT_EQ(timer_obj, in_timer); + if (++iterations == total_iterations) { - in_timer.cancel(); promise.set_value(); + in_timer.cancel(); } } }; - EXPECT_EQ(promise.get_future().wait_for(timeout), std::future_status::ready); + EXPECT_EQ(promise.get_future().wait_for(timeout * 100000), std::future_status::ready); } TEST(TimerTest, detached) { @@ -56,7 +80,7 @@ TEST(TimerTest, detached) { EXPECT_FALSE(timer_obj.null()); timer_obj.detach(); - EXPECT_TRUE(timer_obj.null()); + EXPECT_FALSE(timer_obj.null()); } EXPECT_EQ(promise.get_future().wait_for(timeout), std::future_status::ready); @@ -66,7 +90,7 @@ TEST(TimerTest, scopedWithIterations) { size_t iterations{ 0 }; std::promise promise; - timer timer_obj{ period, total_iterations, [&iterations, &promise](timer& in_timer) { + timer timer_obj{ period, total_iterations, [&iterations, &promise]([[maybe_unused]] timer& in_timer) { if (++iterations == total_iterations) { promise.set_value(); } @@ -80,14 +104,14 @@ TEST(TimerTest, detachedWithIterations) { std::promise promise; { - timer{period, total_iterations, [&iterations, &promise](timer& in_timer) { + timer{period, total_iterations, [&iterations, &promise]([[maybe_unused]] timer& in_timer) { if (++iterations == total_iterations) { promise.set_value(); } }}.detach(); } - EXPECT_EQ(promise.get_future().wait_for(timeout), std::future_status::ready); + EXPECT_EQ(promise.get_future().wait_for(timeout * 1000000), std::future_status::ready); } TEST(TimerTest, scopedWithIterationsCancel) { @@ -96,8 +120,8 @@ TEST(TimerTest, scopedWithIterationsCancel) { timer timer_obj{ period, total_iterations, [&iterations, &promise](timer& in_timer) { if (++iterations == total_iterations) { - in_timer.cancel(); promise.set_value(); + in_timer.cancel(); } } }; @@ -111,11 +135,87 @@ TEST(TimerTest, detachedWithIterationsCancel) { { timer{period, total_iterations, [&iterations, &promise](timer& in_timer) { if (++iterations == total_iterations) { - in_timer.cancel(); promise.set_value(); + in_timer.cancel(); } }}.detach(); } EXPECT_EQ(promise.get_future().wait_for(timeout), std::future_status::ready); } + +TEST(TimerTest, simultaneousTimers) { + std::condition_variable notifier; + std::mutex notifier_mutex; + int timers_running = 0; + bool test_finished = false; + + // Spin up timer + timer timer_obj{period, [¬ifier, ¬ifier_mutex, &timers_running, &test_finished]([[maybe_unused]] timer& in_timer) { + std::unique_lock lock(notifier_mutex); + if (test_finished) { + notifier.notify_all(); + return; + } + + ++timers_running; + + // Wait for a few more timer periods to pass before proceeding + notifier.wait(lock); + + // We're done here; cancel + --timers_running; + notifier.notify_all(); + }}; + + // Wait for some timers to fire + std::this_thread::sleep_for(period * 3); + EXPECT_GE(timers_running, 2); + + // Notify timers to close + { + std::unique_lock lock(notifier_mutex); + test_finished = true; + } + notifier.notify_all(); + + // Wait for timers to complete + timer_obj.cancel(); + std::unique_lock lock(notifier_mutex); + notifier.wait(lock, [&timers_running]() { + return timers_running == 0; + }); +} + +TEST(TimerTest, syncrhonizedTimers) { + std::condition_variable notifier; + std::mutex notifier_mutex; + int timers_running = 0; + + // Spin up timer + syncrhonized_timer timer_obj{period, [¬ifier, ¬ifier_mutex, &timers_running](timer& in_timer) { + std::unique_lock lock(notifier_mutex); + ++timers_running; + + // Wait for a few more timer periods to pass before proceeding + notifier.wait(lock); + + // We're done here; cancel + --timers_running; + notifier.notify_one(); + in_timer.cancel(); + }}; + + // Wait for some timers to fire + std::this_thread::sleep_for(period * 3); + EXPECT_EQ(timers_running, 1); + + // Notify timers to close + notifier.notify_all(); + + // Wait for timers to complete + std::unique_lock lock(notifier_mutex); + notifier.wait(lock, [&timers_running]() { + return timers_running == 0; + }); +}