From dcd5a4aec2e9efe9f0ae9301d40d260d61431217 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 28 Aug 2015 09:47:25 -0700 Subject: [PATCH] address comments in code review --- libraries/networking/src/FileResourceRequest.cpp | 5 ++--- libraries/networking/src/udt/SendQueue.cpp | 13 ++++++------- libraries/networking/src/udt/SendQueue.h | 4 ---- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/libraries/networking/src/FileResourceRequest.cpp b/libraries/networking/src/FileResourceRequest.cpp index 1c5e0a1977..ffaaa0d193 100644 --- a/libraries/networking/src/FileResourceRequest.cpp +++ b/libraries/networking/src/FileResourceRequest.cpp @@ -23,13 +23,12 @@ void FileResourceRequest::doSend() { if (file.open(QFile::ReadOnly)) { _data = file.readAll(); _result = ResourceRequest::Success; - emit finished(); } else { _result = ResourceRequest::AccessDenied; - emit finished(); } } else { _result = ResourceRequest::NotFound; - emit finished(); } + + emit finished(); } diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index 3ee4d48d1d..9a371749e2 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -304,14 +304,13 @@ void SendQueue::run() { handshakeLock.unlock(); bool sentAPacket = maybeResendPacket(); - bool flowWindowFull = false; // if we didn't find a packet to re-send AND we think we can fit a new packet on the wire // (this is according to the current flow window size) then we send out a new packet if (_hasReceivedHandshakeACK && !sentAPacket) { - flowWindowFull = (seqlen(SequenceNumber { (uint32_t) _lastACKSequenceNumber }, _currentSequenceNumber) > - _flowWindowSize); - sentAPacket = maybeSendNewPacket(); + if (seqlen(SequenceNumber { (uint32_t) _lastACKSequenceNumber }, _currentSequenceNumber) > _flowWindowSize) { + sentAPacket = maybeSendNewPacket(); + } } // since we're a while loop, give the thread a chance to process events @@ -354,17 +353,17 @@ void SendQueue::run() { if (uint32_t(_lastACKSequenceNumber) == uint32_t(_currentSequenceNumber)) { // we've sent the client as much data as we have (and they've ACKed it) // either wait for new data to send or 5 seconds before cleaning up the queue - static const auto EMPTY_QUEUES_INACTIVE_TIMEOUT_US = std::chrono::microseconds(5 * 1000 * 1000); + static const auto EMPTY_QUEUES_INACTIVE_TIMEOUT = std::chrono::seconds(5 * 1000 * 1000); // use our condition_variable_any to wait - auto cvStatus = _emptyCondition.wait_for(doubleLock, EMPTY_QUEUES_INACTIVE_TIMEOUT_US); + auto cvStatus = _emptyCondition.wait_for(doubleLock, EMPTY_QUEUES_INACTIVE_TIMEOUT); // we have the double lock again - Make sure to unlock it doubleLock.unlock(); if (cvStatus == std::cv_status::timeout) { qDebug() << "SendQueue to" << _destination << "has been empty for" - << std::chrono::duration_cast(EMPTY_QUEUES_INACTIVE_TIMEOUT_US).count() + << EMPTY_QUEUES_INACTIVE_TIMEOUT.count() << "seconds and receiver has ACKed all packets." << "The queue is considered inactive and will be stopped."; diff --git a/libraries/networking/src/udt/SendQueue.h b/libraries/networking/src/udt/SendQueue.h index 22d52d28f0..c6668f1d09 100644 --- a/libraries/networking/src/udt/SendQueue.h +++ b/libraries/networking/src/udt/SendQueue.h @@ -111,10 +111,6 @@ private: std::atomic _flowWindowSize { 0 }; // Flow control window size (number of packets that can be on wire) - set from CC - // Used to detect when the connection becomes inactive for too long - bool _flowWindowWasFull = false; - time_point _flowWindowFullSince; - mutable std::mutex _naksLock; // Protects the naks list. LossList _naks; // Sequence numbers of packets to resend