QUIC: Crypto stream gets destroyed while waiting for acks.
Reported by
slus...@gmail.com,
Dec 7 2017
|
||
Issue description
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/604.4.7 (KHTML, like Gecko) Version/11.0.2 Safari/604.4.7
Example URL:
Steps to reproduce the problem:
Perform 1-RTT download.
What is the expected behavior?
What went wrong?
When client processes REJ message before ACK in QuicCryptoClientHandshaker::DoReceiveREJ() it calls
session()->connection()->NeuterUnencryptedPackets(). This method discards retransmittability info.
But QuicStream::OnStreamFrameDiscarded() is not called in this case and stream_bytes_outstanding_ is not updated.
Note that QuicStream::OnStreamFrameDiscarded() calls send_buffer_.RemoveStreamFrame().
Therefore, at the end, stream_bytes_outstanding_ is non-zero and and QuicStream::IsWaitingForAcks() returns true.
Simple fix would be:
diff --git a/net/quic/core/quic_sent_packet_manager.cc b/net/quic/core/quic_sent_packet_manager.cc
index db69f08..f6e9bfd 100644
--- a/net/quic/core/quic_sent_packet_manager.cc
+++ b/net/quic/core/quic_sent_packet_manager.cc
@@ -344,7 +344,7 @@ void QuicSentPacketManager::NeuterUnencryptedPackets() {
// perspective.
pending_retransmissions_.erase(packet_number);
unacked_packets_.RemoveFromInFlight(packet_number);
- unacked_packets_.RemoveRetransmittability(packet_number);
+ unacked_packets_.CancelRetransmissionsForPacket(packet_number);
}
}
}
diff --git a/net/quic/core/quic_unacked_packet_map.cc b/net/quic/core/quic_unacked_packet_map.cc
index 8ff3645..5b3781b 100644
--- a/net/quic/core/quic_unacked_packet_map.cc
+++ b/net/quic/core/quic_unacked_packet_map.cc
@@ -253,6 +253,20 @@ void QuicUnackedPacketMap::CancelRetransmissionsForStream(
}
}
+void QuicUnackedPacketMap::CancelRetransmissionsForPacket(
+ QuicPacketNumber packet_number) {
+ DCHECK_GE(packet_number, least_unacked_);
+ DCHECK_LT(packet_number, least_unacked_ + unacked_packets_.size());
+ QuicTransmissionInfo* info =
+ &unacked_packets_[packet_number - least_unacked_];
+ if (stream_notifier_) {
+ for (const auto& frame : info->retransmittable_frames) {
+ stream_notifier_->OnStreamFrameDiscarded(*frame.stream_frame);
+ }
+ }
+ RemoveRetransmittability(info);
+}
+
bool QuicUnackedPacketMap::HasUnackedPackets() const {
return !unacked_packets_.empty();
}
diff --git a/net/quic/core/quic_unacked_packet_map.h b/net/quic/core/quic_unacked_packet_map.h
index 43bc635..bc289e0 100644
--- a/net/quic/core/quic_unacked_packet_map.h
+++ b/net/quic/core/quic_unacked_packet_map.h
@@ -55,6 +55,9 @@ class QUIC_EXPORT_PRIVATE QuicUnackedPacketMap {
// No longer retransmit data for |stream_id|.
void CancelRetransmissionsForStream(QuicStreamId stream_id);
+ // No longer retransmit data for |packet_number|.
+ void CancelRetransmissionsForPacket(QuicPacketNumber packet_number);
+
// Returns true if the unacked packet |packet_number| has retransmittable
// frames. This will return false if the packet has been acked, if a
// previous transmission of this packet was ACK'd, or if this packet has been
Did this work before? N/A
Chrome version: Canary Channel: n/a
OS Version: OS X 10.12.6
Flash Version:
,
Dec 10
Issue has not been modified or commented on in the last 365 days, please re-open or file a new bug if this is still an issue. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
||
►
Sign in to add a comment |
||
Comment 1 by mmenke@chromium.org
, Dec 7 2017