Avoid nested CloseConnection calls in QuicConnection.
Protected by quic_reloadable_flag_quic_avoid_nested_close_connection.
PiperOrigin-RevId: 689062814
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
