Merge pull request #14436 from birarda/bug/no-naks-after-timeout

ensure packets are always added to loss list after estimated timeout
This commit is contained in:
John Conklin II 2018-11-21 15:48:14 -08:00 committed by GitHub
commit 7e2f1ca64a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 27 additions and 7 deletions

View file

@ -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

View file

@ -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 { State::NotStarted };
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> _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;
};
}