Use maximum stream ID in GOAWAY frame for graceful shutdown.
We are aware of HTTP/2 clients that fail outstanding requests upon GOAWAY
instead of retrying them, and because of this we send max stream ID instead of
largest processed stream ID in HTTP/2. In case a similar issue exists or will
develop for QUIC, the only time a GOAWAY frame with a stream ID less than the
maximum possible stream ID is sent should be right before closing the
connection. This also allows us not to implement resetting incoming streams
with stream ID larger than sent in a GOAWAY frame.
Protected by FLAGS_quic_reloadable_flag_quic_goaway_with_max_stream_id.
PiperOrigin-RevId: 341835202
Change-Id: I8a45511d2d8f617de32ba3b5e55eb93bce26e75b
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index 0adcdc7..581a4fa 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -406,7 +406,12 @@
ietf_server_push_enabled_(
GetQuicFlag(FLAGS_quic_enable_http3_server_push)),
http3_max_push_id_sent_(false),
- reject_spdy_settings_(GetQuicReloadableFlag(quic_reject_spdy_settings)) {
+ reject_spdy_settings_(GetQuicReloadableFlag(quic_reject_spdy_settings)),
+ goaway_with_max_stream_id_(
+ GetQuicReloadableFlag(quic_goaway_with_max_stream_id)) {
+ if (goaway_with_max_stream_id_) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_goaway_with_max_stream_id, 1, 2);
+ }
h2_deframer_.set_visitor(spdy_framer_visitor_.get());
h2_deframer_.set_debug_visitor(spdy_framer_visitor_.get());
spdy_framer_.set_debug_visitor(spdy_framer_visitor_.get());
@@ -714,26 +719,46 @@
DCHECK_EQ(perspective(), Perspective::IS_SERVER);
DCHECK(VersionUsesHttp3(transport_version()));
- QuicStreamId stream_id =
- GetLargestPeerCreatedStreamId(/*unidirectional = */ false);
+ QuicStreamId stream_id;
- if (GetQuicReloadableFlag(quic_fix_http3_goaway_stream_id)) {
- QUIC_RELOADABLE_FLAG_COUNT(quic_fix_http3_goaway_stream_id);
- if (stream_id == QuicUtils::GetInvalidStreamId(transport_version())) {
- // No client-initiated bidirectional streams received yet.
- // Send 0 to let client know that all requests can be retried.
- stream_id = 0;
- } else {
- // Tell client that streams starting with the next after the largest
- // received one can be retried.
- stream_id += QuicUtils::StreamIdDelta(transport_version());
+ if (goaway_with_max_stream_id_) {
+ stream_id = QuicUtils::GetMaxClientInitiatedBidirectionalStreamId(
+ transport_version());
+ if (last_sent_http3_goaway_id_.has_value()) {
+ if (last_sent_http3_goaway_id_.value() == stream_id) {
+ // Do not send GOAWAY twice.
+ return;
+ }
+ if (last_sent_http3_goaway_id_.value() < stream_id) {
+ // A previous GOAWAY frame was sent with smaller stream ID. This is not
+ // possible, because the only time a GOAWAY frame with non-maximal
+ // stream ID is sent is right before closing connection.
+ QUIC_BUG << "GOAWAY frame with smaller ID already sent.";
+ return;
+ }
}
- if (last_sent_http3_goaway_id_.has_value() &&
- last_sent_http3_goaway_id_.value() <= stream_id) {
- // MUST not send GOAWAY with identifier larger than previously sent.
- // Do not bother sending one with same identifier as before, since GOAWAY
- // frames on the control stream are guaranteed to be processed in order.
- return;
+ } else {
+ stream_id = GetLargestPeerCreatedStreamId(/*unidirectional = */ false);
+
+ if (GetQuicReloadableFlag(quic_fix_http3_goaway_stream_id)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_fix_http3_goaway_stream_id);
+ if (stream_id == QuicUtils::GetInvalidStreamId(transport_version())) {
+ // No client-initiated bidirectional streams received yet.
+ // Send 0 to let client know that all requests can be retried.
+ stream_id = 0;
+ } else {
+ // Tell client that streams starting with the next after the largest
+ // received one can be retried.
+ stream_id += QuicUtils::StreamIdDelta(transport_version());
+ }
+ if (last_sent_http3_goaway_id_.has_value() &&
+ last_sent_http3_goaway_id_.value() <= stream_id) {
+ // MUST not send GOAWAY with identifier larger than previously sent.
+ // Do not bother sending one with same identifier as before, since
+ // GOAWAY frames on the control stream are guaranteed to be processed in
+ // order.
+ return;
+ }
}
}
@@ -742,6 +767,11 @@
}
void QuicSpdySession::SendHttp3Shutdown() {
+ if (goaway_with_max_stream_id_) {
+ SendHttp3GoAway();
+ return;
+ }
+
DCHECK_EQ(perspective(), Perspective::IS_SERVER);
DCHECK(VersionUsesHttp3(transport_version()));
QuicStreamCount advertised_max_incoming_bidirectional_streams =
@@ -1362,11 +1392,47 @@
}
void QuicSpdySession::BeforeConnectionCloseSent() {
- if (GetQuicReloadableFlag(quic_send_goaway_with_connection_close) &&
- VersionUsesHttp3(transport_version()) && IsEncryptionEstablished()) {
- QUIC_CODE_COUNT(quic_send_goaway_with_connection_close);
- SendHttp3GoAway();
+ if (!GetQuicReloadableFlag(quic_send_goaway_with_connection_close) ||
+ !VersionUsesHttp3(transport_version()) || !IsEncryptionEstablished()) {
+ return;
}
+
+ DCHECK_EQ(perspective(), Perspective::IS_SERVER);
+
+ QUIC_CODE_COUNT(quic_send_goaway_with_connection_close);
+
+ QuicStreamId stream_id =
+ GetLargestPeerCreatedStreamId(/*unidirectional = */ false);
+
+ if (GetQuicReloadableFlag(quic_fix_http3_goaway_stream_id)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_fix_http3_goaway_stream_id);
+ if (stream_id == QuicUtils::GetInvalidStreamId(transport_version())) {
+ // No client-initiated bidirectional streams received yet.
+ // Send 0 to let client know that all requests can be retried.
+ stream_id = 0;
+ } else {
+ // Tell client that streams starting with the next after the largest
+ // received one can be retried.
+ stream_id += QuicUtils::StreamIdDelta(transport_version());
+ }
+ if (last_sent_http3_goaway_id_.has_value() &&
+ last_sent_http3_goaway_id_.value() <= stream_id) {
+ if (goaway_with_max_stream_id_) {
+ // A previous GOAWAY frame was sent with smaller stream ID. This is not
+ // possible, because this is the only method sending a GOAWAY frame with
+ // non-maximal stream ID, and this must only be called once, right
+ // before closing connection.
+ QUIC_BUG << "GOAWAY frame with smaller ID already sent.";
+ }
+ // MUST not send GOAWAY with identifier larger than previously sent.
+ // Do not bother sending one with same identifier as before, since GOAWAY
+ // frames on the control stream are guaranteed to be processed in order.
+ return;
+ }
+ }
+
+ send_control_stream_->SendGoAway(stream_id);
+ last_sent_http3_goaway_id_ = stream_id;
}
void QuicSpdySession::OnCanCreateNewOutgoingStream(bool unidirectional) {
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h
index e3a3f79..5671901 100644
--- a/quic/core/http/quic_spdy_session.h
+++ b/quic/core/http/quic_spdy_session.h
@@ -227,15 +227,14 @@
// Send GOAWAY if the peer is blocked on the implementation max.
bool OnStreamsBlockedFrame(const QuicStreamsBlockedFrame& frame) override;
- // Write GOAWAY frame on the control stream to notify the client that every
- // stream that has not reached the server yet can be retried. Do not send a
- // GOAWAY frame if it could not convey new information to the client with
- // respect to the previous GOAWAY frame.
+ // Write GOAWAY frame with maximum stream ID on the control stream. Called to
+ // initite graceful connection shutdown. Do not use smaller stream ID, in
+ // case client does not implement retry on GOAWAY. Do not send GOAWAY if one
+ // has already been sent.
void SendHttp3GoAway();
- // Write advisory GOAWAY frame on the control stream with the max stream ID
- // that the client could send. If GOAWAY has already been sent, the lesser of
- // its max stream ID and the one advertised via MAX_STREAMS is used.
+ // Same as SendHttp3GoAway(). TODO(bnc): remove when
+ // gfe2_reloadable_flag_quic_goaway_with_max_stream_id flag is deprecated.
void SendHttp3Shutdown();
// Write |headers| for |promised_stream_id| on |original_stream_id| in a
@@ -606,6 +605,9 @@
// Latched value of reloadable flag quic_reject_spdy_settings.
const bool reject_spdy_settings_;
+
+ // Latched value of reloadable flag quic_goaway_with_max_stream_id.
+ const bool goaway_with_max_stream_id_;
};
} // namespace quic
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index 4eb3e5c..434178d 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -1110,17 +1110,27 @@
EXPECT_CALL(*writer_, WritePacket(_, _, _, _, _))
.WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0)));
- // No client-initiated stream has been received, therefore a GOAWAY frame with
- // stream ID = 0 is sent to notify the client that all requests can be retried
- // on a different connection.
- EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 0));
+ if (GetQuicReloadableFlag(quic_goaway_with_max_stream_id)) {
+ // Send max stream id (currently 32 bits).
+ EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 0xfffffffc));
+ } else {
+ // No client-initiated stream has been received, therefore a GOAWAY frame
+ // with stream ID = 0 is sent to notify the client that all requests can be
+ // retried on a different connection.
+ EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 0));
+ }
session_.SendHttp3GoAway();
EXPECT_TRUE(session_.goaway_sent());
+ // New incoming stream is not reset.
const QuicStreamId kTestStreamId =
GetNthClientInitiatedBidirectionalStreamId(transport_version(), 0);
EXPECT_CALL(*connection_, OnStreamReset(kTestStreamId, _)).Times(0);
EXPECT_TRUE(session_.GetOrCreateStream(kTestStreamId));
+
+ // No more GOAWAY frames are sent because they could not convey new
+ // information to the client.
+ session_.SendHttp3GoAway();
}
TEST_P(QuicSpdySessionTestServer, SendHttp3GoAwayAfterStreamIsCreated) {
@@ -1142,10 +1152,15 @@
EXPECT_CALL(*writer_, WritePacket(_, _, _, _, _))
.WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0)));
- // The first stream, of kTestStreamId = 0, could already have been processed.
- // A GOAWAY frame is sent to notify the client that requests starting with
- // stream ID = 4 can be retried on a different connection.
- EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 4));
+ if (GetQuicReloadableFlag(quic_goaway_with_max_stream_id)) {
+ // Send max stream id (currently 32 bits).
+ EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 0xfffffffc));
+ } else {
+ // The first stream, of kTestStreamId = 0, could already have been
+ // processed. A GOAWAY frame is sent to notify the client that requests
+ // starting with stream ID = 4 can be retried on a different connection.
+ EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 4));
+ }
session_.SendHttp3GoAway();
EXPECT_TRUE(session_.goaway_sent());
@@ -1155,6 +1170,10 @@
}
TEST_P(QuicSpdySessionTestServer, SendHttp3Shutdown) {
+ if (GetQuicReloadableFlag(quic_goaway_with_max_stream_id)) {
+ return;
+ }
+
if (!VersionUsesHttp3(transport_version())) {
return;
}
@@ -1176,6 +1195,10 @@
}
TEST_P(QuicSpdySessionTestServer, SendHttp3GoAwayAfterShutdownNotice) {
+ if (GetQuicReloadableFlag(quic_goaway_with_max_stream_id)) {
+ return;
+ }
+
if (!VersionUsesHttp3(transport_version())) {
return;
}
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 5590db2..ca6a3e1 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -56,6 +56,7 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_undecryptable_packets2, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_willing_and_able_to_write2, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_give_sent_packet_to_debug_visitor_after_sent, true)
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_goaway_with_max_stream_id, false)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_granular_qpack_error_codes, false)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_key_update_supported, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_let_connection_handle_pings, true)
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index 983f862..6b4c517 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -921,8 +921,19 @@
return;
}
transport_goaway_sent_ = true;
- control_frame_manager_.WriteOrBufferGoAway(
- error_code, stream_id_manager_.largest_peer_created_stream_id(), reason);
+ if (GetQuicReloadableFlag(quic_goaway_with_max_stream_id)) {
+ DCHECK_EQ(perspective(), Perspective::IS_SERVER);
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_goaway_with_max_stream_id, 2, 2);
+ control_frame_manager_.WriteOrBufferGoAway(
+ error_code,
+ QuicUtils::GetMaxClientInitiatedBidirectionalStreamId(
+ transport_version()),
+ reason);
+ } else {
+ control_frame_manager_.WriteOrBufferGoAway(
+ error_code, stream_id_manager_.largest_peer_created_stream_id(),
+ reason);
+ }
}
void QuicSession::SendBlocked(QuicStreamId id) {
diff --git a/quic/core/quic_utils.cc b/quic/core/quic_utils.cc
index 9def438..3322fed 100644
--- a/quic/core/quic_utils.cc
+++ b/quic/core/quic_utils.cc
@@ -494,6 +494,18 @@
}
// static
+QuicStreamId QuicUtils::GetMaxClientInitiatedBidirectionalStreamId(
+ QuicTransportVersion version) {
+ if (VersionHasIetfQuicFrames(version)) {
+ // Client initiated bidirectional streams have stream IDs divisible by 4.
+ return std::numeric_limits<QuicStreamId>::max() - 3;
+ }
+
+ // Client initiated bidirectional streams have odd stream IDs.
+ return std::numeric_limits<QuicStreamId>::max();
+}
+
+// static
QuicConnectionId QuicUtils::CreateReplacementConnectionId(
const QuicConnectionId& connection_id) {
return CreateReplacementConnectionId(connection_id,
diff --git a/quic/core/quic_utils.h b/quic/core/quic_utils.h
index c6f5ca0..c1a19a8 100644
--- a/quic/core/quic_utils.h
+++ b/quic/core/quic_utils.h
@@ -170,6 +170,10 @@
QuicTransportVersion version,
Perspective perspective);
+ // Returns the largest possible client initiated bidirectional stream ID.
+ static QuicStreamId GetMaxClientInitiatedBidirectionalStreamId(
+ QuicTransportVersion version);
+
// Generates a connection ID of length |expected_connection_id_length|
// derived from |connection_id|.
// This is guaranteed to be deterministic (calling this method with two
