Avoid nested CloseConnection calls in QuicConnection.
Protected by quic_reloadable_flag_quic_avoid_nested_close_connection.
PiperOrigin-RevId: 689062814
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h
index 4285e06..3cf6f59 100755
--- a/quiche/common/quiche_feature_flags_list.h
+++ b/quiche/common/quiche_feature_flags_list.h
@@ -14,6 +14,7 @@
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_add_stream_info_to_idle_close_detail, false, true, "If true, include stream information in idle timeout connection close detail.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_allow_client_enabled_bbr_v2, true, true, "If true, allow client to enable BBRv2 on server via connection option 'B2ON'.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_allow_host_in_request2, true, true, "If true, requests with a host header will be allowed.")
+QUICHE_FLAG(bool, quiche_reloadable_flag_quic_avoid_nested_close_connection, false, true, "If true, if QuicConnection::CloseConnection triggers a nested call to itself, make the nested call a no-op.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_bbr2_extra_acked_window, false, true, "When true, the BBR4 copt sets the extra_acked window to 20 RTTs and BBR5 sets it to 40 RTTs.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_bbr2_probe_two_rounds, true, true, "When true, the BB2U copt causes BBR2 to wait two rounds with out draining the queue before exiting PROBE_UP and BB2S has the same effect in STARTUP.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_bbr2_simplify_inflight_hi, true, true, "When true, the BBHI copt causes QUIC BBRv2 to use a simpler algorithm for raising inflight_hi in PROBE_UP.")
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc
index 274e1b7..843ef84 100644
--- a/quiche/quic/core/quic_connection.cc
+++ b/quiche/quic/core/quic_connection.cc
@@ -22,6 +22,7 @@
#include <utility>
#include <vector>
+#include "absl/cleanup/cleanup.h"
#include "absl/strings/escaping.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/str_format.h"
@@ -4559,6 +4560,17 @@
return;
}
+ if (in_close_connection_) {
+ QUIC_DLOG(INFO) << "Connection is being closed.";
+ return;
+ }
+
+ if (GetQuicReloadableFlag(quic_avoid_nested_close_connection)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_avoid_nested_close_connection);
+ in_close_connection_ = true;
+ }
+ absl::Cleanup cleanup = [this]() { in_close_connection_ = false; };
+
if (ietf_error != NO_IETF_QUIC_ERROR) {
QUIC_DLOG(INFO) << ENDPOINT << "Closing connection: " << connection_id()
<< ", with wire error: " << ietf_error
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h
index e09fa3d..cbb587e 100644
--- a/quiche/quic/core/quic_connection.h
+++ b/quiche/quic/core/quic_connection.h
@@ -2349,6 +2349,9 @@
// close.
bool connected_;
+ // True if the connection is in the CloseConnection stack.
+ bool in_close_connection_ = false;
+
// Set to false if the connection should not send truncated connection IDs to
// the peer, even if the peer supports it.
bool can_truncate_connection_ids_;
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc
index 6aa7ef3..ae219f3 100644
--- a/quiche/quic/core/quic_connection_test.cc
+++ b/quiche/quic/core/quic_connection_test.cc
@@ -1596,6 +1596,41 @@
::testing::ValuesIn(GetTestParams()),
::testing::PrintToStringParamName());
+// Regression test for b/372756997.
+TEST_P(QuicConnectionTest, NoNestedCloseConnection) {
+ if (!GetQuicReloadableFlag(quic_avoid_nested_close_connection)) {
+ // EXPECT_QUIC_BUG tests are expensive so only run one instance of them.
+ if (!IsDefaultTestConfiguration()) {
+ return;
+ }
+ }
+ EXPECT_TRUE(connection_.connected());
+ EXPECT_CALL(visitor_, OnConnectionClosed(_, ConnectionCloseSource::FROM_SELF))
+ .WillRepeatedly(
+ Invoke(this, &QuicConnectionTest::SaveConnectionCloseFrame));
+ EXPECT_CALL(connection_, OnSerializedPacket(_)).Times(AnyNumber());
+
+ // Prepare the writer to fail to send the first connection close packet due
+ // to the packet being too large.
+ writer_->SetShouldWriteFail();
+ writer_->SetWriteError(*writer_->MessageTooBigErrorCode());
+
+ if (GetQuicReloadableFlag(quic_avoid_nested_close_connection)) {
+ connection_.CloseConnection(
+ QUIC_CRYPTO_TOO_MANY_ENTRIES, "Closed by test",
+ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ EXPECT_THAT(saved_connection_close_frame_.quic_error_code,
+ IsError(QUIC_CRYPTO_TOO_MANY_ENTRIES));
+ } else {
+ EXPECT_QUIC_BUG(
+ connection_.CloseConnection(
+ QUIC_CRYPTO_TOO_MANY_ENTRIES, "Closed by test",
+ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET),
+ // 30=QUIC_CRYPTO_TOO_MANY_ENTRIES, 27=QUIC_PACKET_WRITE_ERROR.
+ "Initial error code: 30, new error code: 27");
+ }
+}
+
// These two tests ensure that the QuicErrorCode mapping works correctly.
// Both tests expect to see a Google QUIC close if not running IETF QUIC.
// If running IETF QUIC, the first will generate a transport connection
