From abeba8aea7f1a52bff6bfd06f1187a2b71ffd5d6 Mon Sep 17 00:00:00 2001
From: psychon <psychon@726aef4b-f618-498e-8847-2d620e286838>
Date: Tue, 22 Jul 2008 11:34:38 +0000
Subject: [PATCH] Simplify the connect user code a little

The old code used a timer. That timer needed to be restarted if the user list
was changed (adding / removing of users), because it saved an iterator into
that map between runs, which would become invalid.

The new code in CConnectUserTimer::RunJob() is actually more complicated now,
but the iterator is gone. This also means no need to restart that timer
more or less always. ;) (This also means one theoretical source of bugs less)


git-svn-id: https://znc.svn.sourceforge.net/svnroot/znc/trunk@1143 726aef4b-f618-498e-8847-2d620e286838
---
 znc.cpp | 69 +++++++++++++++++++++++++--------------------------------
 znc.h   |  2 --
 2 files changed, 30 insertions(+), 41 deletions(-)

diff --git a/znc.cpp b/znc.cpp
index 6701064d..07fbab26 100644
--- a/znc.cpp
+++ b/znc.cpp
@@ -203,7 +203,6 @@ bool CZNC::HandleUserDeletion()
 	}
 
 	m_msDelUsers.clear();
-	RestartConnectUser();
 
 	return true;
 }
@@ -888,9 +887,6 @@ bool CZNC::RehashConfig(CString& sError)
 	m_msDelUsers = m_msUsers;
 	m_msUsers.clear();
 
-	// Make sure that timer doesn't have a stale iterator
-	DisableConnectUser();
-
 	if (DoRehash(sError)) {
 #ifdef _MODULES
 		GetModules().OnPostRehash();
@@ -910,9 +906,6 @@ bool CZNC::RehashConfig(CString& sError)
 		m_msDelUsers.erase(m_msDelUsers.begin());
 	}
 
-	// Make sure that users that want to connect do so
-	RestartConnectUser();
-
 	return false;
 }
 
@@ -967,9 +960,6 @@ bool CZNC::DoRehash(CString& sError)
 		m_vpListeners.erase(m_vpListeners.begin());
 	}
 
-	// Make sure that timer doesn't have a stale iterator
-	DisableConnectUser();
-
 	CString sLine;
 	bool bCommented = false;	// support for /**/ style comments
 	CUser* pUser = NULL;	// Used to keep track of which user block we are in
@@ -1564,7 +1554,7 @@ bool CZNC::DoRehash(CString& sError)
 	}
 
 	// Make sure that users that want to connect do so
-	RestartConnectUser();
+	EnableConnectUser();
 
 	return true;
 }
@@ -1726,44 +1716,58 @@ public:
 	CConnectUserTimer(int iSecs) : CCron() {
 		SetName("Connect users");
 		Start(iSecs);
-		m_itUserIter = CZNC::Get().GetUserMap().begin();
+		m_uiPosNextUser = 0;
 	}
 	virtual ~CConnectUserTimer() {}
 
 protected:
 	virtual void RunJob() {
 		unsigned int uiUserCount;
-		map<CString,CUser*>::const_iterator end;
 		bool bUsersLeft = false;
+		const map<CString,CUser*>& mUsers = CZNC::Get().GetUserMap();
+		map<CString,CUser*>::const_iterator it = mUsers.begin();
 
 		uiUserCount = CZNC::Get().GetUserMap().size();
-		end = CZNC::Get().GetUserMap().end();
+
+		if (m_uiPosNextUser >= uiUserCount) {
+			m_uiPosNextUser = 0;
+		}
+
+		for (unsigned int i = 0; i < m_uiPosNextUser; i++) {
+			it++;
+		}
 
 		// Try to connect each user, if this doesnt work, abort
 		for (unsigned int i = 0; i < uiUserCount; i++) {
-			if (m_itUserIter == end) {
-				m_itUserIter = CZNC::Get().GetUserMap().begin();
-			}
+			if (it == mUsers.end())
+				it = mUsers.begin();
 
-			CUser* pUser = m_itUserIter->second;
-
-			m_itUserIter++;
+			CUser* pUser = it->second;
+			it++;
+			m_uiPosNextUser = (m_uiPosNextUser + 1) % uiUserCount;
 
 			// Is this user disconnected and does he want to connect?
-			if (pUser->GetIRCSock() == NULL && pUser->GetIRCConnectEnabled())
+			if (pUser->GetIRCSock() == NULL && pUser->GetIRCConnectEnabled()) {
+				// The timer runs until it once didn't find any users to connect
 				bUsersLeft = true;
 
-			if (CZNC::Get().ConnectUser(pUser))
-				// Wait until next time timer fires
-				return;
+				DEBUG_ONLY(cout << "Connecting user [" << pUser->GetUserName()
+						<< "]" << endl);
+
+				if (CZNC::Get().ConnectUser(pUser))
+					// User connecting, wait until next time timer fires
+					return;
+			}
 		}
 
-		if (bUsersLeft == false)
+		if (bUsersLeft == false) {
+			DEBUG_ONLY(cout << "ConnectUserTimer done" << endl);
 			CZNC::Get().DisableConnectUser();
+		}
 	}
 
 private:
-	map<CString,CUser*>::const_iterator	m_itUserIter;
+	size_t	m_uiPosNextUser;
 };
 
 void CZNC::EnableConnectUser() {
@@ -1782,16 +1786,3 @@ void CZNC::DisableConnectUser() {
 	m_pConnectUserTimer->Stop();
 	m_pConnectUserTimer = NULL;
 }
-
-void CZNC::RestartConnectUser() {
-	DisableConnectUser();
-
-	map<CString, CUser*>::iterator end = m_msUsers.end();
-	for (map<CString,CUser*>::iterator it = m_msUsers.begin(); it != end; it++) {
-		// If there is a user without irc socket we need the timer
-		if (it->second->GetIRCSock() == NULL) {
-			EnableConnectUser();
-			return;
-		}
-	}
-}
diff --git a/znc.h b/znc.h
index d353f49a..3a75fa1e 100644
--- a/znc.h
+++ b/znc.h
@@ -180,8 +180,6 @@ public:
 	// This creates a CConnectUserTimer if we haven't got one yet
 	void EnableConnectUser();
 	void DisableConnectUser();
-	// This needs to be called if anything was added / removed to m_msUsers
-	void RestartConnectUser();
 
 private:
 	bool DoRehash(CString& sError);