Safety checking (a really really clever trick btw) in CullList, to make sure that the user we're removing IS the user that was marked for deletion X seconds ago, even if the user has since quit, to be replaced by another unfortunate wretch who was given the same pointer address (eeep)

git-svn-id: http://svn.inspircd.org/repository/trunk/inspircd@3529 e03df62e-2008-0410-955e-edbf42e46eb7

2 files changed, 26 insertions, 4 deletions

diff --git a/include/cull_list.h b/include/cull_list.h
index 36a8886a0..cc0f01c06 100644
--- a/include/cull_list.h
+++ b/include/cull_list.h
@@ -82,9 +82,12 @@ class CullList
          std::vector<CullItem> list;
 	 /** A list of users who have already been
 	  * placed on the list, as a map for fast
-	  * reference.
+	  * reference. When deleting an item, the
+	  * time_t value stored here must match
+	  * the one of the actual userrec, otherwise
+	  * we don't delete it (its a different user)
 	  */
-	 std::map<userrec*,int> exempt;
+	 std::map<userrec*,time_t> exempt;
 	 
 	 /** Check if a user pointer is valid
 	  * (e.g. it exists in the user hash)
diff --git a/src/cull_list.cpp b/src/cull_list.cpp
index 7c4afa16c..224a9e3da 100644
--- a/src/cull_list.cpp
+++ b/src/cull_list.cpp
@@ -54,10 +54,29 @@ extern user_hash clientlist;
 
 bool CullList::IsValid(userrec* user)
 {
+	time_t esignon = 0;
+	std::map<userrec*,time_t>::iterator es = exempt.find(user);
+	if (es != exempt.end())
+		esignon = es->second;
+
 	for (user_hash::iterator u = clientlist.begin(); u != clientlist.end(); u++)
 	{
+		/*
+		 * BUGFIX
+		 *
+		 * Because there is an undetermined period of time between a user existing,
+		 * and this function being called, we have to check for the following condition:
+		 *
+		 * Between CullList::AddItem(u) being called, and CullList::IsValid(u) being called,
+		 * the user with the pointer u has quit, but only to be REPLACED WITH A NEW USER WHO
+		 * BECAUSE OF ALLOCATION RULES, HAS THE SAME MEMORY ADDRESS! To prevent this, we
+		 * cross reference each pointer to the user's signon time, and if the signon times
+		 * do not match, we return false here to indicate this user is NOT valid as it
+		 * seems to differ from the pointer snapshot we got a few seconds earlier. Should
+		 * prevent a few random crashes during netsplits.
+		 */
 		if (user == u->second)
-			return true;
+			return (u->second->signon == esignon);
 	}
 	return false;
 }
@@ -90,7 +109,7 @@ void CullList::AddItem(userrec* user, std::string reason)
 	{
 	        CullItem item(user,reason);
 	        list.push_back(item);
-	        exempt[user] = 1;
+	        exempt[user] = user->signon;
 	}
 }