gfe-relnote: In QUIC, break QuicStream::CloseRWSide -> QuicSession::CloseStream -> QuicStream::OnClose -> CloseRWSide loop. Protected by gfe2_reloadable_flag_quic_break_session_stream_close_loop.
With this change, a stream can be closed by 4 methods:
1) QuicSession::CloseStream (will soon be removed)
2) QuicSession::ResetStream
3) QuicStream::Reset
4) QuicStream::OnStreamReset
And also make the code path consistent. All -> QuicStream::CloseReadSide/CloseWriteSide -> QuicSession::OnStreamClosed.
PiperOrigin-RevId: 307032801
Change-Id: I1a2f49ddb94cc9642ce8c8fcb514f5adb928d045
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index a4d6cd7..4d02c80 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -2027,7 +2027,11 @@
// Transmit the cancel, and ensure the connection is torn down properly.
SetPacketLossPercentage(0);
QuicStreamId stream_id = GetNthClientInitiatedBidirectionalId(0);
- session->SendRstStream(stream_id, QUIC_STREAM_CANCELLED, 0);
+ if (session->break_close_loop()) {
+ session->ResetStream(stream_id, QUIC_STREAM_CANCELLED, 0);
+ } else {
+ session->SendRstStream(stream_id, QUIC_STREAM_CANCELLED, 0);
+ }
// WaitForEvents waits 50ms and returns true if there are outstanding
// requests.
diff --git a/quic/core/http/quic_client_promised_info_test.cc b/quic/core/http/quic_client_promised_info_test.cc
index 27d9fe8..381160b 100644
--- a/quic/core/http/quic_client_promised_info_test.cc
+++ b/quic/core/http/quic_client_promised_info_test.cc
@@ -227,7 +227,9 @@
EXPECT_CALL(*connection_, SendControlFrame(_));
EXPECT_CALL(*connection_,
OnStreamReset(promise_id_, QUIC_PROMISE_VARY_MISMATCH));
- EXPECT_CALL(session_, CloseStream(promise_id_));
+ if (!session_.break_close_loop()) {
+ EXPECT_CALL(session_, CloseStream(promise_id_));
+ }
promised->HandleClientRequest(client_request_, &delegate);
}
@@ -303,7 +305,9 @@
session_.GetOrCreateStream(promise_id_);
// Cancel the promised stream.
- EXPECT_CALL(session_, CloseStream(promise_id_));
+ if (!session_.break_close_loop()) {
+ EXPECT_CALL(session_, CloseStream(promise_id_));
+ }
EXPECT_CALL(*connection_, SendControlFrame(_));
EXPECT_CALL(*connection_, OnStreamReset(promise_id_, QUIC_STREAM_CANCELLED));
promised->Cancel();
@@ -327,11 +331,17 @@
promise_stream->OnStreamHeaderList(false, headers.uncompressed_header_bytes(),
headers);
- EXPECT_CALL(session_, CloseStream(promise_id_));
+ if (!session_.break_close_loop()) {
+ EXPECT_CALL(session_, CloseStream(promise_id_));
+ }
EXPECT_CALL(*connection_, SendControlFrame(_));
EXPECT_CALL(*connection_,
OnStreamReset(promise_id_, QUIC_STREAM_PEER_GOING_AWAY));
- session_.SendRstStream(promise_id_, QUIC_STREAM_PEER_GOING_AWAY, 0);
+ if (session_.break_close_loop()) {
+ session_.ResetStream(promise_id_, QUIC_STREAM_PEER_GOING_AWAY, 0);
+ } else {
+ session_.SendRstStream(promise_id_, QUIC_STREAM_PEER_GOING_AWAY, 0);
+ }
// Now initiate rendezvous.
TestPushPromiseDelegate delegate(/*match=*/true);
diff --git a/quic/core/http/quic_spdy_client_session_base.cc b/quic/core/http/quic_spdy_client_session_base.cc
index 2655259..63de57c 100644
--- a/quic/core/http/quic_spdy_client_session_base.cc
+++ b/quic/core/http/quic_spdy_client_session_base.cc
@@ -204,7 +204,11 @@
QuicStreamId id,
QuicRstStreamErrorCode error_code) {
DCHECK(QuicUtils::IsServerInitiatedStreamId(transport_version(), id));
- SendRstStream(id, error_code, 0);
+ if (break_close_loop()) {
+ ResetStream(id, error_code, 0);
+ } else {
+ SendRstStream(id, error_code, 0);
+ }
if (!IsOpenStream(id) && !IsClosedStream(id)) {
MaybeIncreaseLargestPeerStreamId(id);
}
@@ -218,6 +222,14 @@
}
}
+void QuicSpdyClientSessionBase::OnStreamClosed(QuicStreamId stream_id) {
+ DCHECK(break_close_loop());
+ QuicSpdySession::OnStreamClosed(stream_id);
+ if (!VersionUsesHttp3(transport_version())) {
+ headers_stream()->MaybeReleaseSequencerBuffer();
+ }
+}
+
bool QuicSpdyClientSessionBase::ShouldReleaseHeadersStreamSequencerBuffer() {
return !HasActiveRequestStreams() && promised_by_id_.empty();
}
diff --git a/quic/core/http/quic_spdy_client_session_base.h b/quic/core/http/quic_spdy_client_session_base.h
index 9d702e3..282a8cf 100644
--- a/quic/core/http/quic_spdy_client_session_base.h
+++ b/quic/core/http/quic_spdy_client_session_base.h
@@ -105,6 +105,9 @@
// Release headers stream's sequencer buffer if it's empty.
void CloseStreamInner(QuicStreamId stream_id, bool rst_sent) override;
+ // Release headers stream's sequencer buffer if it's empty.
+ void OnStreamClosed(QuicStreamId stream_id) override;
+
// Returns true if there are no active requests and no promised streams.
bool ShouldReleaseHeadersStreamSequencerBuffer() override;
diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc
index 65dd43a..dc72a73 100644
--- a/quic/core/http/quic_spdy_client_session_test.cc
+++ b/quic/core/http/quic_spdy_client_session_test.cc
@@ -354,7 +354,11 @@
.Times(AtLeast(1))
.WillRepeatedly(Invoke(&ClearControlFrame));
EXPECT_CALL(*connection_, OnStreamReset(_, _)).Times(1);
- session_->SendRstStream(stream_id, QUIC_STREAM_PEER_GOING_AWAY, 0);
+ if (session_->break_close_loop()) {
+ session_->ResetStream(stream_id, QUIC_STREAM_PEER_GOING_AWAY, 0);
+ } else {
+ session_->SendRstStream(stream_id, QUIC_STREAM_PEER_GOING_AWAY, 0);
+ }
// A new stream cannot be created as the reset stream still counts as an open
// outgoing stream until closed by the server.
@@ -406,7 +410,11 @@
.Times(AtLeast(1))
.WillRepeatedly(Invoke(&ClearControlFrame));
EXPECT_CALL(*connection_, OnStreamReset(_, _)).Times(1);
- session_->SendRstStream(stream_id, QUIC_STREAM_PEER_GOING_AWAY, 0);
+ if (session_->break_close_loop()) {
+ session_->ResetStream(stream_id, QUIC_STREAM_PEER_GOING_AWAY, 0);
+ } else {
+ session_->SendRstStream(stream_id, QUIC_STREAM_PEER_GOING_AWAY, 0);
+ }
// The stream receives trailers with final byte offset, but the header value
// is non-numeric and should be treated as malformed.
@@ -861,7 +869,12 @@
EXPECT_CALL(*connection_, SendControlFrame(_));
EXPECT_CALL(*connection_,
OnStreamReset(promised_stream_id_, QUIC_STREAM_PEER_GOING_AWAY));
- session_->SendRstStream(promised_stream_id_, QUIC_STREAM_PEER_GOING_AWAY, 0);
+ if (session_->break_close_loop()) {
+ session_->ResetStream(promised_stream_id_, QUIC_STREAM_PEER_GOING_AWAY, 0);
+ } else {
+ session_->SendRstStream(promised_stream_id_, QUIC_STREAM_PEER_GOING_AWAY,
+ 0);
+ }
QuicClientPromisedInfo* promised =
session_->GetPromisedById(promised_stream_id_);
EXPECT_NE(promised, nullptr);
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index f46d914..9db9f22 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -317,6 +317,7 @@
&helper_, &alarm_factory_, perspective, SupportedVersions(GetParam()));
session_ = std::make_unique<StrictMock<TestSession>>(connection_);
session_->Initialize();
+ connection_->AdvanceTime(QuicTime::Delta::FromSeconds(1));
ON_CALL(*session_, WritevData(_, _, _, _, _, _))
.WillByDefault(
Invoke(session_.get(), &MockQuicSpdySession::ConsumeData));
