From 1638ee61936bc91758be39c3463c6e46d0d655e7 Mon Sep 17 00:00:00 2001 From: attilamolnar Date: Tue, 9 Apr 2013 23:51:06 +0200 Subject: [PATCH] Timer changes and TimerManager enhancements Timer::Tick() now has a bool return value: if false is returned the timer is deleted using operator delete, otherwise, if it's a repeating timer then it's rescheduled (readded) Timers are removed from the TimerManager automatically at destruction Timers are now stored in a multimap instead of a sorted vector --- include/inspsocket.h | 2 +- include/timer.h | 58 +++++++++++------------ src/dns.cpp | 6 ++- src/inspsocket.cpp | 9 ++-- src/modules/extra/m_pgsql.cpp | 13 +++-- src/modules/m_spanningtree/cachetimer.cpp | 5 +- src/modules/m_spanningtree/cachetimer.h | 2 +- src/modules/m_spanningtree/main.cpp | 3 +- src/timer.cpp | 53 ++++++++++----------- 9 files changed, 78 insertions(+), 73 deletions(-) diff --git a/include/inspsocket.h b/include/inspsocket.h index 75c6dc511..ccc2301ed 100644 --- a/include/inspsocket.h +++ b/include/inspsocket.h @@ -92,7 +92,7 @@ class CoreExport SocketTimeout : public Timer /** Handle tick event */ - virtual void Tick(time_t now); + virtual bool Tick(time_t now); }; /** diff --git a/include/timer.h b/include/timer.h index 0dda6876e..6f1834178 100644 --- a/include/timer.h +++ b/include/timer.h @@ -21,6 +21,8 @@ #pragma once +class Module; + /** Timer class for one-second resolution timers * Timer provides a facility which allows module * developers to create one-shot timers. The timer @@ -28,61 +30,70 @@ * resolution. To use Timer, inherit a class from * Timer, then insert your inherited class into the * queue using Server::AddTimer(). The Tick() method of - * your object (which you should override) will be called + * your object (which you have to override) will be called * at the given time. */ class CoreExport Timer { - private: /** The triggering time */ time_t trigger; + /** Number of seconds between triggers */ - long secs; + unsigned int secs; + /** True if this is a repeating timer */ bool repeat; + public: /** Default constructor, initializes the triggering time + * @param mod The module that created this timer * @param secs_from_now The number of seconds from now to trigger the timer * @param now The time now * @param repeating Repeat this timer every secs_from_now seconds if set to true */ - Timer(long secs_from_now, time_t now, bool repeating = false) + Timer(unsigned int secs_from_now, time_t now, bool repeating = false) { trigger = now + secs_from_now; secs = secs_from_now; repeat = repeating; } - /** Default destructor, does nothing. + /** Default destructor, removes the timer from the timer manager */ - virtual ~Timer() { } + virtual ~Timer(); /** Retrieve the current triggering time */ - virtual time_t GetTimer() + time_t GetTrigger() const { return trigger; } /** Sets the trigger timeout to a new value + * This does not update the bookkeeping in TimerManager, use SetInterval() + * to change the interval between ticks while keeping TimerManager updated */ - virtual void SetTimer(time_t t) + void SetTrigger(time_t nexttrigger) { - trigger = t; + trigger = nexttrigger; } + /** Sets the interval between two ticks. + */ + void SetInterval(time_t interval); + /** Called when the timer ticks. * You should override this method with some useful code to * handle the tick event. */ - virtual void Tick(time_t TIME) = 0; + virtual bool Tick(time_t TIME) = 0; /** Returns true if this timer is set to repeat */ - bool GetRepeat() + bool GetRepeat() const { return repeat; } @@ -90,7 +101,7 @@ class CoreExport Timer /** Returns the interval (number of seconds between ticks) * of this timer object. */ - long GetSecs() + unsigned int GetInterval() const { return secs; } @@ -98,12 +109,6 @@ class CoreExport Timer /** Cancels the repeat state of a repeating timer. * If you call this method, then the next time your * timer ticks, it will be removed immediately after. - * You should use this method call to remove a recurring - * timer if you wish to do so within the timer's Tick - * event, as calling TimerManager::DelTimer() from within - * the Timer::Tick() method is dangerous and may - * cause a segmentation fault. Calling CancelRepeat() - * is safe in this case. */ void CancelRepeat() { @@ -111,6 +116,7 @@ class CoreExport Timer } }; +typedef std::multimap TimerMap; /** This class manages sets of Timers, and triggers them at their defined times. * This will ensure timers are not missed, as well as removing timers that have @@ -118,17 +124,11 @@ class CoreExport Timer */ class CoreExport TimerManager { - protected: /** A list of all pending timers */ - std::vector Timers; + TimerMap Timers; public: - /** Constructor - */ - TimerManager(); - ~TimerManager(); - /** Tick all pending Timers * @param TIME the current system time */ @@ -139,12 +139,8 @@ class CoreExport TimerManager */ void AddTimer(Timer *T); - /** Delete an Timer - * @param T an Timer derived class to delete + /** Remove a Timer + * @param T an Timer derived class to remove */ void DelTimer(Timer* T); - - /** Compares two timers - */ - static bool TimerComparison( Timer *one, Timer*two); }; diff --git a/src/dns.cpp b/src/dns.cpp index 461722b29..b9c605e78 100644 --- a/src/dns.cpp +++ b/src/dns.cpp @@ -123,9 +123,10 @@ class CacheTimer : public Timer CacheTimer(DNS* thisdns) : Timer(3600, ServerInstance->Time(), true), dns(thisdns) { } - virtual void Tick(time_t) + virtual bool Tick(time_t) { dns->PruneCache(); + return true; } }; @@ -143,7 +144,7 @@ class RequestTimeout : public Timer Tick(0); } - void Tick(time_t) + bool Tick(time_t) { if (ServerInstance->Res->requests[watchid] == watch) { @@ -157,6 +158,7 @@ class RequestTimeout : public Timer ServerInstance->Res->requests[watchid] = NULL; delete watch; } + return false; } }; diff --git a/src/inspsocket.cpp b/src/inspsocket.cpp index fdd002ad6..5f2da2341 100644 --- a/src/inspsocket.cpp +++ b/src/inspsocket.cpp @@ -431,12 +431,12 @@ void StreamSocket::WriteData(const std::string &data) ServerInstance->SE->ChangeEventMask(this, FD_ADD_TRIAL_WRITE); } -void SocketTimeout::Tick(time_t) +bool SocketTimeout::Tick(time_t) { ServerInstance->Logs->Log("SOCKET", LOG_DEBUG,"SocketTimeout::Tick"); if (ServerInstance->SE->GetRef(this->sfd) != this->sock) - return; + return false; if (this->sock->state == I_CONNECTING) { @@ -452,6 +452,7 @@ void SocketTimeout::Tick(time_t) } this->sock->Timeout = NULL; + return false; } void BufferedSocket::OnConnected() { } @@ -476,8 +477,8 @@ BufferedSocket::~BufferedSocket() this->Close(); if (Timeout) { - ServerInstance->Timers->DelTimer(Timeout); - Timeout = NULL; + // The timer is removed from the TimerManager in Timer::~Timer() + delete Timeout; } } diff --git a/src/modules/extra/m_pgsql.cpp b/src/modules/extra/m_pgsql.cpp index 6d2e0c88a..4690f9851 100644 --- a/src/modules/extra/m_pgsql.cpp +++ b/src/modules/extra/m_pgsql.cpp @@ -62,7 +62,7 @@ class ReconnectTimer : public Timer ReconnectTimer(ModulePgSQL* m) : Timer(5, ServerInstance->Time(), false), mod(m) { } - virtual void Tick(time_t TIME); + virtual bool Tick(time_t TIME); }; struct QueueItem @@ -504,6 +504,11 @@ class ModulePgSQL : public Module ConnMap connections; ReconnectTimer* retimer; + ModulePgSQL() + : retimer(NULL) + { + } + void init() { ReadConf(); @@ -514,8 +519,7 @@ class ModulePgSQL : public Module virtual ~ModulePgSQL() { - if (retimer) - ServerInstance->Timers->DelTimer(retimer); + delete retimer; ClearAllConnections(); } @@ -594,10 +598,11 @@ class ModulePgSQL : public Module } }; -void ReconnectTimer::Tick(time_t time) +bool ReconnectTimer::Tick(time_t time) { mod->retimer = NULL; mod->ReadConf(); + return false; } void SQLConn::DelayReconnect() diff --git a/src/modules/m_spanningtree/cachetimer.cpp b/src/modules/m_spanningtree/cachetimer.cpp index 6703e84b1..4fdc7056f 100644 --- a/src/modules/m_spanningtree/cachetimer.cpp +++ b/src/modules/m_spanningtree/cachetimer.cpp @@ -24,12 +24,13 @@ /* $ModDep: m_spanningtree/cachetimer.h m_spanningtree/utils.h */ -CacheRefreshTimer::CacheRefreshTimer(SpanningTreeUtilities *Util) : Timer(3600, ServerInstance->Time(), true), Utils(Util) +CacheRefreshTimer::CacheRefreshTimer(SpanningTreeUtilities* Util) : Timer(3600, ServerInstance->Time(), true), Utils(Util) { } -void CacheRefreshTimer::Tick(time_t TIME) +bool CacheRefreshTimer::Tick(time_t TIME) { Utils->RefreshIPCache(); + return true; } diff --git a/src/modules/m_spanningtree/cachetimer.h b/src/modules/m_spanningtree/cachetimer.h index acfb7434c..5d2278cd6 100644 --- a/src/modules/m_spanningtree/cachetimer.h +++ b/src/modules/m_spanningtree/cachetimer.h @@ -31,5 +31,5 @@ class CacheRefreshTimer : public Timer SpanningTreeUtilities *Utils; public: CacheRefreshTimer(SpanningTreeUtilities* Util); - virtual void Tick(time_t TIME); + virtual bool Tick(time_t TIME); }; diff --git a/src/modules/m_spanningtree/main.cpp b/src/modules/m_spanningtree/main.cpp index bd49a3075..029f5e888 100644 --- a/src/modules/m_spanningtree/main.cpp +++ b/src/modules/m_spanningtree/main.cpp @@ -70,7 +70,7 @@ void ModuleSpanningTree::init() ServerInstance->Modules->AddService(commands->fhost); ServerInstance->Modules->AddService(commands->fident); ServerInstance->Modules->AddService(commands->fname); - RefreshTimer = new CacheRefreshTimer(Utils); + RefreshTimer = new CacheRefreshTimer(this, Utils); ServerInstance->Timers->AddTimer(RefreshTimer); Implementation eventlist[] = @@ -907,6 +907,7 @@ CullResult ModuleSpanningTree::cull() { Utils->cull(); ServerInstance->Timers->DelTimer(RefreshTimer); + delete RefreshTimer; return this->Module::cull(); } diff --git a/src/timer.cpp b/src/timer.cpp index a1ee0b488..f098a5e6b 100644 --- a/src/timer.cpp +++ b/src/timer.cpp @@ -25,55 +25,54 @@ #include "inspircd.h" #include "timer.h" -TimerManager::TimerManager() +void Timer::SetInterval(time_t newinterval) { + ServerInstance->Timers->DelTimer(this); + secs = newinterval; + SetTrigger(ServerInstance->Time() + newinterval); + ServerInstance->Timers->AddTimer(this); } -TimerManager::~TimerManager() +Timer::~Timer() { - for(std::vector::iterator i = Timers.begin(); i != Timers.end(); i++) - delete *i; + ServerInstance->Timers->DelTimer(this); } void TimerManager::TickTimers(time_t TIME) { - while ((Timers.size()) && (TIME > (*Timers.begin())->GetTimer())) + for (TimerMap::iterator i = Timers.begin(); i != Timers.end(); ) { - std::vector::iterator i = Timers.begin(); - Timer *t = (*i); + Timer* t = i->second; + if (t->GetTrigger() > TIME) + break; - // Probable fix: move vector manipulation to *before* we modify the vector. - Timers.erase(i); + Timers.erase(i++); - t->Tick(TIME); - if (t->GetRepeat()) + if (!t->Tick(TIME)) + delete t; + else if (t->GetRepeat()) { - t->SetTimer(TIME + t->GetSecs()); + t->SetTrigger(TIME + t->GetInterval()); AddTimer(t); } - else - delete t; } } -void TimerManager::DelTimer(Timer* T) +void TimerManager::DelTimer(Timer* t) { - std::vector::iterator i = std::find(Timers.begin(), Timers.end(), T); + std::pair itpair = Timers.equal_range(t->GetTrigger()); - if (i != Timers.end()) + for (TimerMap::iterator i = itpair.first; i != itpair.second; ++i) { - delete (*i); - Timers.erase(i); + if (i->second == t) + { + Timers.erase(i); + break; + } } } -void TimerManager::AddTimer(Timer* T) -{ - Timers.push_back(T); - sort(Timers.begin(), Timers.end(), TimerManager::TimerComparison); -} - -bool TimerManager::TimerComparison( Timer *one, Timer *two) +void TimerManager::AddTimer(Timer* t) { - return (one->GetTimer()) < (two->GetTimer()); + Timers.insert(std::make_pair(t->GetTrigger(), t)); } -- 2.39.2