Simplifies and fix bugs in FbTk::Timer * Calling Timer::setTimeout() from within Timer::start() might lead to ugly behavior (as experienced in bugs #3590078, #3600143, etc; see commit 4d307dcd10af9d817ff5c05fc40ae7487564cb31, fixes the problem partially). * Stop a timer first, then call the handler (via Timer::fireTimeout()). A given handler might call Timer::start() again, which (re)adds the Timer to the control list .. the following Timer::stop() would remove it again. * Use 'm_start' as indicator if timer is running. * Move the (now quite short) code of ::addTimer / ::removeTimer into the Timer::start() and Timer::stop() functions.
Mathias Gumz akira at fluxbox dot org
2 files changed,
36 insertions(+),
41 deletions(-)
M
src/FbTk/Timer.cc
→
src/FbTk/Timer.cc
@@ -55,7 +55,6 @@ #include <cstdio>
#include <vector> #include <set> - namespace { struct TimerCompare {@@ -64,51 +63,30 @@ return a->getEndTime() < b->getEndTime();
} }; typedef std::set<FbTk::Timer*, TimerCompare> TimerList; - TimerList s_timerlist; - -/// add a timer to the static list -void addTimer(FbTk::Timer *timer) { - - assert(timer); - int interval = timer->getInterval(); - - // interval timers have their timeout change every time they are started! - if (interval != 0) { - timer->setTimeout(interval * FbTk::FbTime::IN_SECONDS); - } - - s_timerlist.insert(timer); -} - -/// remove a timer from the static list -void removeTimer(FbTk::Timer *timer) { - - assert(timer); - s_timerlist.erase(timer); -} - - } namespace FbTk { -Timer::Timer():m_timing(false), m_once(false), m_interval(0) { +Timer::Timer() : + m_once(false), + m_interval(0), + m_start(0) { } Timer::Timer(const RefCount<Slot<void> > &handler): m_handler(handler), - m_timing(false), m_once(false), - m_interval(0) { + m_interval(0), + m_start(0) { } Timer::~Timer() { - if (isTiming()) stop(); + stop(); }@@ -131,19 +109,31 @@ }
void Timer::start() { - m_start = FbTk::FbTime::now(); + // only add Timers that actually DO something + if ( ( ! isTiming() || m_interval > 0 ) && m_handler) { + + // in case start() gets triggered on a started + // timer with 'm_interval != 0' we have to remove + // it from s_timerlist before restarting it + stop(); - // only add Timers that actually DO something - if ((! m_timing || m_interval != 0) && m_handler) { - m_timing = true; - ::addTimer(this); + m_start = FbTk::FbTime::now(); + + // interval timers have their timeout change every + // time they are started! + if (m_interval != 0) { + m_timeout = m_interval * FbTk::FbTime::IN_SECONDS; + } + s_timerlist.insert(this); } } void Timer::stop() { - m_timing = false; - ::removeTimer(this); + if (isTiming()) { + s_timerlist.erase(this); + m_start = 0; + } } uint64_t Timer::getEndTime() const {@@ -216,10 +206,16 @@ for (i = 0; i < ts; ++i) {
FbTk::Timer& t = *timeouts[i]; - t.fireTimeout(); + // first we stop the timer to remove it + // from s_timerlist t.stop(); - if (! t.doOnce()) { // restart the current timer + // then we call the handler which might (re)start 't' + // on it's own + t.fireTimeout(); + + // restart 't' if needed + if (!t.doOnce() && !t.isTiming()) { t.start(); } }
M
src/FbTk/Timer.hh
→
src/FbTk/Timer.hh
@@ -61,7 +61,7 @@ void stop();
static void updateTimers(int file_descriptor); - int isTiming() const { return m_timing; } + int isTiming() const { return (m_start > 0); } int getInterval() const { return m_interval; } int doOnce() const { return m_once; }@@ -77,11 +77,10 @@
private: RefCount<Slot<void> > m_handler; ///< what to do on a timeout - bool m_timing; ///< clock running? bool m_once; ///< do timeout only once? int m_interval; ///< Is an interval-only timer (e.g. clock), in seconds - uint64_t m_start; ///< start time in microseconds + uint64_t m_start; ///< start time in microseconds, 0 if not running uint64_t m_timeout; ///< time length in microseconds };