gfe-relnote: Close QUIC connection is there are too many (> 1000) buffered control frames in control frame manager. Protected by gfe2_reloadable_flag_quic_add_upper_limit_of_buffered_control_frames. PiperOrigin-RevId: 261002263 Change-Id: I4dd92d5c1ea159d05cf35719f1a7ccc36f7b3fd3
diff --git a/quic/core/quic_control_frame_manager.cc b/quic/core/quic_control_frame_manager.cc index 511ef2b..ac4488b 100644 --- a/quic/core/quic_control_frame_manager.cc +++ b/quic/core/quic_control_frame_manager.cc
@@ -14,11 +14,25 @@ namespace quic { +namespace { + +// The maximum number of buffered control frames which are waiting to be ACKed +// or sent for the first time. +const size_t kMaxNumControlFrames = 1000; + +} // namespace + QuicControlFrameManager::QuicControlFrameManager(QuicSession* session) : last_control_frame_id_(kInvalidControlFrameId), least_unacked_(1), least_unsent_(1), - session_(session) {} + session_(session), + add_upper_limit_(GetQuicReloadableFlag( + quic_add_upper_limit_of_buffered_control_frames)) { + if (add_upper_limit_) { + QUIC_RELOADABLE_FLAG_COUNT(quic_add_upper_limit_of_buffered_control_frames); + } +} QuicControlFrameManager::~QuicControlFrameManager() { while (!control_frames_.empty()) { @@ -30,6 +44,15 @@ void QuicControlFrameManager::WriteOrBufferQuicFrame(QuicFrame frame) { const bool had_buffered_frames = HasBufferedFrames(); control_frames_.emplace_back(frame); + if (add_upper_limit_ && control_frames_.size() > kMaxNumControlFrames) { + session_->connection()->CloseConnection( + QUIC_TOO_MANY_BUFFERED_CONTROL_FRAMES, + QuicStrCat("More than ", kMaxNumControlFrames, + "buffered control frames, least_unacked: ", least_unacked_, + ", least_unsent_: ", least_unsent_), + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return; + } if (had_buffered_frames) { return; } @@ -101,6 +124,15 @@ } control_frames_.emplace_back( QuicFrame(QuicPingFrame(++last_control_frame_id_))); + if (add_upper_limit_ && control_frames_.size() > kMaxNumControlFrames) { + session_->connection()->CloseConnection( + QUIC_TOO_MANY_BUFFERED_CONTROL_FRAMES, + QuicStrCat("More than ", kMaxNumControlFrames, + "buffered control frames, least_unacked: ", least_unacked_, + ", least_unsent_: ", least_unsent_), + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return; + } WriteBufferedFrames(); } @@ -172,6 +204,9 @@ } if (!QuicContainsKey(pending_retransmissions_, id)) { pending_retransmissions_[id] = true; + QUIC_BUG_IF(pending_retransmissions_.size() > control_frames_.size()) + << "least_unacked_: " << least_unacked_ + << ", least_unsent_: " << least_unsent_; } }
diff --git a/quic/core/quic_control_frame_manager.h b/quic/core/quic_control_frame_manager.h index a4c2678..54ca479 100644 --- a/quic/core/quic_control_frame_manager.h +++ b/quic/core/quic_control_frame_manager.h
@@ -146,6 +146,9 @@ // Last sent window update frame for each stream. QuicSmallMap<QuicStreamId, QuicControlFrameId, 10> window_update_frames_; + + // Latched value of quic_add_upper_limit_of_buffered_control_frames. + const bool add_upper_limit_; }; } // namespace quic
diff --git a/quic/core/quic_control_frame_manager_test.cc b/quic/core/quic_control_frame_manager_test.cc index 216ba74..8059ff6 100644 --- a/quic/core/quic_control_frame_manager_test.cc +++ b/quic/core/quic_control_frame_manager_test.cc
@@ -4,6 +4,7 @@ #include "net/third_party/quiche/src/quic/core/quic_control_frame_manager.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h" #include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" #include "net/third_party/quiche/src/quic/platform/api/quic_ptr_util.h" #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" @@ -288,6 +289,27 @@ EXPECT_FALSE(manager_->WillingToWrite()); } +TEST_F(QuicControlFrameManagerTest, TooManyBufferedControlFrames) { + SetQuicReloadableFlag(quic_add_upper_limit_of_buffered_control_frames, true); + Initialize(); + EXPECT_CALL(*connection_, SendControlFrame(_)) + .Times(5) + .WillRepeatedly(Invoke(&ClearControlFrame)); + // Flush buffered frames. + manager_->OnCanWrite(); + // Write 995 control frames. + EXPECT_CALL(*connection_, SendControlFrame(_)).WillOnce(Return(false)); + for (size_t i = 0; i < 995; ++i) { + manager_->WriteOrBufferRstStream(kTestStreamId, QUIC_STREAM_CANCELLED, 0); + } + // Verify write one more control frame causes connection close. + EXPECT_CALL( + *connection_, + CloseConnection(QUIC_TOO_MANY_BUFFERED_CONTROL_FRAMES, _, + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET)); + manager_->WriteOrBufferRstStream(kTestStreamId, QUIC_STREAM_CANCELLED, 0); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quic/core/quic_error_codes.cc b/quic/core/quic_error_codes.cc index f13eec4..1be1786 100644 --- a/quic/core/quic_error_codes.cc +++ b/quic/core/quic_error_codes.cc
@@ -157,6 +157,7 @@ RETURN_STRING_LITERAL(QUIC_IETF_GQUIC_ERROR_MISSING); RETURN_STRING_LITERAL( QUIC_WINDOW_UPDATE_RECEIVED_ON_READ_UNIDIRECTIONAL_STREAM); + RETURN_STRING_LITERAL(QUIC_TOO_MANY_BUFFERED_CONTROL_FRAMES); RETURN_STRING_LITERAL(QUIC_LAST_ERROR); // Intentionally have no default case, so we'll break the build
diff --git a/quic/core/quic_error_codes.h b/quic/core/quic_error_codes.h index abc666d..534d0b5 100644 --- a/quic/core/quic_error_codes.h +++ b/quic/core/quic_error_codes.h
@@ -334,8 +334,11 @@ // Received WindowUpdate on a READ_UNIDIRECTIONAL stream. QUIC_WINDOW_UPDATE_RECEIVED_ON_READ_UNIDIRECTIONAL_STREAM = 123, + // There are too many buffered control frames in control frame manager. + QUIC_TOO_MANY_BUFFERED_CONTROL_FRAMES = 124, + // No error. Used as bound while iterating. - QUIC_LAST_ERROR = 124, + QUIC_LAST_ERROR = 125, }; // QuicErrorCodes is encoded as a single octet on-the-wire. static_assert(static_cast<int>(QUIC_LAST_ERROR) <=