diff --git a/src/common/timer/timer.cpp b/src/common/timer/timer.cpp index 14c07ef..66450cc 100644 --- a/src/common/timer/timer.cpp +++ b/src/common/timer/timer.cpp @@ -17,6 +17,7 @@ */ #include "timer.hpp" +#include #include "assert.hpp" #include "impl/timer_manager.hpp" #include "impl/timer_context.hpp" @@ -35,35 +36,33 @@ public: callback_with_iterations(const callback_with_iterations& in_callback) : m_callback{ in_callback.m_callback }, - m_iterations{ in_callback.m_iterations } { + m_iterations{ in_callback.m_iterations.load() } { // 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 } { + m_iterations{ in_callback.m_iterations.load() } { // Empty ctor body } void operator()(timer& in_timer) { cancel_detector detector{ m_cancel_token }; - { - std::unique_lock iterations_guard(m_iterations_mutex); + auto iterations = m_iterations.load(); + do { // Ensure timer is not already expired - if (m_iterations == 0) { + if (iterations == 0) { return; } // Decrement iterations and cancel if necessary - --m_iterations; - } + } while (!m_iterations.compare_exchange_weak(iterations, iterations - 1)); // 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(); @@ -73,8 +72,7 @@ public: private: timer::function_t m_callback; - timer::iterations_t m_iterations; - std::mutex m_iterations_mutex; + std::atomic m_iterations; cancel_token m_cancel_token; }; @@ -161,4 +159,4 @@ void timer::cancel() { } } -} // namespace jessilib \ No newline at end of file +} // namespace jessilib diff --git a/src/common/timer/timer_manager.cpp b/src/common/timer/timer_manager.cpp index 16a9eeb..5488d79 100644 --- a/src/common/timer/timer_manager.cpp +++ b/src/common/timer/timer_manager.cpp @@ -44,7 +44,9 @@ void timer_manager::loop() { 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()) { + if (m_cvar.wait_until(lock, (*itr)->next()) == std::cv_status::timeout + && itr == m_active_timers.begin() && itr != m_active_timers.end()) { + // 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)