utility/timer: Implement heap based userdata
Change-Id: I38b127e4fb147f207f32ff44a9f8d82ee7866ba9
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/src/sdeventplus/utility/timer.cpp b/src/sdeventplus/utility/timer.cpp
index d078b06..5c1d16a 100644
--- a/src/sdeventplus/utility/timer.cpp
+++ b/src/sdeventplus/utility/timer.cpp
@@ -1,5 +1,6 @@
-#include <functional>
+#include <memory>
#include <sdeventplus/clock.hpp>
+#include <sdeventplus/types.hpp>
#include <sdeventplus/utility/timer.hpp>
#include <stdexcept>
#include <utility>
@@ -10,76 +11,37 @@
{
template <ClockId Id>
-Timer<Id>::Timer(Timer&& other) :
- expired(std::move(other.expired)),
- initialized(std::move(other.initialized)),
- callback(std::move(other.callback)), clock(std::move(other.clock)),
- interval(std::move(other.interval)), timeSource(std::move(other.timeSource))
-{
- timeSource.set_callback(std::bind(&Timer::internalCallback, this,
- std::placeholders::_1,
- std::placeholders::_2));
-}
-
-template <ClockId Id>
-Timer<Id>& Timer<Id>::operator=(Timer&& other)
-{
- if (this != &other)
- {
- try
- {
- timeSource.set_enabled(source::Enabled::Off);
- }
- catch (std::bad_optional_access&)
- {
- // This is normal for a moved object
- }
-
- expired = std::move(other.expired);
- initialized = std::move(other.initialized);
- callback = std::move(other.callback);
- clock = std::move(other.clock);
- interval = std::move(other.interval);
- timeSource = std::move(other.timeSource);
- timeSource.set_callback(std::bind(&Timer::internalCallback, this,
- std::placeholders::_1,
- std::placeholders::_2));
- }
- return *this;
-}
-
-template <ClockId Id>
Timer<Id>::Timer(const Event& event, Callback&& callback,
std::optional<Duration> interval,
typename source::Time<Id>::Accuracy accuracy) :
- expired(false),
- initialized(interval.has_value()), callback(std::move(callback)),
- clock(event), interval(interval),
- timeSource(event, clock.now() + interval.value_or(Duration::zero()),
- accuracy,
- std::bind(&Timer::internalCallback, this, std::placeholders::_1,
- std::placeholders::_2))
+ userdata(nullptr),
+ timeSource(event,
+ Clock<Id>(event).now() + interval.value_or(Duration::zero()),
+ accuracy, nullptr)
{
+ auto timerData = std::make_unique<detail::TimerData<Id>>(
+ *this, std::move(callback), interval);
+ userdata = timerData.get();
+ timerData->userdata = timerData.get();
+ auto cb = [timerData = std::move(timerData)](
+ source::Time<Id>&, typename source::Time<Id>::TimePoint) {
+ timerData->internalCallback();
+ };
+ timeSource.set_callback(std::move(cb));
setEnabled(interval.has_value());
}
template <ClockId Id>
-Timer<Id>::~Timer()
+Timer<Id>::Timer(const Timer<Id>& other, sdeventplus::internal::NoOwn) :
+ userdata(other.userdata),
+ timeSource(other.timeSource, sdeventplus::internal::NoOwn())
{
- try
- {
- timeSource.set_enabled(source::Enabled::Off);
- }
- catch (std::bad_optional_access&)
- {
- // This is normal for a moved object
- }
}
template <ClockId Id>
void Timer<Id>::set_callback(Callback&& callback)
{
- this->callback = std::move(callback);
+ userdata->callback = std::move(callback);
}
template <ClockId Id>
@@ -91,7 +53,7 @@
template <ClockId Id>
bool Timer<Id>::hasExpired() const
{
- return expired;
+ return userdata->expired;
}
template <ClockId Id>
@@ -103,7 +65,7 @@
template <ClockId Id>
std::optional<typename Timer<Id>::Duration> Timer<Id>::getInterval() const
{
- return interval;
+ return userdata->interval;
}
template <ClockId Id>
@@ -115,7 +77,7 @@
}
auto end = timeSource.get_time();
- auto now = clock.now();
+ auto now = userdata->clock.now();
if (end < now)
{
return Duration{0};
@@ -126,7 +88,7 @@
template <ClockId Id>
void Timer<Id>::setEnabled(bool enabled)
{
- if (enabled && !initialized)
+ if (enabled && !userdata->initialized)
{
throw std::runtime_error("Timer was never initialized");
}
@@ -137,58 +99,57 @@
template <ClockId Id>
void Timer<Id>::setRemaining(Duration remaining)
{
- timeSource.set_time(clock.now() + remaining);
- initialized = true;
+ timeSource.set_time(userdata->clock.now() + remaining);
+ userdata->initialized = true;
}
template <ClockId Id>
void Timer<Id>::resetRemaining()
{
- setRemaining(interval.value());
+ setRemaining(userdata->interval.value());
}
template <ClockId Id>
void Timer<Id>::setInterval(std::optional<Duration> interval)
{
- this->interval = interval;
+ userdata->interval = interval;
}
template <ClockId Id>
void Timer<Id>::clearExpired()
{
- expired = false;
+ userdata->expired = false;
}
template <ClockId Id>
void Timer<Id>::restart(std::optional<Duration> interval)
{
clearExpired();
- initialized = false;
+ userdata->initialized = false;
setInterval(interval);
- if (interval)
+ if (userdata->interval)
{
resetRemaining();
}
- setEnabled(interval.has_value());
+ setEnabled(userdata->interval.has_value());
}
template <ClockId Id>
void Timer<Id>::restartOnce(Duration remaining)
{
clearExpired();
- initialized = false;
+ userdata->initialized = false;
setInterval(std::nullopt);
setRemaining(remaining);
setEnabled(true);
}
template <ClockId Id>
-void Timer<Id>::internalCallback(source::Time<Id>&,
- typename source::Time<Id>::TimePoint)
+void Timer<Id>::internalCallback()
{
- expired = true;
- initialized = false;
- if (interval)
+ userdata->expired = true;
+ userdata->initialized = false;
+ if (userdata->interval)
{
resetRemaining();
}
@@ -197,9 +158,9 @@
setEnabled(false);
}
- if (callback)
+ if (userdata->callback)
{
- callback(*this);
+ userdata->callback(*userdata);
}
}
@@ -209,5 +170,22 @@
template class Timer<ClockId::RealTimeAlarm>;
template class Timer<ClockId::BootTimeAlarm>;
+namespace detail
+{
+
+template <ClockId Id>
+TimerData<Id>::TimerData(const Timer<Id>& base,
+ typename Timer<Id>::Callback&& callback,
+ std::optional<typename Timer<Id>::Duration> interval) :
+ Timer<Id>(base, sdeventplus::internal::NoOwn()),
+ expired(false), initialized(interval.has_value()),
+ callback(std::move(callback)),
+ clock(Event(base.timeSource.get_event(), sdeventplus::internal::NoOwn())),
+ interval(interval)
+{
+}
+
+} // namespace detail
+
} // namespace utility
} // namespace sdeventplus
diff --git a/src/sdeventplus/utility/timer.hpp b/src/sdeventplus/utility/timer.hpp
index 1453dbe..b29d41d 100644
--- a/src/sdeventplus/utility/timer.hpp
+++ b/src/sdeventplus/utility/timer.hpp
@@ -6,12 +6,19 @@
#include <sdeventplus/clock.hpp>
#include <sdeventplus/event.hpp>
#include <sdeventplus/source/time.hpp>
+#include <sdeventplus/types.hpp>
namespace sdeventplus
{
namespace utility
{
+namespace detail
+{
+template <ClockId Id>
+class TimerData;
+} // namespace detail
+
/** @class Timer<Id>
* @brief A simple, repeating timer around an sd_event time source
* @details Adds a timer to the SdEvent loop that runs a user defined callback
@@ -37,12 +44,6 @@
*/
using Callback = fu2::unique_function<void(Timer<Id>&)>;
- Timer(const Timer& other) = delete;
- Timer(Timer&& other);
- Timer& operator=(const Timer& other) = delete;
- Timer& operator=(Timer&& other);
- virtual ~Timer();
-
/** @brief Creates a new timer on the given event loop.
* This timer is created enabled by default if passed an interval.
*
@@ -61,6 +62,17 @@
typename source::Time<Id>::Accuracy accuracy =
std::chrono::milliseconds{1});
+ /** @brief Constructs a new non-owning Timer from an existing timer
+ * Does not take a reference on the passed in timer
+ * Does not release the reference it is given
+ * NOTE: This will still take a reference during future copies
+ * @internal
+ *
+ * @param[in] other - The other Timer to copy
+ * @param[in] - Denotes no reference taken or release
+ */
+ Timer(const Timer& timer, sdeventplus::internal::NoOwn);
+
/** @brief Sets the callback
*
* @param[in] callback - The function executed on timer elapse
@@ -156,26 +168,47 @@
*/
void restartOnce(Duration remaining);
- private:
- /** @brief Tracks the expiration status of the timer */
- bool expired;
- /** @brief Tracks whether or not the expiration timeout is valid */
- bool initialized;
- /** @brief User defined callback run on each expiration */
- Callback callback;
- /** @brief Clock used for updating the time source */
- Clock<Id> clock;
- /** @brief Interval between each timer expiration */
- std::optional<Duration> interval;
+ protected:
+ /** @brief Reference to the heap allocated Timer.
+ * Lifetime and ownership is managed by the timeSource
+ */
+ detail::TimerData<Id>* userdata;
/** @brief Underlying sd_event time source that backs the timer */
source::Time<Id> timeSource;
/** @brief Used as a helper to run our user defined callback on the
* timeSource
*/
- void internalCallback(source::Time<Id>&,
- typename source::Time<Id>::TimePoint);
+ void internalCallback();
+
+ friend detail::TimerData<Id>;
};
+namespace detail
+{
+
+template <ClockId Id>
+class TimerData : public Timer<Id>
+{
+ private:
+ /** @brief Tracks the expiration status of the timer */
+ bool expired;
+ /** @brief Tracks whether or not the expiration timeout is valid */
+ bool initialized;
+ /** @brief User defined callback run on each expiration */
+ typename Timer<Id>::Callback callback;
+ /** @brief Clock used for updating the time source */
+ Clock<Id> clock;
+ /** @brief Interval between each timer expiration */
+ std::optional<typename Timer<Id>::Duration> interval;
+
+ public:
+ TimerData(const Timer<Id>& base, typename Timer<Id>::Callback&& callback,
+ std::optional<typename Timer<Id>::Duration> interval);
+
+ friend Timer<Id>;
+};
+
+} // namespace detail
} // namespace utility
} // namespace sdeventplus
diff --git a/test/utility/timer.cpp b/test/utility/timer.cpp
index 70fda1b..7cf9113 100644
--- a/test/utility/timer.cpp
+++ b/test/utility/timer.cpp
@@ -94,7 +94,6 @@
{
if (timer)
{
- expectSetEnabled(source::Enabled::Off);
timer.reset();
handler_destroy(handler_userdata);
}
@@ -377,7 +376,6 @@
EXPECT_CALL(mock,
sd_event_source_set_enabled(
expected_source2, static_cast<int>(source::Enabled::Off)))
- .WillOnce(Return(0))
.WillOnce(Return(0));
TestTimer local_timer(*event, nullptr);