From 23857ea8c2f1dc98a1d9408afcb7242c2da3589c Mon Sep 17 00:00:00 2001 From: birarda Date: Mon, 19 Nov 2018 17:38:58 -0800 Subject: [PATCH] ensure packets are added to loss list after timeout --- libraries/networking/src/udt/SendQueue.cpp | 30 ++++++++++++++++++---- libraries/networking/src/udt/SendQueue.h | 4 +-- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index 9cba4970ac..3178217a36 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -134,6 +134,7 @@ void SendQueue::stop() { } int SendQueue::sendPacket(const Packet& packet) { + _lastPacketSentAt = std::chrono::high_resolution_clock::now(); return _socket->writeDatagram(packet.getData(), packet.getDataSize(), _destination); } @@ -501,12 +502,31 @@ bool SendQueue::isInactive(bool attemptedToSendPacket) { // We think the client is still waiting for data (based on the sequence number gap) // Let's wait either for a response from the client or until the estimated timeout // (plus the sync interval to allow the client to respond) has elapsed - auto waitDuration = std::chrono::microseconds(_estimatedTimeout + _syncInterval); - + + auto estimatedTimeout = std::chrono::microseconds(_estimatedTimeout); + + // cap our maximum estimated timeout to the already unreasonable 5 seconds + const auto MAXIMUM_ESTIMATED_TIMEOUT = std::chrono::seconds(5); + + if (estimatedTimeout > MAXIMUM_ESTIMATED_TIMEOUT) { + estimatedTimeout = MAXIMUM_ESTIMATED_TIMEOUT; + } + // use our condition_variable_any to wait - auto cvStatus = _emptyCondition.wait_for(locker, waitDuration); - - if (cvStatus == std::cv_status::timeout && (_packets.isEmpty() || isFlowWindowFull()) && _naks.isEmpty() + auto cvStatus = _emptyCondition.wait_for(locker, estimatedTimeout); + + // when we wake-up check if we're "stuck" either if we've slept for the estimated timeout + // or it has been that long since the last time we sent a packet + + // we are stuck if all of the following are true + // - there are no new packets to send or the flow window is full and we can't send any new packets + // - there are no packets to resend + // - the client has yet to ACK some sent packets + auto now = std::chrono::high_resolution_clock::now(); + + if ((cvStatus == std::cv_status::timeout || (now - _lastPacketSentAt > estimatedTimeout)) + && (_packets.isEmpty() || isFlowWindowFull()) + && _naks.isEmpty() && 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 65b1b89c7e..c1faac3b22 100644 --- a/libraries/networking/src/udt/SendQueue.h +++ b/libraries/networking/src/udt/SendQueue.h @@ -68,7 +68,6 @@ public: void setPacketSendPeriod(int newPeriod) { _packetSendPeriod = newPeriod; } void setEstimatedTimeout(int estimatedTimeout) { _estimatedTimeout = estimatedTimeout; } - void setSyncInterval(int syncInterval) { _syncInterval = syncInterval; } public slots: void stop(); @@ -124,7 +123,6 @@ private: std::atomic _state { State::NotStarted }; std::atomic _estimatedTimeout { 0 }; // Estimated timeout, set from CC - std::atomic _syncInterval { udt::DEFAULT_SYN_INTERVAL_USECS }; // Sync interval, set from CC std::atomic _flowWindowSize { 0 }; // Flow control window size (number of packets that can be on wire) - set from CC @@ -140,6 +138,8 @@ private: std::condition_variable _handshakeACKCondition; std::condition_variable_any _emptyCondition; + + std::chrono::high_resolution_clock::time_point _lastPacketSentAt; }; }