From 6b37d1f7e9bd8d451ab920ab09aa2d2e24877d62 Mon Sep 17 00:00:00 2001 From: brain Date: Fri, 9 Feb 2007 18:13:13 +0000 Subject: Add and properly test the ability for an InspTimer to DelTimer itself from within its own Tick method. This wasnt supported before and would corrupt the iterator in the TickTimers() method of TimerManager. Non the less, peaveys new (perfectly sensible) fix broke it because i didnt document the caveat about DelTimer() :p This FIXES m_ident and possibly some other stuff. git-svn-id: http://svn.inspircd.org/repository/trunk/inspircd@6553 e03df62e-2008-0410-955e-edbf42e46eb7 --- include/timer.h | 7 ++++++- src/modules/m_ident.cpp | 8 ++++++-- src/timer.cpp | 42 ++++++++++++++++++++++++++++++++++++++---- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/include/timer.h b/include/timer.h index 2d12d84e5..dbf83c2c8 100644 --- a/include/timer.h +++ b/include/timer.h @@ -111,7 +111,9 @@ class TimerManager : public Extensible /** A map of timergroups, each group has a specific trigger time */ typedef std::map timerlist; - + /** Set when ticking timers, to prevent deletion while iterating + */ + bool CantDeleteHere; private: /** The current timer set, a map of timergroups @@ -119,6 +121,9 @@ class TimerManager : public Extensible timerlist Timers; public: + /** Constructor + */ + TimerManager(); /** Tick all pending InspTimers * @param TIME the current system time */ diff --git a/src/modules/m_ident.cpp b/src/modules/m_ident.cpp index a2574a19f..525a2dc97 100644 --- a/src/modules/m_ident.cpp +++ b/src/modules/m_ident.cpp @@ -52,6 +52,7 @@ class RFC1413 : public InspSocket strcpy(newident,"~"); strlcat(newident,u->ident,IDENTMAX); strlcpy(u->ident,newident,IDENTMAX); + Instance->next_call = Instance->Time(); } } @@ -121,6 +122,7 @@ class RFC1413 : public InspSocket // Fixes issue reported by webs, 7 Jun 2006 if (u && (Instance->SE->GetRef(ufd) == u)) { + Instance->next_call = Instance->Time(); u->Shrink("ident_data"); } } @@ -129,6 +131,7 @@ class RFC1413 : public InspSocket { if (u && (Instance->SE->GetRef(ufd) == u)) { + Instance->next_call = Instance->Time(); u->Shrink("ident_data"); } } @@ -176,6 +179,7 @@ class RFC1413 : public InspSocket } else { + Instance->next_call = Instance->Time(); return true; } } @@ -236,7 +240,7 @@ class ModuleIdent : public Module strcpy(newident,"~"); strlcat(newident,user->ident,IDENTMAX); strlcpy(user->ident,newident,IDENTMAX); - //delete ident; + ServerInstance->next_call = ServerInstance->Time(); } return 0; } @@ -286,12 +290,12 @@ class ModuleIdent : public Module { ident->u = NULL; ServerInstance->SE->DelFd(ident); - //delete ident; } } virtual ~ModuleIdent() { + ServerInstance->next_call = ServerInstance->Time(); } virtual Version GetVersion() diff --git a/src/timer.cpp b/src/timer.cpp index 6be7aa06c..e7635c528 100644 --- a/src/timer.cpp +++ b/src/timer.cpp @@ -14,15 +14,22 @@ #include "inspircd.h" #include "timer.h" +TimerManager::TimerManager() : CantDeleteHere(false) +{ +} + void TimerManager::TickTimers(time_t TIME) { + this->CantDeleteHere = true; timerlist::iterator found = Timers.find(TIME); if (found != Timers.end()) { timergroup* x = found->second; - /* - * There are pending timers to trigger + /* There are pending timers to trigger. + * WARNING: Timers may delete themselves from within + * their own Tick methods! see the comment below in + * the DelTimer method. */ for (timergroup::iterator y = x->begin(); y != x->end(); y++) { @@ -41,10 +48,30 @@ void TimerManager::TickTimers(time_t TIME) Timers.erase(found); DELETE(x); } + + this->CantDeleteHere = false; } void TimerManager::DelTimer(InspTimer* T) { + if (this->CantDeleteHere) + { + /* If a developer tries to delete a timer from within its own Tick method, + * then chances are this is just going to totally fuck over the timergroup + * and timerlist iterators and cause a crash. Thanks to peavey and Bricker + * for noticing this bug. + * If we're within the tick loop when the DelTimer is called (signified + * by the var 'CantDeleteHere') then we simply return for non-repeating + * timers, and cancel the repeat on repeating timers. We can do this because + * we know that the timer tick loop will safely delete the timer for us + * anyway and therefore we avoid stack corruption. + */ + if (T->GetRepeat()) + T->CancelRepeat(); + else + return; + } + timerlist::iterator found = Timers.find(T->GetTimer()); if (found != Timers.end()) @@ -58,15 +85,16 @@ void TimerManager::DelTimer(InspTimer* T) DELETE(n); x->erase(y); if (!x->size()) + { Timers.erase(found); + } return; } } } } -/* - * Because some muppets may do odd things, and their ircd may lock up due +/** Because some muppets may do odd things, and their ircd may lock up due * to crappy 3rd party modules, or they may change their system time a bit, * this accounts for shifts of up to 120 secs by looking behind for missed * timers and executing them. This is only executed once every 5 secs. @@ -75,6 +103,10 @@ void TimerManager::DelTimer(InspTimer* T) */ void TimerManager::TickMissedTimers(time_t TIME) { + /** See comment above in TickTimers + */ + this->CantDeleteHere = true; + for (time_t n = TIME-1; n > TIME-120; n--) { timerlist::iterator found = Timers.find(n); @@ -99,6 +131,8 @@ void TimerManager::TickMissedTimers(time_t TIME) DELETE(x); } } + + this->CantDeleteHere = false; } void TimerManager::AddTimer(InspTimer* T, long secs_from_now) -- cgit v1.2.3