commit 3fcf30007a5b555a6c1c7c00f9790cc4e70e703e
author fayang <fayang@google.com> Thu Mar 24 09:16:41 2022 -0700
committer Copybara-Service <copybara-worker@google.com> Thu Mar 24 09:17:42 2022 -0700
tree cf9a55e9326e7c25a84e9579d7dba867e5e326b4
parent 8c9750c1af7a647ebfbca8e7fccf7207475ede87

Log QUIC_BUG if connection sends multiple connection closes.

PiperOrigin-RevId: 437010467

quic/core/quic_connection.cc

diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 6234489..17f4b7d 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -4537,6 +4537,16 @@
     auto* frame = new QuicConnectionCloseFrame(
         transport_version(), error, ietf_error, details,
         framer_.current_received_frame_type());
+    if (level == ENCRYPTION_FORWARD_SECURE) {
+      if (connection_close_frame_sent_.has_value()) {
+        QUIC_BUG(quic_send_multiple_connection_closes)
+            << ENDPOINT << "Already sent connection close: "
+            << connection_close_frame_sent_.value()
+            << ", going to send connection close: " << *frame;
+      } else {
+        connection_close_frame_sent_ = *frame;
+      }
+    }
     packet_creator_.ConsumeRetransmittableControlFrame(QuicFrame(frame));
     packet_creator_.FlushCurrentPacket();
   }

quic/core/quic_connection.h

diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index ec39ff1..a0738e5 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -2255,6 +2255,10 @@
   const bool flush_after_coalesce_higher_space_packets_ =
       GetQuicReloadableFlag(quic_flush_after_coalesce_higher_space_packets);
 
+  // Records the 1-RTT connection close frame sent.
+  // TODO(b/180103273): remove this after the bug gets fixed.
+  absl::optional<QuicConnectionCloseFrame> connection_close_frame_sent_;
+
   // TODO(b/205023946) Debug-only fields, to be deprecated after the bug is
   // fixed.
   absl::optional<QuicWallTime> quic_bug_10511_43_timestamp_;

quic/core/quic_connection_test.cc

diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 9fe34d3..6a943c5 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -15699,6 +15699,38 @@
   EXPECT_EQ(1u, writer_->stream_frames().size());
 }
 
+// Regression test for b/180103273
+TEST_P(QuicConnectionTest, SendMultipleConnectionCloses) {
+  if (!version().HasIetfQuicFrames() ||
+      !GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
+    return;
+  }
+  set_perspective(Perspective::IS_SERVER);
+  // Finish handshake.
+  QuicConnectionPeer::SetAddressValidated(&connection_);
+  connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+  notifier_.NeuterUnencryptedData();
+  connection_.NeuterUnencryptedPackets();
+  connection_.OnHandshakeComplete();
+  connection_.RemoveEncrypter(ENCRYPTION_INITIAL);
+  connection_.RemoveEncrypter(ENCRYPTION_HANDSHAKE);
+  EXPECT_CALL(visitor_, GetHandshakeState())
+      .WillRepeatedly(Return(HANDSHAKE_COMPLETE));
+
+  SendStreamDataToPeer(1, "foo", 0, NO_FIN, nullptr);
+  ASSERT_TRUE(connection_.BlackholeDetectionInProgress());
+  // Verify BeforeConnectionCloseSent gets called twice while OnConnectionClosed
+  // is called once.
+  EXPECT_CALL(visitor_, BeforeConnectionCloseSent()).Times(2);
+  EXPECT_CALL(visitor_, OnConnectionClosed(_, _));
+  // Send connection close w/o closing connection.
+  QuicConnectionPeer::SendConnectionClosePacket(
+      &connection_, INTERNAL_ERROR, QUIC_INTERNAL_ERROR, "internal error");
+  // Fire blackhole detection alarm.
+  EXPECT_QUIC_BUG(connection_.GetBlackholeDetectorAlarm()->Fire(),
+                  "Already sent connection close");
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace quic