From b2a417c1554f81d9ce7b35c8eea6b5e571ed62b6 Mon Sep 17 00:00:00 2001
From: Pierre Ossman <ossman@cendio.se>
Date: Fri, 11 Dec 2015 19:32:21 +0100
Subject: [PATCH] Use a queue for congestion pings

This reduces the data sent, and avoids any problems with the
client corrupting it.
---
 common/rfb/VNCSConnectionST.cxx | 40 +++++++++++++++++++++------------
 common/rfb/VNCSConnectionST.h   |  6 +++--
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx
index d2206f9b..ef49a28e 100644
--- a/common/rfb/VNCSConnectionST.cxx
+++ b/common/rfb/VNCSConnectionST.cxx
@@ -72,7 +72,7 @@ VNCSConnectionST::VNCSConnectionST(VNCServerST* server_, network::Socket *s,
     fenceDataLen(0), fenceData(NULL),
     baseRTT(-1), congWindow(0), ackedOffset(0), sentOffset(0),
     minRTT(-1), seenCongestion(false),
-    pingCounter(0), congestionTimer(this),
+    congestionTimer(this),
     server(server_), updates(false),
     updateRenderedCursor(false), removeRenderedCursor(false),
     continuousUpdates(false), encodeManager(this), pointerEventTime(0),
@@ -659,6 +659,8 @@ void VNCSConnectionST::setDesktopSize(int fb_width, int fb_height,
 
 void VNCSConnectionST::fence(rdr::U32 flags, unsigned len, const char data[])
 {
+  rdr::U8 type;
+
   if (flags & fenceFlagRequest) {
     if (flags & fenceFlagSyncNext) {
       pendingSyncFence = true;
@@ -682,18 +684,20 @@ void VNCSConnectionST::fence(rdr::U32 flags, unsigned len, const char data[])
     return;
   }
 
-  struct RTTInfo rttInfo;
+  if (len < 1)
+    vlog.error("Fence response of unexpected size received");
 
-  switch (len) {
+  type = data[0];
+
+  switch (type) {
   case 0:
     // Initial dummy fence;
     break;
-  case sizeof(struct RTTInfo):
-    memcpy(&rttInfo, data, sizeof(struct RTTInfo));
-    handleRTTPong(rttInfo);
+  case 1:
+    handleRTTPong();
     break;
   default:
-    vlog.error("Fence response of unexpected size received");
+    vlog.error("Fence response of unexpected type received");
   }
 }
 
@@ -734,7 +738,8 @@ void VNCSConnectionST::supportsLocalCursor()
 
 void VNCSConnectionST::supportsFence()
 {
-  writer()->writeFence(fenceFlagRequest, 0, NULL);
+  char type = 0;
+  writer()->writeFence(fenceFlagRequest, sizeof(type), &type);
 }
 
 void VNCSConnectionST::supportsContinuousUpdates()
@@ -768,6 +773,7 @@ bool VNCSConnectionST::handleTimeout(Timer* t)
 void VNCSConnectionST::writeRTTPing()
 {
   struct RTTInfo rttInfo;
+  char type;
 
   if (!cp.supportsFence)
     return;
@@ -778,13 +784,14 @@ void VNCSConnectionST::writeRTTPing()
   rttInfo.offset = sock->outStream().length();
   rttInfo.inFlight = rttInfo.offset - ackedOffset;
 
+  pings.push_back(rttInfo);
+
   // We need to make sure any old update are already processed by the
   // time we get the response back. This allows us to reliably throttle
   // back on client overload, as well as network overload.
+  type = 1;
   writer()->writeFence(fenceFlagRequest | fenceFlagBlockBefore,
-                       sizeof(struct RTTInfo), (const char*)&rttInfo);
-
-  pingCounter++;
+                       sizeof(type), &type);
 
   sentOffset = rttInfo.offset;
 
@@ -793,11 +800,16 @@ void VNCSConnectionST::writeRTTPing()
     congestionTimer.start(__rfbmin(baseRTT * 2, 100));
 }
 
-void VNCSConnectionST::handleRTTPong(const struct RTTInfo &rttInfo)
+void VNCSConnectionST::handleRTTPong()
 {
+  struct RTTInfo rttInfo;
   unsigned rtt, delay;
 
-  pingCounter--;
+  if (pings.empty())
+    return;
+
+  rttInfo = pings.front();
+  pings.pop_front();
 
   rtt = msSince(&rttInfo.tv);
   if (rtt < 1)
@@ -881,7 +893,7 @@ bool VNCSConnectionST::isCongested()
   // This could further clog up the tubes, but congestion control isn't
   // really working properly right now anyway as the wire would otherwise
   // be idle for at least RTT/2.
-  if (pingCounter == 1)
+  if (pings.size() == 1)
     return false;
 
   return true;
diff --git a/common/rfb/VNCSConnectionST.h b/common/rfb/VNCSConnectionST.h
index 74a6946d..ccca6744 100644
--- a/common/rfb/VNCSConnectionST.h
+++ b/common/rfb/VNCSConnectionST.h
@@ -27,7 +27,9 @@
 #ifndef __RFB_VNCSCONNECTIONST_H__
 #define __RFB_VNCSCONNECTIONST_H__
 
+#include <list>
 #include <set>
+
 #include <rfb/SConnection.h>
 #include <rfb/SMsgWriter.h>
 #include <rfb/VNCServerST.h>
@@ -160,7 +162,7 @@ namespace rfb {
 
     // Congestion control
     void writeRTTPing();
-    void handleRTTPong(const struct RTTInfo &rttInfo);
+    void handleRTTPong();
     bool isCongested();
     void updateCongestion();
 
@@ -195,8 +197,8 @@ namespace rfb {
 
     unsigned minRTT;
     bool seenCongestion;
-    unsigned pingCounter;
     Timer congestionTimer;
+    std::list<struct RTTInfo> pings;
 
     VNCServerST* server;
     SimpleUpdateTracker updates;
-- 
2.39.5