From: Alexander Barton <alex@barton.de>
Date: Mon, 25 May 2020 17:02:16 +0000 (+0200)
Subject: Fix PING-PONG handling when processing backlog in read buffers
X-Git-Tag: rel-26-rc2~11
X-Git-Url: https://arthur.barton.de/cgi-bin/gitweb.cgi?p=ngircd-alex.git;a=commitdiff_plain;h=dc6807338e240d8093f43337dab7bfe488c35c4a

Fix PING-PONG handling when processing backlog in read buffers

Prior to this commit, the PONG wasn't registered correctly, becauuse the
"last ping" time was set to time(NULL), which could be bigger than the
"last data" time stamp, for example when handling the read buffer took
more than 1 second -- and this resulted in the PONG time out kicking in
effectively disconnecting a newly linked server for example, because
ngIRCd thought it was still waiting for a PONG: last data < last ping.

Now the "last ping" value has three possible values:

    0: new connection, no PING, no PONG so far.
    1: got a PONG, no longer waiting for a PONG.
  <t>: time stamp of last sent out PING command.
---

diff --git a/src/ngircd/conn-func.c b/src/ngircd/conn-func.c
index 72d38b86..3cf8a3a6 100644
--- a/src/ngircd/conn-func.c
+++ b/src/ngircd/conn-func.c
@@ -45,13 +45,17 @@ Conn_UpdateIdle(CONN_ID Idx)
 /**
  * Update "ping timestamp", the time of the last outgoing PING request.
  *
+ * the value 0 signals a newly connected client including servers during the
+ * initial "server burst"; and 1 means that no PONG is pending for a PING.
+ *
  * @param Idx Connection index.
+ * @param TimeStamp 0, 1, or time stamp.
  */
 GLOBAL void
-Conn_UpdatePing(CONN_ID Idx)
+Conn_UpdatePing(CONN_ID Idx, time_t TimeStamp)
 {
 	assert(Idx > NONE);
-	My_Connections[Idx].lastping = time(NULL);
+	My_Connections[Idx].lastping = TimeStamp;
 }
 
 /*
diff --git a/src/ngircd/conn-func.h b/src/ngircd/conn-func.h
index 1d05acc8..37f70363 100644
--- a/src/ngircd/conn-func.h
+++ b/src/ngircd/conn-func.h
@@ -30,7 +30,7 @@
 
 
 GLOBAL void Conn_UpdateIdle PARAMS((CONN_ID Idx));
-GLOBAL void Conn_UpdatePing PARAMS((CONN_ID Idx));
+GLOBAL void Conn_UpdatePing PARAMS((CONN_ID Idx, time_t TimeStamp));
 
 GLOBAL time_t Conn_GetSignon PARAMS((CONN_ID Idx));
 GLOBAL time_t Conn_GetIdle PARAMS(( CONN_ID Idx ));
diff --git a/src/ngircd/conn.c b/src/ngircd/conn.c
index c304fdb5..94c6c4a8 100644
--- a/src/ngircd/conn.c
+++ b/src/ngircd/conn.c
@@ -1866,6 +1866,9 @@ Check_Connections(void)
 	CLIENT *c;
 	CONN_ID i;
 	char msg[64];
+	time_t time_now;
+
+	time_now = time(NULL);
 
 	for (i = 0; i < Pool_Size; i++) {
 		if (My_Connections[i].sock < 0)
@@ -1880,7 +1883,7 @@ Check_Connections(void)
 			    My_Connections[i].lastdata) {
 				/* We already sent a ping */
 				if (My_Connections[i].lastping <
-				    time(NULL) - Conf_PongTimeout) {
+				    time_now - Conf_PongTimeout) {
 					/* Timeout */
 					snprintf(msg, sizeof(msg),
 						 "Ping timeout: %d seconds",
@@ -1889,10 +1892,10 @@ Check_Connections(void)
 					Conn_Close(i, NULL, msg, true);
 				}
 			} else if (My_Connections[i].lastdata <
-				   time(NULL) - Conf_PingTimeout) {
+				   time_now - Conf_PingTimeout) {
 				/* We need to send a PING ... */
 				LogDebug("Connection %d: sending PING ...", i);
-				Conn_UpdatePing(i);
+				Conn_UpdatePing(i, time_now);
 				Conn_WriteStr(i, "PING :%s",
 					      Client_ID(Client_ThisServer()));
 			}
@@ -1903,7 +1906,7 @@ Check_Connections(void)
 			 * still not registered. */
 
 			if (My_Connections[i].lastdata <
-			    time(NULL) - Conf_PongTimeout) {
+			    time_now - Conf_PongTimeout) {
 				LogDebug
 				    ("Unregistered connection %d timed out ...",
 				     i);
diff --git a/src/ngircd/irc-login.c b/src/ngircd/irc-login.c
index 846b10d6..f682e216 100644
--- a/src/ngircd/irc-login.c
+++ b/src/ngircd/irc-login.c
@@ -877,11 +877,12 @@ IRC_PONG(CLIENT *Client, REQUEST *Req)
 		    (long)(time(NULL) - Conn_GetSignon(conn)),
 		    time(NULL) - Conn_GetSignon(conn) == 1 ? "" : "s",
 		    Client_UserCount(), Channel_CountVisible(NULL));
-		Conn_UpdatePing(conn);
 	} else
 		LogDebug("Connection %d: received PONG. Lag: %ld seconds.",
 			 conn, (long)(time(NULL) - Conn_LastPing(conn)));
 
+	/* We got a PONG, so signal that none is pending on this connection. */
+	Conn_UpdatePing(conn, 1);
 	return CONNECTED;
 } /* IRC_PONG */