m_spanningtree Rewrite PING logic to use Timers

7 files changed, 195 insertions, 120 deletions

diff --git a/src/modules/m_spanningtree/main.cpp b/src/modules/m_spanningtree/main.cpp
index c21064683..31d822789 100644
--- a/src/modules/m_spanningtree/main.cpp
+++ b/src/modules/m_spanningtree/main.cpp
@@ -153,64 +153,6 @@ std::string ModuleSpanningTree::TimeToStr(time_t secs)
 			+ ConvToStr(secs) + "s");
 }
 
-void ModuleSpanningTree::DoPingChecks(time_t curtime)
-{
-	/*
-	 * Cancel remote burst mode on any servers which still have it enabled due to latency/lack of data.
-	 * This prevents lost REMOTECONNECT notices
-	 */
-	long ts = ServerInstance->Time() * 1000 + (ServerInstance->Time_ns() / 1000000);
-
-restart:
-	for (server_hash::iterator i = Utils->serverlist.begin(); i != Utils->serverlist.end(); i++)
-	{
-		TreeServer *s = i->second;
-
-		// Skip myself
-		if (s->IsRoot())
-			continue;
-
-		// Do not ping servers that are not fully connected yet!
-		// Servers which are connected to us have IsLocal() == true and if they're fully connected
-		// then Socket->LinkState == CONNECTED. Servers that are linked to another server are always fully connected.
-		if (s->IsLocal() && s->GetSocket()->GetLinkState() != CONNECTED)
-			continue;
-
-		// Now do PING checks on all servers
-		// Only ping if this server needs one
-		if (curtime >= s->NextPingTime())
-		{
-			// And if they answered the last
-			if (s->AnsweredLastPing())
-			{
-				// They did, send a ping to them
-				s->SetNextPingTime(curtime + Utils->PingFreq);
-				s->GetSocket()->WriteLine(CmdBuilder("PING").push(s->GetID()));
-				s->LastPingMsec = ts;
-			}
-			else
-			{
-				// They didn't answer the last ping, if they are locally connected, get rid of them.
-				if (s->IsLocal())
-				{
-					TreeSocket* sock = s->GetSocket();
-					sock->SendError("Ping timeout");
-					sock->Close();
-					goto restart;
-				}
-			}
-		}
-
-		// If warn on ping enabled and not warned and the difference is sufficient and they didn't answer the last ping...
-		if ((Utils->PingWarnTime) && (!s->Warned) && (curtime >= s->NextPingTime() - (Utils->PingFreq - Utils->PingWarnTime)) && (!s->AnsweredLastPing()))
-		{
-			/* The server hasnt responded, send a warning to opers */
-			ServerInstance->SNO->WriteToSnoMask('l',"Server \002%s\002 has not responded to PING for %d seconds, high latency.", s->GetName().c_str(), Utils->PingWarnTime);
-			s->Warned = true;
-		}
-	}
-}
-
 void ModuleSpanningTree::ConnectServer(Autoconnect* a, bool on_timer)
 {
 	if (!a)
@@ -471,7 +413,6 @@ void ModuleSpanningTree::OnUserMessage(User* user, void* dest, int target_type,
 void ModuleSpanningTree::OnBackgroundTimer(time_t curtime)
 {
 	AutoConnectServers(curtime);
-	DoPingChecks(curtime);
 	DoConnectTimeout(curtime);
 }
 
diff --git a/src/modules/m_spanningtree/main.h b/src/modules/m_spanningtree/main.h
index c81eb2d0b..13c743f73 100644
--- a/src/modules/m_spanningtree/main.h
+++ b/src/modules/m_spanningtree/main.h
@@ -103,10 +103,6 @@ class ModuleSpanningTree : public Module
 	 */
 	ModResult HandleRemoteWhois(const std::vector<std::string>& parameters, User* user);
 
-	/** Ping all local servers
-	 */
-	void DoPingChecks(time_t curtime);
-
 	/** Connect a server locally
 	 */
 	void ConnectServer(Link* x, Autoconnect* y = NULL);
diff --git a/src/modules/m_spanningtree/pingtimer.cpp b/src/modules/m_spanningtree/pingtimer.cpp
new file mode 100644
index 000000000..1c96259bf
--- /dev/null
+++ b/src/modules/m_spanningtree/pingtimer.cpp
@@ -0,0 +1,102 @@
+/*
+ * InspIRCd -- Internet Relay Chat Daemon
+ *
+ *   Copyright (C) 2015 Attila Molnar <attilamolnar@hush.com>
+ *
+ * This file is part of InspIRCd.  InspIRCd is free software: you can
+ * redistribute it and/or modify it under the terms of the GNU General Public
+ * License as published by the Free Software Foundation, version 2.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+#include "inspircd.h"
+
+#include "pingtimer.h"
+#include "treeserver.h"
+#include "commandbuilder.h"
+
+PingTimer::PingTimer(TreeServer* ts)
+	: Timer(Utils->PingFreq)
+	, server(ts)
+	, state(PS_SENDPING)
+{
+}
+
+PingTimer::State PingTimer::TickInternal()
+{
+	// Timer expired, take next action based on what happened last time
+	if (state == PS_SENDPING)
+	{
+		// Last ping was answered, send next ping
+		server->GetSocket()->WriteLine(CmdBuilder("PING").push(server->GetID()));
+		LastPingMsec = ServerInstance->Time() * 1000 + (ServerInstance->Time_ns() / 1000000);
+		// Warn next unless warnings are disabled. If they are, jump straight to timeout.
+		if (Utils->PingWarnTime)
+			return PS_WARN;
+		else
+			return PS_TIMEOUT;
+	}
+	else if (state == PS_WARN)
+	{
+		// No pong arrived in PingWarnTime seconds, send a warning to opers
+		ServerInstance->SNO->WriteToSnoMask('l', "Server \002%s\002 has not responded to PING for %d seconds, high latency.", server->GetName().c_str(), GetInterval());
+		return PS_TIMEOUT;
+	}
+	else // PS_TIMEOUT
+	{
+		// They didn't answer the last ping, if they are locally connected, get rid of them
+		if (server->IsLocal())
+		{
+			TreeSocket* sock = server->GetSocket();
+			sock->SendError("Ping timeout");
+			sock->Close();
+		}
+
+		// If the server is non-locally connected, don't do anything until we get a PONG.
+		// This is to avoid pinging the server and warning opers more than once.
+		// If they do answer eventually, we will move to the PS_SENDPING state and ping them again.
+		return PS_IDLE;
+	}
+}
+
+void PingTimer::SetState(State newstate)
+{
+	state = newstate;
+
+	// Set when should the next Tick() happen based on the state
+	if (state == PS_SENDPING)
+		SetInterval(Utils->PingFreq);
+	else if (state == PS_WARN)
+		SetInterval(Utils->PingWarnTime);
+	else if (state == PS_TIMEOUT)
+		SetInterval(Utils->PingFreq - Utils->PingWarnTime);
+
+	// If state == PS_IDLE, do not set the timer, see above why
+}
+
+bool PingTimer::Tick(time_t currtime)
+{
+	if (server->IsDead())
+		return false;
+
+	SetState(TickInternal());
+	return false;
+}
+
+void PingTimer::OnPong()
+{
+	// Calculate RTT
+	long ts = ServerInstance->Time() * 1000 + (ServerInstance->Time_ns() / 1000000);
+	server->rtt = ts - LastPingMsec;
+
+	// Change state to send ping next, also reschedules the timer appropriately
+	SetState(PS_SENDPING);
+}
diff --git a/src/modules/m_spanningtree/pingtimer.h b/src/modules/m_spanningtree/pingtimer.h
new file mode 100644
index 000000000..753558689
--- /dev/null
+++ b/src/modules/m_spanningtree/pingtimer.h
@@ -0,0 +1,77 @@
+/*
+ * InspIRCd -- Internet Relay Chat Daemon
+ *
+ *   Copyright (C) 2015 Attila Molnar <attilamolnar@hush.com>
+ *
+ * This file is part of InspIRCd.  InspIRCd is free software: you can
+ * redistribute it and/or modify it under the terms of the GNU General Public
+ * License as published by the Free Software Foundation, version 2.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+#pragma once
+
+class TreeServer;
+
+/** Handles PINGing servers and killing them on timeout
+ */
+class PingTimer : public Timer
+{
+	enum State
+	{
+		/** Send PING next */
+		PS_SENDPING,
+		/** Warn opers next */
+		PS_WARN,
+		/** Kill the server next due to ping timeout */
+		PS_TIMEOUT,
+		/** Do nothing */
+		PS_IDLE
+	};
+
+	/** Server the timer is interacting with
+	 */
+	TreeServer* const server;
+
+	/** What to do when the timer ticks next
+	 */
+	State state;
+
+	/** Last ping time in milliseconds, used to calculate round trip time
+	 */
+	unsigned long LastPingMsec;
+
+	/** Update internal state and reschedule timer according to the new state
+	 * @param newstate State to change to
+	 */
+	void SetState(State newstate);
+
+	/** Process timer tick event
+	 * @return State to change to
+	 */
+	State TickInternal();
+
+	/** Called by the TimerManager when the timer expires
+	 * @param currtime Time now
+	 * @return Always false, we reschedule ourselves instead
+	 */
+	bool Tick(time_t currtime) CXX11_OVERRIDE;
+
+ public:
+	/** Construct the timer. This doesn't schedule the timer.
+	 * @param server TreeServer to interact with
+	 */
+	PingTimer(TreeServer* server);
+
+	/** Register a PONG from the server
+	 */
+	void OnPong();
+};
diff --git a/src/modules/m_spanningtree/pong.cpp b/src/modules/m_spanningtree/pong.cpp
index a7dc64f83..5d97f2af2 100644
--- a/src/modules/m_spanningtree/pong.cpp
+++ b/src/modules/m_spanningtree/pong.cpp
@@ -35,9 +35,7 @@ CmdResult CommandPong::HandleServer(TreeServer* server, std::vector<std::string>
 	if (params[0] == ServerInstance->Config->GetSID())
 	{
 		// PONG for us
-		long ts = ServerInstance->Time() * 1000 + (ServerInstance->Time_ns() / 1000000);
-		server->rtt = ts - server->LastPingMsec;
-		server->SetPingFlag();
+		server->OnPong();
 	}
 	return CMD_SUCCESS;
 }
diff --git a/src/modules/m_spanningtree/treeserver.cpp b/src/modules/m_spanningtree/treeserver.cpp
index 534315ff7..e004f897e 100644
--- a/src/modules/m_spanningtree/treeserver.cpp
+++ b/src/modules/m_spanningtree/treeserver.cpp
@@ -38,8 +38,9 @@ TreeServer::TreeServer()
 	, VersionString(ServerInstance->GetVersionString())
 	, fullversion(ServerInstance->GetVersionString(true))
 	, Socket(NULL), sid(ServerInstance->Config->GetSID()), behind_bursting(0), isdead(false)
+	, pingtimer(this)
 	, ServerUser(ServerInstance->FakeClient)
-	, age(ServerInstance->Time()), Warned(false), UserCount(ServerInstance->Users.GetLocalUsers().size())
+	, age(ServerInstance->Time()), UserCount(ServerInstance->Users.GetLocalUsers().size())
 	, OperCount(0), rtt(0), StartBurst(0), Hidden(false)
 {
 	AddHashEntry();
@@ -52,13 +53,14 @@ TreeServer::TreeServer()
 TreeServer::TreeServer(const std::string& Name, const std::string& Desc, const std::string& id, TreeServer* Above, TreeSocket* Sock, bool Hide)
 	: Server(Name, Desc)
 	, Parent(Above), Socket(Sock), sid(id), behind_bursting(Parent->behind_bursting), isdead(false)
+	, pingtimer(this)
 	, ServerUser(new FakeUser(id, this))
-	, age(ServerInstance->Time()), Warned(false), UserCount(0), OperCount(0), rtt(0), StartBurst(0), Hidden(Hide)
+	, age(ServerInstance->Time()), UserCount(0), OperCount(0), rtt(0), StartBurst(0), Hidden(Hide)
 {
 	ServerInstance->Logs->Log(MODNAME, LOG_DEBUG, "New server %s behind_bursting %u", GetName().c_str(), behind_bursting);
 	CheckULine();
-	SetNextPingTime(ServerInstance->Time() + Utils->PingFreq);
-	SetPingFlag();
+
+	ServerInstance->Timers.AddTimer(&pingtimer);
 
 	/* find the 'route' for this server (e.g. the one directly connected
 	 * to the local server, which we can use to reach it)
@@ -135,11 +137,6 @@ void TreeServer::FinishBurstInternal()
 		behind_bursting--;
 	ServerInstance->Logs->Log(MODNAME, LOG_DEBUG, "FinishBurstInternal() %s behind_bursting %u", GetName().c_str(), behind_bursting);
 
-	if (!IsBehindBursting())
-	{
-		SetNextPingTime(ServerInstance->Time() + Utils->PingFreq);
-		SetPingFlag();
-	}
 	for (ChildServers::const_iterator i = Children.begin(); i != Children.end(); ++i)
 	{
 		TreeServer* child = *i;
@@ -261,27 +258,6 @@ void TreeServer::AddHashEntry()
 	Utils->sidlist[sid] = this;
 }
 
-void TreeServer::SetNextPingTime(time_t t)
-{
-	this->NextPing = t;
-	LastPingWasGood = false;
-}
-
-time_t TreeServer::NextPingTime()
-{
-	return NextPing;
-}
-
-bool TreeServer::AnsweredLastPing()
-{
-	return LastPingWasGood;
-}
-
-void TreeServer::SetPingFlag()
-{
-	LastPingWasGood = true;
-}
-
 CullResult TreeServer::cull()
 {
 	// Recursively cull all servers that are under us in the tree
diff --git a/src/modules/m_spanningtree/treeserver.h b/src/modules/m_spanningtree/treeserver.h
index 4465de15e..1a0203ba0 100644
--- a/src/modules/m_spanningtree/treeserver.h
+++ b/src/modules/m_spanningtree/treeserver.h
@@ -22,6 +22,7 @@
 #pragma once
 
 #include "treesocket.h"
+#include "pingtimer.h"
 
 /** Each server in the tree is represented by one class of
  * type TreeServer. A locally connected TreeServer can
@@ -49,8 +50,6 @@ class TreeServer : public Server
 	std::string fullversion;
 
 	TreeSocket* Socket;			/* Socket used to communicate with this server */
-	time_t NextPing;			/* After this time, the server should be PINGed*/
-	bool LastPingWasGood;			/* True if the server responded to the last PING with a PONG */
 	std::string sid;			/* Server ID */
 
 	/** Counter counting how many servers are bursting in front of this server, including
@@ -64,6 +63,10 @@ class TreeServer : public Server
 	 */
 	bool isdead;
 
+	/** Timer handling PINGing the server and killing it on timeout
+	 */
+	PingTimer pingtimer;
+
 	/** This method is used to add this TreeServer to the
 	 * hash maps. It is only called by the constructors.
 	 */
@@ -82,8 +85,6 @@ class TreeServer : public Server
 	FakeUser* const ServerUser;		/* User representing this server */
 	const time_t age;
 
-	bool Warned;				/* True if we've warned opers about high latency on this server */
-
 	unsigned int UserCount;			/* How many users are on this server? [note: doesn't care about +i] */
 	unsigned int OperCount;			/* How many opers are on this server? */
 
@@ -143,18 +144,6 @@ class TreeServer : public Server
 	 */
 	const std::string& GetFullVersion() const { return fullversion; }
 
-	/** Set time we are next due to ping this server
-	 */
-	void SetNextPingTime(time_t t);
-
-	/** Get the time we are next due to ping this server
-	 */
-	time_t NextPingTime();
-
-	/** Last ping time in milliseconds, used to calculate round trip time
-	 */
-	unsigned long LastPingMsec;
-
 	/** Round trip time of last ping
 	 */
 	unsigned long rtt;
@@ -167,14 +156,6 @@ class TreeServer : public Server
 	 */
 	bool Hidden;
 
-	/** True if the server answered their last ping
-	 */
-	bool AnsweredLastPing();
-
-	/** Set the server as responding to its last ping
-	 */
-	void SetPingFlag();
-
 	/** Get the TreeSocket pointer for local servers.
 	 * For remote servers, this returns NULL.
 	 */
@@ -234,6 +215,10 @@ class TreeServer : public Server
 	 */
 	void BeginBurst(unsigned long startms = 0);
 
+	/** Register a PONG from the server
+	 */
+	void OnPong() { pingtimer.OnPong(); }
+
 	CullResult cull();
 
 	/** Destructor, deletes ServerUser unless IsRoot()