commit cf918148e2912e02980964533c64033a98ac8ecb
author vasilvv <vasilvv@google.com> Mon Mar 30 11:45:27 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Mon Mar 30 11:46:07 2020 -0700
tree a2bca293f36d51dbf1d6a451d3d1631239a3047e
parent 17c433bca285fadc8e3a6f5903a2f89524242f20

Ensure QuicTransportStream::Visitor::OnFinRead() is only called once.

This fixes a non-deterministic crash in Chromium tests: https://bugs.chromium.org/p/chromium/issues/detail?id=1065854.  This also fixes a potential bug in which the stream is never marked as closed.

gfe-relnote: n/a (code not used in production)
PiperOrigin-RevId: 303787933
Change-Id: I3c4f39e709e27dc5f81ccc78729323da1810fda5

quic/quic_transport/quic_transport_stream.cc

diff --git a/quic/quic_transport/quic_transport_stream.cc b/quic/quic_transport/quic_transport_stream.cc
index 2824771..401ae7e 100644
--- a/quic/quic_transport/quic_transport_stream.cc
+++ b/quic/quic_transport/quic_transport_stream.cc
@@ -35,8 +35,8 @@
   iov.iov_base = buffer;
   iov.iov_len = buffer_size;
   const size_t result = sequencer()->Readv(&iov, 1);
-  if (sequencer()->IsClosed() && visitor_ != nullptr) {
-    visitor_->OnFinRead();
+  if (sequencer()->IsClosed()) {
+    MaybeNotifyFinRead();
   }
   return result;
 }
@@ -111,10 +111,7 @@
 
 void QuicTransportStream::OnDataAvailable() {
   if (sequencer()->IsClosed()) {
-    if (visitor_ != nullptr) {
-      visitor_->OnFinRead();
-    }
-    OnFinRead();
+    MaybeNotifyFinRead();
     return;
   }
 
@@ -138,4 +135,13 @@
   }
 }
 
+void QuicTransportStream::MaybeNotifyFinRead() {
+  if (visitor_ == nullptr || fin_read_notified_) {
+    return;
+  }
+  fin_read_notified_ = true;
+  visitor_->OnFinRead();
+  OnFinRead();
+}
+
 }  // namespace quic

quic/quic_transport/quic_transport_stream.h

diff --git a/quic/quic_transport/quic_transport_stream.h b/quic/quic_transport/quic_transport_stream.h
index d558cdb..f2bac88 100644
--- a/quic/quic_transport/quic_transport_stream.h
+++ b/quic/quic_transport/quic_transport_stream.h
@@ -63,8 +63,11 @@
   using QuicStream::WriteMemSlices;
   using QuicStream::WriteOrBufferData;
 
+  void MaybeNotifyFinRead();
+
   QuicTransportSessionInterface* session_interface_;
   std::unique_ptr<Visitor> visitor_ = nullptr;
+  bool fin_read_notified_ = false;
 };
 
 }  // namespace quic