diff --git a/src/common/include/display_device/retry_scheduler.h b/src/common/include/display_device/retry_scheduler.h index aecab81..106d991 100644 --- a/src/common/include/display_device/retry_scheduler.h +++ b/src/common/include/display_device/retry_scheduler.h @@ -30,6 +30,17 @@ namespace display_device { */ explicit SchedulerStopToken(std::function cleanup); + /** + * @brief Deleted copy constructor. + */ + SchedulerStopToken(const SchedulerStopToken &) = delete; + + /** + * @brief Deleted copy operator. + */ + SchedulerStopToken & + operator=(const SchedulerStopToken &) = delete; + /** * @brief Executes cleanup logic if scheduler stop was requested. */ @@ -64,39 +75,41 @@ namespace display_device { }; /** - * @brief Check if the function signature matches the acceptable signature for RetryScheduler::execute - * without a stop token. + * @brief A convenience template struct helper for adding const to the type. */ - template - concept ExecuteWithoutStopToken = requires(FunctionT exec_fn) { - exec_fn(std::declval()); + template + struct AutoConst; + + template + struct AutoConst { + using type = T; + }; + + template + struct AutoConst { + using type = std::add_const_t; }; /** - * @brief Check if the function signature matches the acceptable signature for RetryScheduler::execute (const) - * without a stop token. + * @brief A convenience template helper for adding const to the type. */ - template - concept ExecuteWithoutStopTokenConst = requires(FunctionT exec_fn) { - exec_fn(std::declval()); - }; + template + using auto_const_t = typename AutoConst::type; /** * @brief Check if the function signature matches the acceptable signature for RetryScheduler::execute - * with a stop token. + * without a stop token. */ template - concept ExecuteWithStopToken = requires(FunctionT exec_fn) { - exec_fn(std::declval(), std::declval()); - }; + concept ExecuteWithoutStopToken = requires(FunctionT exec_fn, T &value) { exec_fn(value); }; /** - * @brief Check if the function signature matches the acceptable signature for RetryScheduler::execute (const) + * @brief Check if the function signature matches the acceptable signature for RetryScheduler::execute * with a stop token. */ template - concept ExecuteWithStopTokenConst = requires(FunctionT exec_fn) { - exec_fn(std::declval(), std::declval()); + concept ExecuteWithStopToken = requires(FunctionT exec_fn, T &value, SchedulerStopToken &token) { + exec_fn(value, token); }; /** @@ -104,12 +117,6 @@ namespace display_device { */ template concept ExecuteCallbackLike = ExecuteWithoutStopToken || ExecuteWithStopToken; - - /** - * @brief Check if the function signature matches the acceptable signature for RetryScheduler::execute (const). - */ - template - concept ExecuteCallbackLikeConst = ExecuteWithoutStopTokenConst || ExecuteWithStopTokenConst; } // namespace detail /** @@ -250,8 +257,58 @@ namespace display_device { } } + /** + * @brief A non-const variant of the `executeImpl` method. See it for details. + */ + template + auto + execute(FunctionT &&exec_fn) { + return executeImpl(*this, std::forward(exec_fn)); + } + + /** + * @brief A const variant of the `executeImpl` method. See it for details. + */ + template + auto + execute(FunctionT &&exec_fn) const { + return executeImpl(*this, std::forward(exec_fn)); + } + + /** + * @brief Check whether anything is scheduled for execution. + * @return True if something is scheduled, false otherwise. + */ + [[nodiscard]] bool + isScheduled() const { + return static_cast(m_retry_function); + } + + /** + * @brief Stop the scheduled function - will no longer be execute once THIS method returns. + */ + void + stop() { + std::lock_guard lock { m_mutex }; + stopUnlocked(); + } + + private: + static std::chrono::milliseconds + takeNextDuration(std::vector &durations) { + if (durations.size() > 1) { + const auto front_it { std::begin(durations) }; + const auto front_value { *front_it }; + durations.erase(front_it); + return front_value; + } + + return durations.empty() ? std::chrono::milliseconds::zero() : durations.back(); + } + /** * @brief Execute arbitrary logic using the provided interface in a thread-safe manner. + * @param self A reference to *this. * @param exec_fn Provides thread-safe access to the interface for executing arbitrary logic. * Acceptable function signatures are: * - AnyReturnType(T &); @@ -259,6 +316,7 @@ namespace display_device { * `stop_token` is an optional parameter that allows to stop scheduler during * the same call. * @return Return value from the executor callback. + * @note This method is not to be used directly. Intead the `execute` method is to be used. * @examples * std::unique_ptr iface = getIface(...); * RetryScheduler scheduler{std::move(iface)}; @@ -284,65 +342,32 @@ namespace display_device { * }); * @examples_end */ - template - requires detail::ExecuteCallbackLike - auto - execute(FunctionT exec_fn) { + static auto + executeImpl(auto &self, auto &&exec_fn) + requires detail::ExecuteCallbackLike + { + using FunctionT = decltype(exec_fn); + constexpr bool IsConst = std::is_const_v>; + if constexpr (detail::OptionalFunction) { if (!exec_fn) { throw std::logic_error { "Empty callback function provided in RetryScheduler::execute!" }; } } - std::lock_guard lock { m_mutex }; + std::lock_guard lock { self.m_mutex }; + detail::auto_const_t, IsConst> &iface_ref { *self.m_iface }; if constexpr (detail::ExecuteWithStopToken) { - SchedulerStopToken stop_token { [&]() { stopUnlocked(); } }; - return exec_fn(*m_iface, stop_token); + detail::auto_const_t stop_token { [&self]() { + if constexpr (!IsConst) { + self.stopUnlocked(); + } + } }; + return std::forward(exec_fn)(iface_ref, stop_token); } else { - return exec_fn(*m_iface); - } - } - - /** - * @brief A const variant of the `execute` method. See non-const variant for details. - */ - template - requires detail::ExecuteCallbackLikeConst - auto - execute(FunctionT exec_fn) const { - return const_cast(this)->execute(std::move(exec_fn)); - } - - /** - * @brief Check whether anything is scheduled for execution. - * @return True if something is scheduled, false otherwise. - */ - [[nodiscard]] bool - isScheduled() const { - return static_cast(m_retry_function); - } - - /** - * @brief Stop the scheduled function - will no longer be execute once THIS method returns. - */ - void - stop() { - std::lock_guard lock { m_mutex }; - stopUnlocked(); - } - - private: - static std::chrono::milliseconds - takeNextDuration(std::vector &durations) { - if (durations.size() > 1) { - const auto front_it { std::begin(durations) }; - const auto front_value { *front_it }; - durations.erase(front_it); - return front_value; + return std::forward(exec_fn)(iface_ref); } - - return durations.empty() ? std::chrono::milliseconds::zero() : durations.back(); } /** @@ -378,7 +403,7 @@ namespace display_device { std::vector m_sleep_durations; /**< Sleep times for the timer. */ std::function m_retry_function { nullptr }; /**< Function to be executed until it succeeds. */ - std::mutex m_mutex {}; /**< A mutex for synchronizing thread and "external" access. */ + mutable std::mutex m_mutex {}; /**< A mutex for synchronizing thread and "external" access. */ std::condition_variable m_sleep_cv {}; /**< Condition variable for waking up thread. */ bool m_syncing_thread { false }; /**< Safeguard for the condition variable to prevent sporadic thread wake-ups. */ bool m_keep_alive { true }; /**< When set to false, scheduler thread will exit. */ diff --git a/tests/unit/general/test_retry_scheduler.cpp b/tests/unit/general/test_retry_scheduler.cpp index 4840b2b..d002835 100644 --- a/tests/unit/general/test_retry_scheduler.cpp +++ b/tests/unit/general/test_retry_scheduler.cpp @@ -14,27 +14,12 @@ namespace { // A dummy struct for the tests struct TestIface { std::vector m_durations; - }; - - template - constexpr bool - isValidNonConstCallback(FunctionT) { - using namespace display_device::detail; - if constexpr (ExecuteCallbackLike) { - return true; - } - return false; - } - template - constexpr bool - isValidConstCallback(FunctionT) { - using namespace display_device::detail; - if constexpr (ExecuteCallbackLikeConst) { - return true; - } - return false; - } + void + nonConstMethod() { /* noop */ } + void + constMethod() const { /* noop */ } + }; // Test fixture(s) for this file class RetrySchedulerTest: public BaseTest { @@ -57,12 +42,12 @@ TEST_F_S(Schedule, NullptrCallbackProvided) { } TEST_F_S(Schedule, NoDurations) { - EXPECT_THAT([&]() { m_impl.schedule([](auto, auto) {}, { .m_sleep_durations = {} }); }, + EXPECT_THAT([&]() { m_impl.schedule([](auto, auto&) {}, { .m_sleep_durations = {} }); }, ThrowsMessage(HasSubstr("At least 1 sleep duration must be specified in RetryScheduler::schedule!"))); } TEST_F_S(Schedule, ZeroDuration) { - EXPECT_THAT([&]() { m_impl.schedule([](auto, auto) {}, { .m_sleep_durations = { 0ms } }); }, + EXPECT_THAT([&]() { m_impl.schedule([](auto, auto&) {}, { .m_sleep_durations = { 0ms } }); }, ThrowsMessage(HasSubstr("All of the durations specified in RetryScheduler::schedule must be larger than a 0!"))); } @@ -107,7 +92,7 @@ TEST_F_S(Schedule, SchedulingDurations) { TEST_F_S(Schedule, SchedulerInteruptAndReplacement) { int counter_a { 0 }; - m_impl.schedule([&counter_a](auto, auto) { counter_a++; }, { .m_sleep_durations = { 5ms } }); + m_impl.schedule([&counter_a](auto, auto&) { counter_a++; }, { .m_sleep_durations = { 5ms } }); while (counter_a < 3) { std::this_thread::sleep_for(1ms); @@ -115,7 +100,7 @@ TEST_F_S(Schedule, SchedulerInteruptAndReplacement) { int counter_a_last_value { 0 }; int counter_b { 0 }; - m_impl.schedule([&counter_a_last_value, &counter_a, &counter_b](auto, auto) { + m_impl.schedule([&counter_a_last_value, &counter_a, &counter_b](auto, auto&) { std::this_thread::sleep_for(15ms); counter_a_last_value = counter_a; counter_b++; @@ -272,7 +257,7 @@ TEST_F_S(Schedule, ExceptionThrown, DuringImmediateCall) { auto &logger { display_device::Logger::get() }; int counter_a { 0 }; - m_impl.schedule([&](auto, auto) { counter_a++; }, { .m_sleep_durations = { 1ms } }); + m_impl.schedule([&](auto, auto&) { counter_a++; }, { .m_sleep_durations = { 1ms } }); while (counter_a < 3) { std::this_thread::sleep_for(1ms); } @@ -283,7 +268,7 @@ TEST_F_S(Schedule, ExceptionThrown, DuringImmediateCall) { }); EXPECT_TRUE(m_impl.isScheduled()); - m_impl.schedule([&](auto, auto) { + m_impl.schedule([&](auto, auto&) { throw std::runtime_error("Get rekt!"); }, { .m_sleep_durations = { 1ms } }); @@ -292,7 +277,7 @@ TEST_F_S(Schedule, ExceptionThrown, DuringImmediateCall) { // Verify that scheduler still works int counter_b { 0 }; - m_impl.schedule([&](auto, auto) { counter_b++; }, { .m_sleep_durations = { 1ms } }); + m_impl.schedule([&](auto, auto&) { counter_b++; }, { .m_sleep_durations = { 1ms } }); while (counter_b < 3) { std::this_thread::sleep_for(1ms); } @@ -311,7 +296,7 @@ TEST_F_S(Schedule, ExceptionThrown, DuringScheduledCall) { bool first_call { true }; EXPECT_EQ(output, ""); - m_impl.schedule([&](auto, auto) { + m_impl.schedule([&](auto, auto&) { if (!first_call) { throw std::runtime_error("Get rekt!"); } @@ -326,7 +311,7 @@ TEST_F_S(Schedule, ExceptionThrown, DuringScheduledCall) { // Verify that scheduler still works int counter { 0 }; - m_impl.schedule([&](auto, auto) { counter++; }, { .m_sleep_durations = { 1ms } }); + m_impl.schedule([&](auto, auto&) { counter++; }, { .m_sleep_durations = { 1ms } }); while (counter < 3) { std::this_thread::sleep_for(1ms); } @@ -348,14 +333,14 @@ TEST_F_S(Execute, Const, NullptrCallbackProvided) { TEST_F_S(Execute, SchedulerNotStopped) { int counter { 0 }; - m_impl.schedule([&](auto, auto) { counter++; }, { .m_sleep_durations = { 1ms } }); + m_impl.schedule([&](auto, auto&) { counter++; }, { .m_sleep_durations = { 1ms } }); while (counter < 3) { std::this_thread::sleep_for(1ms); } int counter_before_sleep { 0 }; int counter_after_sleep { 0 }; - m_impl.execute([&](auto, auto) { + m_impl.execute([&](auto, auto&) { counter_before_sleep = counter; std::this_thread::sleep_for(15ms); counter_after_sleep = counter; @@ -374,14 +359,14 @@ TEST_F_S(Execute, SchedulerNotStopped) { TEST_F_S(Execute, SchedulerStopped) { int counter { 0 }; - m_impl.schedule([&](auto, auto) { counter++; }, { .m_sleep_durations = { 1ms } }); + m_impl.schedule([&](auto, auto&) { counter++; }, { .m_sleep_durations = { 1ms } }); while (counter < 3) { std::this_thread::sleep_for(1ms); } int counter_before_sleep { 0 }; int counter_after_sleep { 0 }; - m_impl.execute([&](auto, auto stop_token) { + m_impl.execute([&](auto, auto& stop_token) { counter_before_sleep = counter; std::this_thread::sleep_for(15ms); counter_after_sleep = counter; @@ -395,11 +380,11 @@ TEST_F_S(Execute, SchedulerStopped) { TEST_F_S(Execute, SchedulerStopped, ExceptThatItWasNotRunning) { EXPECT_FALSE(m_impl.isScheduled()); - m_impl.execute([](auto, auto stop_token) { stop_token.requestStop(); }); + m_impl.execute([](auto, auto& stop_token) { stop_token.requestStop(); }); EXPECT_FALSE(m_impl.isScheduled()); int counter { 0 }; - m_impl.schedule([&](auto, auto) { counter++; }, { .m_sleep_durations = { 1ms } }); + m_impl.schedule([&](auto, auto&) { counter++; }, { .m_sleep_durations = { 1ms } }); while (counter < 3) { std::this_thread::sleep_for(1ms); } @@ -410,7 +395,7 @@ TEST_F_S(Execute, SchedulerStopped, ExceptThatItWasNotRunning) { TEST_F_S(Execute, ExceptionThrown) { int counter { 0 }; - m_impl.schedule([&](auto, auto) { counter++; }, { .m_sleep_durations = { 1ms } }); + m_impl.schedule([&](auto, auto&) { counter++; }, { .m_sleep_durations = { 1ms } }); while (counter < 3) { std::this_thread::sleep_for(1ms); } @@ -428,12 +413,12 @@ TEST_F_S(Execute, ExceptionThrown) { TEST_F_S(Execute, ExceptionThrown, BeforeStopToken) { int counter { 0 }; - m_impl.schedule([&](auto, auto) { counter++; }, { .m_sleep_durations = { 1ms } }); + m_impl.schedule([&](auto, auto&) { counter++; }, { .m_sleep_durations = { 1ms } }); while (counter < 3) { std::this_thread::sleep_for(1ms); } - EXPECT_THAT([&]() { m_impl.execute([](auto, auto stop_token) { + EXPECT_THAT([&]() { m_impl.execute([](auto, auto& stop_token) { throw std::runtime_error("Get rekt!"); stop_token.requestStop(); }); }, @@ -447,12 +432,12 @@ TEST_F_S(Execute, ExceptionThrown, BeforeStopToken) { TEST_F_S(Execute, ExceptionThrown, AfterStopToken) { int counter { 0 }; - m_impl.schedule([&](auto, auto) { counter++; }, { .m_sleep_durations = { 1ms } }); + m_impl.schedule([&](auto, auto&) { counter++; }, { .m_sleep_durations = { 1ms } }); while (counter < 3) { std::this_thread::sleep_for(1ms); } - EXPECT_THAT([&]() { m_impl.execute([](auto, auto stop_token) { + EXPECT_THAT([&]() { m_impl.execute([](auto, auto& stop_token) { stop_token.requestStop(); throw std::runtime_error("Get rekt!"); }); }, @@ -462,40 +447,35 @@ TEST_F_S(Execute, ExceptionThrown, AfterStopToken) { } TEST_F_S(Execute, ConstVsNonConst, WithoutStopToken) { - const auto const_callback = [](const TestIface &) { /* noop */ }; - const auto non_const_callback = [](TestIface &) { /* noop */ }; - - // Verify that concepts are working as expected - static_assert(isValidNonConstCallback(const_callback), "Invalid non-const callback"); - static_assert(isValidNonConstCallback(non_const_callback), "Invalid non-const callback"); - static_assert(isValidConstCallback(const_callback), "Invalid const callback"); - static_assert(!isValidConstCallback(non_const_callback), "Invalid const callback"); + const auto const_callback = [](const TestIface& iface) { iface.constMethod(); }; + const auto non_const_callback = [](TestIface& iface) { iface.nonConstMethod(); }; + const auto const_callback_auto = [](const auto& iface) { iface.constMethod(); }; + const auto non_const_callback_auto = [](auto& iface) { iface.nonConstMethod(); }; // Verify it compiles with non-const auto &non_const_impl { m_impl }; non_const_impl.execute(const_callback); non_const_impl.execute(non_const_callback); + non_const_impl.execute(const_callback_auto); + non_const_impl.execute(non_const_callback_auto); - // Verify it compiles with const + // Verify it compiles with const (commented out code will not compile) const auto &const_impl { m_impl }; const_impl.execute(const_callback); + // const_impl.execute(non_const_callback); + const_impl.execute(const_callback_auto); + // const_impl.execute(non_const_callback_auto); } TEST_F_S(Execute, ConstVsNonConst, WithStopToken) { - const auto const_const_callback = [](const TestIface &, const display_device::SchedulerStopToken &) { /* noop */ }; - const auto const_non_const_callback = [](const TestIface &, display_device::SchedulerStopToken &) { /* noop */ }; - const auto non_const_const_callback = [](TestIface &, const display_device::SchedulerStopToken &) { /* noop */ }; - const auto non_const_non_const_callback = [](TestIface &, display_device::SchedulerStopToken &) { /* noop */ }; - - // Verify that concepts are working as expected - static_assert(isValidNonConstCallback(const_const_callback), "Invalid non-const callback"); - static_assert(isValidNonConstCallback(const_non_const_callback), "Invalid non-const callback"); - static_assert(isValidNonConstCallback(non_const_const_callback), "Invalid non-const callback"); - static_assert(isValidNonConstCallback(non_const_non_const_callback), "Invalid non-const callback"); - static_assert(isValidConstCallback(const_const_callback), "Invalid const callback"); - static_assert(!isValidConstCallback(const_non_const_callback), "Invalid const callback"); - static_assert(!isValidConstCallback(non_const_const_callback), "Invalid const callback"); - static_assert(!isValidConstCallback(non_const_non_const_callback), "Invalid const callback"); + const auto const_const_callback = [](const TestIface &iface, const display_device::SchedulerStopToken &token) { iface.constMethod(); (void)token.stopRequested(); }; + const auto const_non_const_callback = [](const TestIface &iface, display_device::SchedulerStopToken &token) { iface.constMethod(); token.requestStop(); }; + const auto non_const_const_callback = [](TestIface &iface, const display_device::SchedulerStopToken &token) { iface.nonConstMethod(); (void)token.stopRequested(); }; + const auto non_const_non_const_callback = [](TestIface &iface, display_device::SchedulerStopToken &token) { iface.nonConstMethod(); token.requestStop(); }; + const auto const_const_callback_auto = [](const auto &iface, const auto &token) { iface.constMethod(); (void)token.stopRequested(); }; + const auto const_non_const_callback_auto = [](const auto &iface, auto &token) { iface.constMethod(); token.requestStop(); }; + const auto non_const_const_callback_auto = [](auto &iface, const auto &token) { iface.nonConstMethod(); (void)token.stopRequested(); }; + const auto non_const_non_const_callback_auto = [](auto &iface, auto &token) { iface.nonConstMethod(); token.requestStop(); }; // Verify it compiles with non-const auto &non_const_impl { m_impl }; @@ -503,10 +483,21 @@ TEST_F_S(Execute, ConstVsNonConst, WithStopToken) { non_const_impl.execute(const_non_const_callback); non_const_impl.execute(non_const_const_callback); non_const_impl.execute(non_const_non_const_callback); + non_const_impl.execute(const_const_callback_auto); + non_const_impl.execute(const_non_const_callback_auto); + non_const_impl.execute(non_const_const_callback_auto); + non_const_impl.execute(non_const_non_const_callback_auto); - // Verify it compiles with const + // Verify it compiles with const (commented out code will not compile) const auto &const_impl { m_impl }; const_impl.execute(const_const_callback); + // const_impl.execute(const_non_const_callback); + // const_impl.execute(non_const_const_callback); + // const_impl.execute(non_const_non_const_callback); + const_impl.execute(const_const_callback_auto); + // const_impl.execute(const_non_const_callback_auto); + // const_impl.execute(non_const_const_callback_auto); + // const_impl.execute(non_const_non_const_callback_auto); } TEST_F_S(Stop) { @@ -515,7 +506,7 @@ TEST_F_S(Stop) { EXPECT_FALSE(m_impl.isScheduled()); int counter { 0 }; - m_impl.schedule([&](auto, auto) { counter++; }, { .m_sleep_durations = { 1ms } }); + m_impl.schedule([&](auto, auto&) { counter++; }, { .m_sleep_durations = { 1ms } }); while (counter < 3) { std::this_thread::sleep_for(1ms); } @@ -530,7 +521,7 @@ TEST_F_S(ThreadCleanupInDestructor) { { display_device::RetryScheduler scheduler { std::make_unique() }; - scheduler.schedule([&](auto, auto) { counter++; }, { .m_sleep_durations = { 1ms } }); + scheduler.schedule([&](auto, auto&) { counter++; }, { .m_sleep_durations = { 1ms } }); while (counter < 3) { std::this_thread::sleep_for(1ms); }