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

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<time_t, timergroup*> 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)