diff --git a/src/util/Repeat.cpp b/src/util/Repeat.cpp index a5bfef7d9..118d5f1a0 100644 --- a/src/util/Repeat.cpp +++ b/src/util/Repeat.cpp @@ -27,12 +27,16 @@ Repeat::Repeat(boost::asio::io_context& ioc) : timer_(ioc) { } +Repeat::~Repeat() +{ + *stopped_ = true; +} + void Repeat::stop() { - stopping_ = true; + *stopped_ = true; timer_.cancel(); - semaphore_.acquire(); } } // namespace util diff --git a/src/util/Repeat.hpp b/src/util/Repeat.hpp index d3506f42b..56c54a503 100644 --- a/src/util/Repeat.hpp +++ b/src/util/Repeat.hpp @@ -19,6 +19,8 @@ #pragma once +#include "util/Assert.hpp" + #include #include #include @@ -26,7 +28,7 @@ #include #include #include -#include +#include namespace util { @@ -36,8 +38,7 @@ namespace util { */ class Repeat { boost::asio::steady_timer timer_; - std::atomic_bool stopping_{false}; - std::binary_semaphore semaphore_{0}; + std::shared_ptr stopped_ = std::make_shared(true); public: /** @@ -47,6 +48,11 @@ class Repeat { */ Repeat(boost::asio::io_context& ioc); + /** + * @brief Destroy the Repeat object + */ + ~Repeat(); + /** * @brief Stop repeating * @note This method will block to ensure the repeating is actually stopped. But blocking time should be very short. @@ -56,6 +62,7 @@ class Repeat { /** * @brief Start asynchronously repeating + * @note stop() must be called before start() is called for the second time * * @tparam Action The action type * @param interval The interval to repeat @@ -65,7 +72,9 @@ class Repeat { void start(std::chrono::steady_clock::duration interval, Action&& action) { - stopping_ = false; + ASSERT(*stopped_, "Repeat should be stopped before the next use"); + // Create a new variable for each start() to make each start()-stop() session independent + stopped_ = std::make_shared(false); startImpl(interval, std::forward(action)); } @@ -75,9 +84,10 @@ class Repeat { startImpl(std::chrono::steady_clock::duration interval, Action&& action) { timer_.expires_after(interval); - timer_.async_wait([this, interval, action = std::forward(action)](auto const&) mutable { - if (stopping_) { - semaphore_.release(); + timer_.async_wait([this, interval, stopping = stopped_, action = std::forward(action)]( + auto const& errorCode + ) mutable { + if (errorCode or *stopping) { return; } action(); diff --git a/tests/unit/util/RepeatTests.cpp b/tests/unit/util/RepeatTests.cpp index 69a6bf8cb..460ce1851 100644 --- a/tests/unit/util/RepeatTests.cpp +++ b/tests/unit/util/RepeatTests.cpp @@ -34,7 +34,7 @@ using namespace util; using testing::AtLeast; -struct RepeatTests : SyncAsioContextTest { +struct RepeatTest : SyncAsioContextTest { Repeat repeat{ctx}; testing::StrictMock> handlerMock; @@ -51,14 +51,14 @@ struct RepeatTests : SyncAsioContextTest { } }; -TEST_F(RepeatTests, CallsHandler) +TEST_F(RepeatTest, CallsHandler) { repeat.start(std::chrono::milliseconds{1}, handlerMock.AsStdFunction()); EXPECT_CALL(handlerMock, Call).Times(AtLeast(10)); runContextFor(std::chrono::milliseconds{20}); } -TEST_F(RepeatTests, StopsOnStop) +TEST_F(RepeatTest, StopsOnStop) { withRunningContext([this]() { repeat.start(std::chrono::milliseconds{1}, handlerMock.AsStdFunction()); @@ -68,10 +68,10 @@ TEST_F(RepeatTests, StopsOnStop) }); } -TEST_F(RepeatTests, RunsAfterStop) +TEST_F(RepeatTest, RunsAfterStop) { withRunningContext([this]() { - for ([[maybe_unused]] auto _ : std::ranges::iota_view(0, 2)) { + for ([[maybe_unused]] auto i : std::ranges::iota_view(0, 2)) { repeat.start(std::chrono::milliseconds{1}, handlerMock.AsStdFunction()); EXPECT_CALL(handlerMock, Call).Times(AtLeast(1)); std::this_thread::sleep_for(std::chrono::milliseconds{10}); @@ -79,3 +79,16 @@ TEST_F(RepeatTests, RunsAfterStop) } }); } + +struct RepeatDeathTest : RepeatTest {}; + +TEST_F(RepeatDeathTest, DiesWhenStartCalledTwice) +{ + EXPECT_DEATH( + { + repeat.start(std::chrono::seconds{1}, []() {}); + repeat.start(std::chrono::seconds{1}, []() {}); + }, + "Assertion .* failed.*" + ); +}