In QuicConnection::OnCanWrite(), early return after writing buffered crypto frames if the connection gets closed.
Protected by quic_reloadable_flag_quic_no_write_control_frame_upon_connection_close.
PiperOrigin-RevId: 467718239
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h
index 24386b3..f043e97 100644
--- a/quiche/quic/core/quic_flags_list.h
+++ b/quiche/quic/core/quic_flags_list.h
@@ -17,6 +17,8 @@
QUIC_FLAG(quic_restart_flag_quic_testonly_default_true, true)
// If bytes in flight has dipped below 1.25*MaxBW in the last round, do not exit PROBE_UP due to excess queue buildup.
QUIC_FLAG(quic_reloadable_flag_quic_bbr2_no_probe_up_exit_if_no_queue, true)
+// If trrue, early return before write control frame in OnCanWrite() if the connection is already closed.
+QUIC_FLAG(quic_reloadable_flag_quic_no_write_control_frame_upon_connection_close, true)
// If true and BBQ1 connection option is set, QUIC BBR will use a pacing gain of 2.773 at startup and 0.5 at DRAIN.
QUIC_FLAG(quic_reloadable_flag_quic_bbr2_support_new_startup_pacing_gain, true)
// If true, 1) remove all experiments that tunes blackhole detection delay or path degrading delay, and 2) ensure network blackhole delay is at least path degrading delay plus 2 PTOs.
diff --git a/quiche/quic/core/quic_session.cc b/quiche/quic/core/quic_session.cc
index f8fd2af..8d87de1 100644
--- a/quiche/quic/core/quic_session.cc
+++ b/quiche/quic/core/quic_session.cc
@@ -627,9 +627,12 @@
if (crypto_stream->HasBufferedCryptoFrames()) {
crypto_stream->WriteBufferedCryptoFrames();
}
- if (crypto_stream->HasBufferedCryptoFrames()) {
- // Cannot finish writing buffered crypto frames, connection is write
- // blocked.
+ if ((GetQuicReloadableFlag(
+ quic_no_write_control_frame_upon_connection_close) &&
+ !connection_->connected()) ||
+ crypto_stream->HasBufferedCryptoFrames()) {
+ // Cannot finish writing buffered crypto frames, connection is either
+ // write blocked or closed.
return;
}
}
@@ -846,7 +849,7 @@
bool QuicSession::WriteControlFrame(const QuicFrame& frame,
TransmissionType type) {
- QUICHE_DCHECK(connection()->connected())
+ QUIC_BUG_IF(quic_bug_12435_11, !connection()->connected())
<< ENDPOINT << "Try to write control frames when connection is closed.";
if (!IsEncryptionEstablished()) {
// Suppress the write before encryption gets established.
diff --git a/quiche/quic/core/quic_session_test.cc b/quiche/quic/core/quic_session_test.cc
index 5cd4cb9..916ed1b 100644
--- a/quiche/quic/core/quic_session_test.cc
+++ b/quiche/quic/core/quic_session_test.cc
@@ -3088,6 +3088,46 @@
}
}
+TEST_P(QuicSessionTestServer, BufferedCryptoFrameCausesWriteError) {
+ if (!VersionHasIetfQuicFrames(transport_version())) {
+ return;
+ }
+ std::string data(1350, 'a');
+ TestCryptoStream* crypto_stream = session_.GetMutableCryptoStream();
+ // Only consumed 1000 bytes.
+ EXPECT_CALL(*connection_, SendCryptoData(ENCRYPTION_FORWARD_SECURE, 1350, 0))
+ .WillOnce(Return(1000));
+ crypto_stream->WriteCryptoData(ENCRYPTION_FORWARD_SECURE, data);
+ EXPECT_TRUE(session_.HasPendingHandshake());
+ EXPECT_TRUE(session_.WillingAndAbleToWrite());
+
+ EXPECT_CALL(*connection_,
+ SendCryptoData(ENCRYPTION_FORWARD_SECURE, 350, 1000))
+ .WillOnce(Return(0));
+ // Buffer the HANDSHAKE_DONE frame.
+ EXPECT_CALL(*connection_, SendControlFrame(_)).WillOnce(Return(false));
+ CryptoHandshakeMessage msg;
+ session_.GetMutableCryptoStream()->OnHandshakeMessage(msg);
+
+ // Flush both frames.
+ EXPECT_CALL(*connection_,
+ SendCryptoData(ENCRYPTION_FORWARD_SECURE, 350, 1000))
+ .WillOnce(testing::InvokeWithoutArgs([this]() {
+ connection_->ReallyCloseConnection(
+ QUIC_PACKET_WRITE_ERROR, "write error",
+ ConnectionCloseBehavior::SILENT_CLOSE);
+ return 350;
+ }));
+ if (!GetQuicReloadableFlag(
+ quic_no_write_control_frame_upon_connection_close)) {
+ EXPECT_CALL(*connection_, SendControlFrame(_)).WillOnce(Return(false));
+ EXPECT_QUIC_BUG(session_.OnCanWrite(),
+ "Try to write control frames when connection is closed");
+ } else {
+ session_.OnCanWrite();
+ }
+}
+
// A client test class that can be used when the automatic configuration is not
// desired.
class QuicSessionTestClientUnconfigured : public QuicSessionTestBase {
