Keep connection alive as long as the node is connected

libraries/networking/src/udt/Connection.cpp

@@ -11,6 +11,8 @@
 
 #include "Connection.h"
 
+#include <random>
+
 #include <QtCore/QThread>
 
 #include <NumericalConstants.h>
@@ -60,6 +62,15 @@ Connection::Connection(Socket* parentSocket, HifiSockAddr destination, std::uniq
     _ack2Packet = ControlPacket::create(ControlPacket::ACK2, ACK2_PAYLOAD_BYTES);
     _lossReport = ControlPacket::create(ControlPacket::NAK, NAK_PACKET_PAYLOAD_BYTES);
     _handshakeACK = ControlPacket::create(ControlPacket::HandshakeACK, HANDSHAKE_ACK_PAYLOAD_BYTES);
+
+
+    // setup psuedo-random number generation shared by all connections
+    static std::random_device rd;
+    static std::mt19937 generator(rd());
+    static std::uniform_int_distribution<> distribution(0, SequenceNumber::MAX);
+
+    // randomize the intial sequence number
+    _initialSequenceNumber = SequenceNumber(distribution(generator));
 }
 
 Connection::~Connection() {
@@ -81,9 +92,6 @@ void Connection::stopSendQueue() {
         sendQueue->stop();
         sendQueue->deleteLater();
         
-        // since we're stopping the send queue we should consider our handshake ACK not receieved
-        _hasReceivedHandshakeACK = false;
-        
         // wait on the send queue thread so we know the send queue is gone
         sendQueueThread->quit();
         sendQueueThread->wait();
@@ -101,13 +109,22 @@ void Connection::setMaxBandwidth(int maxBandwidth) {
 
 SendQueue& Connection::getSendQueue() {
     if (!_sendQueue) {
-
         // we may have a sequence number from the previous inactive queue - re-use that so that the
         // receiver is getting the sequence numbers it expects (given that the connection must still be active)
 
         // Lasily create send queue
-        _sendQueue = SendQueue::create(_parentSocket, _destination);
+
-        _lastReceivedACK = _sendQueue->getCurrentSequenceNumber();
+        if (!_hasReceivedHandshakeACK) {
+            // First time creating a send queue for this connection
+            _sendQueue = SendQueue::create(_parentSocket, _destination, _initialSequenceNumber - 1);
+            _lastReceivedACK = _sendQueue->getCurrentSequenceNumber();
+        } else {
+            // Connection already has a handshake from a previous send queue
+            _sendQueue = SendQueue::create(_parentSocket, _destination, _lastReceivedACK);
+            // This connection has already gone through the handshake
+            // bypass it in the send queue
+            _sendQueue->handshakeACK();
+        }
 
 #ifdef UDT_CONNECTION_DEBUG
         qCDebug(networking) << "Created SendQueue for connection to" << _destination;
@@ -142,14 +159,6 @@ void Connection::queueInactive() {
 #ifdef UDT_CONNECTION_DEBUG
     qCDebug(networking) << "Connection to" << _destination << "has stopped its SendQueue.";
 #endif
-    
-    if (!_hasReceivedHandshake || !_isReceivingData) {
-#ifdef UDT_CONNECTION_DEBUG
-        qCDebug(networking) << "Connection SendQueue to" << _destination << "stopped and no data is being received - stopping connection.";
-#endif
-        
-        deactivate();
-    }
 }
 
 void Connection::queueTimeout() {
@@ -208,19 +217,6 @@ void Connection::sync() {
             && duration_cast<seconds>(sincePacketReceive).count() >= MIN_SECONDS_BEFORE_EXPIRY ) {
             // the receive side of this connection is expired
             _isReceivingData = false;
-            
-            // if we don't have a send queue that means the whole connection has expired and we can emit our signal
-            // otherwise we'll wait for it to also timeout before cleaning up
-            if (!_sendQueue) {
-
-#ifdef UDT_CONNECTION_DEBUG
-                qCDebug(networking) << "Connection to" << _destination << "no longer receiving any data and there is currently no send queue - stopping connection.";
-#endif
-                
-                deactivate();
-                
-                return;
-            }
         }
         
         // reset the number of light ACKs or non SYN ACKs during this sync interval
@@ -242,26 +238,6 @@ void Connection::sync() {
                 sendTimeoutNAK();
             }
         }
-    } else if (!_sendQueue) {
-        // we haven't received a packet and we're not sending
-        // this most likely means we were started erroneously
-        // check the start time for this connection and auto expire it after 5 seconds of not receiving or sending any data
-        static const int CONNECTION_NOT_USED_EXPIRY_SECONDS = 5;
-        auto secondsSinceStart = duration_cast<seconds>(p_high_resolution_clock::now() - _connectionStart).count();
-        
-        if (secondsSinceStart >= CONNECTION_NOT_USED_EXPIRY_SECONDS) {
-            // it's been CONNECTION_NOT_USED_EXPIRY_SECONDS and nothing has actually happened with this connection
-            // consider it inactive and emit our inactivity signal
-            
-#ifdef UDT_CONNECTION_DEBUG
-            qCDebug(networking) << "Connection to" << _destination << "did not receive or send any data in last"
-                << CONNECTION_NOT_USED_EXPIRY_SECONDS << "seconds - stopping connection.";
-#endif
-            
-            deactivate();
-            
-            return;
-        }
     }
 }
 
@@ -827,11 +803,13 @@ void Connection::processHandshakeACK(ControlPacketPointer controlPacket) {
         SequenceNumber initialSequenceNumber;
         controlPacket->readPrimitive(&initialSequenceNumber);
 
-        // hand off this handshake ACK to the send queue so it knows it can start sending
+        if (initialSequenceNumber == _initialSequenceNumber) {
-        getSendQueue().handshakeACK(initialSequenceNumber);
+            // hand off this handshake ACK to the send queue so it knows it can start sending
+            getSendQueue().handshakeACK();
 
-        // indicate that handshake ACK was received
+            // indicate that handshake ACK was received
-        _hasReceivedHandshakeACK = true;
+            _hasReceivedHandshakeACK = true;
+        }
     }
 }
 

libraries/networking/src/udt/Connection.h

@@ -73,8 +73,6 @@ public:
     
     ConnectionStats::Stats sampleStats() { return _stats.sample(); }
 
-    bool isActive() const { return _isActive; }
-
     HifiSockAddr getDestination() const { return _destination; }
 
     void setMaxBandwidth(int maxBandwidth);
@@ -83,7 +81,6 @@ public:
 
 signals:
     void packetSent();
-    void connectionInactive(const HifiSockAddr& sockAddr);
     void receiverHandshakeRequestComplete(const HifiSockAddr& sockAddr);
 
 private slots:
@@ -112,8 +109,6 @@ private:
     void resetReceiveState();
     void resetRTT();
     
-    void deactivate() { _isActive = false; emit connectionInactive(_destination); }
-    
     SendQueue& getSendQueue();
     SequenceNumber nextACK() const;
     void updateRTT(int rtt);
@@ -138,9 +133,9 @@ private:
     p_high_resolution_clock::time_point _lastReceiveTime; // holds the last time we received anything from sender
     
     bool _isReceivingData { false }; // flag used for expiry of receipt portion of connection
-    bool _isActive { true }; // flag used for inactivity of connection
 
-    SequenceNumber _initialReceiveSequenceNumber; // Randomized by peer SendQueue on creation, identifies connection during re-connect requests
+    SequenceNumber _initialSequenceNumber; // Randomized on Connection creation, identifies connection during re-connect requests
+    SequenceNumber _initialReceiveSequenceNumber; // Randomized by peer Connection on creation, identifies connection during re-connect requests
 
     LossList _lossList; // List of all missing packets
     SequenceNumber _lastReceivedSequenceNumber; // The largest sequence number received from the peer

libraries/networking/src/udt/SendQueue.cpp

@@ -12,7 +12,6 @@
 #include "SendQueue.h"
 
 #include <algorithm>
-#include <random>
 #include <thread>
 
 #include <QtCore/QCoreApplication>
@@ -62,10 +61,10 @@ private:
     Mutex2& _mutex2;
 };
 
-std::unique_ptr<SendQueue> SendQueue::create(Socket* socket, HifiSockAddr destination) {
+std::unique_ptr<SendQueue> SendQueue::create(Socket* socket, HifiSockAddr destination, SequenceNumber currentSequenceNumber) {
     Q_ASSERT_X(socket, "SendQueue::create", "Must be called with a valid Socket*");
     
-    auto queue = std::unique_ptr<SendQueue>(new SendQueue(socket, destination));
+    auto queue = std::unique_ptr<SendQueue>(new SendQueue(socket, destination, currentSequenceNumber));
 
     // Setup queue private thread
     QThread* thread = new QThread;
@@ -84,20 +83,12 @@ std::unique_ptr<SendQueue> SendQueue::create(Socket* socket, HifiSockAddr destin
     return queue;
 }
     
-SendQueue::SendQueue(Socket* socket, HifiSockAddr dest) :
+SendQueue::SendQueue(Socket* socket, HifiSockAddr dest, SequenceNumber currentSequenceNumber) :
     _socket(socket),
     _destination(dest)
 {
-    // setup psuedo-random number generation for all instances of SendQueue
+    // set our member variables from current sequence number
-    static std::random_device rd;
+    _currentSequenceNumber = currentSequenceNumber;
-    static std::mt19937 generator(rd());
-    static std::uniform_int_distribution<> distribution(0, SequenceNumber::MAX);
-
-    // randomize the intial sequence number
-    _initialSequenceNumber = SequenceNumber(distribution(generator));
-
-    // set our member variables from randomized initial number
-    _currentSequenceNumber = _initialSequenceNumber - 1;
     _atomicCurrentSequenceNumber = uint32_t(_currentSequenceNumber);
     _lastACKSequenceNumber = uint32_t(_currentSequenceNumber) - 1;
 
@@ -114,8 +105,8 @@ void SendQueue::queuePacket(std::unique_ptr<Packet> packet) {
     // call notify_one on the condition_variable_any in case the send thread is sleeping waiting for packets
     _emptyCondition.notify_one();
     
-    if (!this->thread()->isRunning() && _state == State::NotStarted) {
+    if (!thread()->isRunning() && _state == State::NotStarted) {
-        this->thread()->start();
+        thread()->start();
     }
 }
 
@@ -125,8 +116,8 @@ void SendQueue::queuePacketList(std::unique_ptr<PacketList> packetList) {
     // call notify_one on the condition_variable_any in case the send thread is sleeping waiting for packets
     _emptyCondition.notify_one();
     
-    if (!this->thread()->isRunning() && _state == State::NotStarted) {
+    if (!thread()->isRunning() && _state == State::NotStarted) {
-        this->thread()->start();
+        thread()->start();
     }
 }
 
@@ -225,8 +216,11 @@ void SendQueue::sendHandshake() {
     std::unique_lock<std::mutex> handshakeLock { _handshakeMutex };
     if (!_hasReceivedHandshakeACK) {
         // we haven't received a handshake ACK from the client, send another now
+        // if the handshake hasn't been completed, then the initial sequence number
+        // should be the current sequence number + 1
+        SequenceNumber initialSequenceNumber = _currentSequenceNumber + 1;
         auto handshakePacket = ControlPacket::create(ControlPacket::Handshake, sizeof(SequenceNumber));
-        handshakePacket->writePrimitive(_initialSequenceNumber);
+        handshakePacket->writePrimitive(initialSequenceNumber);
         _socket->writeBasePacket(*handshakePacket, _destination);
         
         // we wait for the ACK or the re-send interval to expire
@@ -235,18 +229,16 @@ void SendQueue::sendHandshake() {
     }
 }
 
-void SendQueue::handshakeACK(SequenceNumber initialSequenceNumber) {
+void SendQueue::handshakeACK() {
-    if (initialSequenceNumber == _initialSequenceNumber) {
+    {
-        {
+        std::lock_guard<std::mutex> locker { _handshakeMutex };
-            std::lock_guard<std::mutex> locker { _handshakeMutex };
+        _hasReceivedHandshakeACK = true;
-            _hasReceivedHandshakeACK = true;
-        }
-
-        _lastReceiverResponse = QDateTime::currentMSecsSinceEpoch();
-
-        // Notify on the handshake ACK condition
-        _handshakeACKCondition.notify_one();
     }
+
+    _lastReceiverResponse = QDateTime::currentMSecsSinceEpoch();
+
+    // Notify on the handshake ACK condition
+    _handshakeACKCondition.notify_one();
 }
 
 SequenceNumber SendQueue::getNextSequenceNumber() {

libraries/networking/src/udt/SendQueue.h

@@ -50,7 +50,7 @@ public:
         Stopped
     };
     
-    static std::unique_ptr<SendQueue> create(Socket* socket, HifiSockAddr destination);
+    static std::unique_ptr<SendQueue> create(Socket* socket, HifiSockAddr destination, SequenceNumber currentSequenceNumber);
 
     virtual ~SendQueue();
     
@@ -76,7 +76,7 @@ public slots:
     void nak(SequenceNumber start, SequenceNumber end);
     void fastRetransmit(SequenceNumber ack);
     void overrideNAKListFromPacket(ControlPacket& packet);
-    void handshakeACK(SequenceNumber initialSequenceNumber);
+    void handshakeACK();
 
 signals:
     void packetSent(int wireSize, int payloadSize, SequenceNumber seqNum, p_high_resolution_clock::time_point timePoint);
@@ -91,7 +91,7 @@ private slots:
     void run();
     
 private:
-    SendQueue(Socket* socket, HifiSockAddr dest);
+    SendQueue(Socket* socket, HifiSockAddr dest, SequenceNumber currentSequenceNumber);
     SendQueue(SendQueue& other) = delete;
     SendQueue(SendQueue&& other) = delete;
     
@@ -116,8 +116,6 @@ private:
     Socket* _socket { nullptr }; // Socket to send packet on
     HifiSockAddr _destination; // Destination addr
     
-    SequenceNumber _initialSequenceNumber; // Randomized on SendQueue creation, identifies connection during re-connect requests
-    
     std::atomic<uint32_t> _lastACKSequenceNumber { 0 }; // Last ACKed sequence number
     
     SequenceNumber _currentSequenceNumber { 0 }; // Last sequence number sent out

libraries/networking/src/udt/Socket.cpp

@@ -257,9 +257,6 @@ Connection* Socket::findOrCreateConnection(const HifiSockAddr& sockAddr) {
             congestionControl->setMaxBandwidth(_maxBandwidth);
             auto connection = std::unique_ptr<Connection>(new Connection(this, sockAddr, std::move(congestionControl)));
 
-            // we queue the connection to cleanup connection in case it asks for it during its own rate control sync
-            QObject::connect(connection.get(), &Connection::connectionInactive, this, &Socket::cleanupConnection);
-
             // allow higher-level classes to find out when connections have completed a handshake
             QObject::connect(connection.get(), &Connection::receiverHandshakeRequestComplete,
                              this, &Socket::clientHandshakeRequestComplete);