New issue
Advanced search Search tips

Issue 792887 link

Starred by 1 user

Issue metadata

Status: Archived
Owner: ----
Closed: Dec 10
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

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:
 
Components: -Internals>Network Internals>Network>QUIC
Project Member

Comment 2 by sheriffbot@chromium.org, Dec 10

Status: Archived (was: Unconfirmed)
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