From b2beeae6b1a445d50de6a565afcb274516b57740 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Tue, 12 Dec 2017 17:16:07 -0800 Subject: [PATCH] Keep connection alive as long as the node is connected --- libraries/networking/src/udt/Connection.cpp | 82 ++++++++------------- libraries/networking/src/udt/Connection.h | 9 +-- libraries/networking/src/udt/SendQueue.cpp | 52 ++++++------- libraries/networking/src/udt/SendQueue.h | 8 +- libraries/networking/src/udt/Socket.cpp | 3 - 5 files changed, 57 insertions(+), 97 deletions(-) diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index f42049f107..7a31bbeedc 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -11,6 +11,8 @@ #include "Connection.h" +#include + #include #include @@ -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(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(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 - getSendQueue().handshakeACK(initialSequenceNumber); - - // indicate that handshake ACK was received - _hasReceivedHandshakeACK = true; + if (initialSequenceNumber == _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 + _hasReceivedHandshakeACK = true; + } } } diff --git a/libraries/networking/src/udt/Connection.h b/libraries/networking/src/udt/Connection.h index c134081dde..a13c29adc8 100644 --- a/libraries/networking/src/udt/Connection.h +++ b/libraries/networking/src/udt/Connection.h @@ -72,8 +72,6 @@ public: void queueReceivedMessagePacket(std::unique_ptr packet); ConnectionStats::Stats sampleStats() { return _stats.sample(); } - - bool isActive() const { return _isActive; } HifiSockAddr getDestination() const { return _destination; } @@ -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 diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index 0c029751aa..ef249b8f75 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -12,7 +12,6 @@ #include "SendQueue.h" #include -#include #include #include @@ -62,10 +61,10 @@ private: Mutex2& _mutex2; }; -std::unique_ptr SendQueue::create(Socket* socket, HifiSockAddr destination) { +std::unique_ptr 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(new SendQueue(socket, destination)); + auto queue = std::unique_ptr(new SendQueue(socket, destination, currentSequenceNumber)); // Setup queue private thread QThread* thread = new QThread; @@ -84,20 +83,12 @@ std::unique_ptr 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 - 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)); - - // set our member variables from randomized initial number - _currentSequenceNumber = _initialSequenceNumber - 1; + // set our member variables from current sequence number + _currentSequenceNumber = currentSequenceNumber; _atomicCurrentSequenceNumber = uint32_t(_currentSequenceNumber); _lastACKSequenceNumber = uint32_t(_currentSequenceNumber) - 1; @@ -114,8 +105,8 @@ void SendQueue::queuePacket(std::unique_ptr 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) { - this->thread()->start(); + if (!thread()->isRunning() && _state == State::NotStarted) { + thread()->start(); } } @@ -125,8 +116,8 @@ void SendQueue::queuePacketList(std::unique_ptr 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) { - this->thread()->start(); + if (!thread()->isRunning() && _state == State::NotStarted) { + thread()->start(); } } @@ -225,8 +216,11 @@ void SendQueue::sendHandshake() { std::unique_lock 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) { - if (initialSequenceNumber == _initialSequenceNumber) { - { - std::lock_guard locker { _handshakeMutex }; - _hasReceivedHandshakeACK = true; - } - - _lastReceiverResponse = QDateTime::currentMSecsSinceEpoch(); - - // Notify on the handshake ACK condition - _handshakeACKCondition.notify_one(); +void SendQueue::handshakeACK() { + { + std::lock_guard locker { _handshakeMutex }; + _hasReceivedHandshakeACK = true; } + + _lastReceiverResponse = QDateTime::currentMSecsSinceEpoch(); + + // Notify on the handshake ACK condition + _handshakeACKCondition.notify_one(); } SequenceNumber SendQueue::getNextSequenceNumber() { diff --git a/libraries/networking/src/udt/SendQueue.h b/libraries/networking/src/udt/SendQueue.h index 484afcb88e..b6cec15ffd 100644 --- a/libraries/networking/src/udt/SendQueue.h +++ b/libraries/networking/src/udt/SendQueue.h @@ -50,7 +50,7 @@ public: Stopped }; - static std::unique_ptr create(Socket* socket, HifiSockAddr destination); + static std::unique_ptr 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; @@ -115,8 +115,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 _lastACKSequenceNumber { 0 }; // Last ACKed sequence number diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index a3374a0f47..55643985c8 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -257,9 +257,6 @@ Connection* Socket::findOrCreateConnection(const HifiSockAddr& sockAddr) { congestionControl->setMaxBandwidth(_maxBandwidth); auto connection = std::unique_ptr(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);