From 08dff9c7aca7a14d692fc50a6551d3aed755c1d8 Mon Sep 17 00:00:00 2001
From: Stephen Birarda <commit@birarda.com>
Date: Mon, 21 Mar 2016 09:06:39 -0700
Subject: [PATCH] handle expiry check even if a packet was sent

---
 libraries/networking/src/udt/SendQueue.cpp | 49 ++++++++++------------
 libraries/networking/src/udt/SendQueue.h   |  1 -
 2 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp
index bda12b2e4d..90f6cf9813 100644
--- a/libraries/networking/src/udt/SendQueue.cpp
+++ b/libraries/networking/src/udt/SendQueue.cpp
@@ -133,7 +133,6 @@ void SendQueue::sendPacket(const Packet& packet) {
     
 void SendQueue::ack(SequenceNumber ack) {
     // this is a response from the client, re-set our timeout expiry and our last response time
-    _timeoutExpiryCount = 0;
     _lastReceiverResponse = uint64_t(QDateTime::currentMSecsSinceEpoch());
     
     if (_lastACKSequenceNumber == (uint32_t) ack) {
@@ -161,7 +160,6 @@ void SendQueue::ack(SequenceNumber ack) {
 
 void SendQueue::nak(SequenceNumber start, SequenceNumber end) {
     // this is a response from the client, re-set our timeout expiry
-    _timeoutExpiryCount = 0;
     _lastReceiverResponse = uint64_t(QDateTime::currentMSecsSinceEpoch());
     
     {
@@ -175,7 +173,6 @@ void SendQueue::nak(SequenceNumber start, SequenceNumber end) {
 
 void SendQueue::overrideNAKListFromPacket(ControlPacket& packet) {
     // this is a response from the client, re-set our timeout expiry
-    _timeoutExpiryCount = 0;
     _lastReceiverResponse = uint64_t(QDateTime::currentMSecsSinceEpoch());
     
     {
@@ -438,28 +435,31 @@ bool SendQueue::maybeResendPacket() {
 }
 
 bool SendQueue::isInactive(bool sentAPacket) {
-    if (!sentAPacket) {
-        // check if it is time to break this connection
-        
-        // that will be the case if we have had 16 timeouts since hearing back from the client, and it has been
-        // at least 5 seconds
-        static const int NUM_TIMEOUTS_BEFORE_INACTIVE = 16;
-        static const int MIN_SECONDS_BEFORE_INACTIVE_MS = 5 * 1000;
-        if (_timeoutExpiryCount >= NUM_TIMEOUTS_BEFORE_INACTIVE &&
-            _lastReceiverResponse > 0 &&
-            (QDateTime::currentMSecsSinceEpoch() - _lastReceiverResponse) > MIN_SECONDS_BEFORE_INACTIVE_MS) {
-            // If the flow window has been full for over CONSIDER_INACTIVE_AFTER,
-            // then signal the queue is inactive and return so it can be cleaned up
-            
+    // check for connection timeout first
+
+    // that will be the case if we have had 16 timeouts since hearing back from the client, and it has been
+    // at least 5 seconds
+    static const int NUM_TIMEOUTS_BEFORE_INACTIVE = 16;
+    static const int MIN_SECONDS_BEFORE_INACTIVE_MS = 5 * 1000;
+
+    auto sinceLastResponse = (QDateTime::currentMSecsSinceEpoch() - _lastReceiverResponse);
+
+    if (sinceLastResponse >= quint64(NUM_TIMEOUTS_BEFORE_INACTIVE * _estimatedTimeout) &&
+        _lastReceiverResponse > 0 &&
+        sinceLastResponse > MIN_SECONDS_BEFORE_INACTIVE_MS) {
+        // If the flow window has been full for over CONSIDER_INACTIVE_AFTER,
+        // then signal the queue is inactive and return so it can be cleaned up
+
 #ifdef UDT_CONNECTION_DEBUG
-            qCDebug(networking) << "SendQueue to" << _destination << "reached" << NUM_TIMEOUTS_BEFORE_INACTIVE << "timeouts"
-                << "and 5s before receiving any ACK/NAK and is now inactive. Stopping.";
+        qCDebug(networking) << "SendQueue to" << _destination << "reached" << NUM_TIMEOUTS_BEFORE_INACTIVE << "timeouts"
+            << "and 5s before receiving any ACK/NAK and is now inactive. Stopping.";
 #endif
-            
-            deactivate();
-            return true;
-        }
-        
+
+        deactivate();
+        return true;
+    }
+
+    if (!sentAPacket) {
         // During our processing above we didn't send any packets
         
         // If that is still the case we should use a condition_variable_any to sleep until we have data to handle.
@@ -504,9 +504,6 @@ bool SendQueue::isInactive(bool sentAPacket) {
                 auto cvStatus = _emptyCondition.wait_for(locker, waitDuration);
                 
                 if (cvStatus == std::cv_status::timeout) {
-                    // increase the number of timeouts
-                    ++_timeoutExpiryCount;
-                    
                     if (SequenceNumber(_lastACKSequenceNumber) < _currentSequenceNumber) {
                         // after a timeout if we still have sent packets that the client hasn't ACKed we
                         // add them to the loss list
diff --git a/libraries/networking/src/udt/SendQueue.h b/libraries/networking/src/udt/SendQueue.h
index 0390f2ff1f..0c8dfd8766 100644
--- a/libraries/networking/src/udt/SendQueue.h
+++ b/libraries/networking/src/udt/SendQueue.h
@@ -118,7 +118,6 @@ private:
     
     std::atomic<int> _estimatedTimeout { 0 }; // Estimated timeout, set from CC
     std::atomic<int> _syncInterval { udt::DEFAULT_SYN_INTERVAL_USECS }; // Sync interval, set from CC
-    std::atomic<int> _timeoutExpiryCount { 0 }; // The number of times the timeout has expired without response from client
     std::atomic<uint64_t> _lastReceiverResponse { 0 }; // Timestamp for the last time we got new data from the receiver (ACK/NAK)
     
     std::atomic<int> _flowWindowSize { 0 }; // Flow control window size (number of packets that can be on wire) - set from CC