deprecate gfe2_reloadable_flag_quic_remove_zombie_streams.
PiperOrigin-RevId: 334253770
Change-Id: I52556e52b468948510cc10d62bf7c679ee4f8505
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index 50b7904..7a0bd9a 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -3162,7 +3162,6 @@
QuicSession* session = GetClientSession();
ASSERT_TRUE(session);
// Verify canceled stream does not become zombie.
- EXPECT_TRUE(QuicSessionPeer::zombie_streams(session).empty());
EXPECT_EQ(1u, QuicSessionPeer::closed_streams(session).size());
}
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index 1a6ecb0..0f806f7 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -426,10 +426,6 @@
for (auto& stream : *closed_streams()) {
static_cast<QuicSpdyStream*>(stream.get())->ClearSession();
}
- DCHECK(!remove_zombie_streams() || zombie_streams().empty());
- for (auto const& kv : zombie_streams()) {
- static_cast<QuicSpdyStream*>(kv.second.get())->ClearSession();
- }
for (auto const& kv : stream_map()) {
if (!kv.second->is_static()) {
static_cast<QuicSpdyStream*>(kv.second.get())->ClearSession();
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index 15b56f7..4737902 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -100,9 +100,7 @@
is_configured_(false),
enable_round_robin_scheduling_(false),
was_zero_rtt_rejected_(false),
- liveness_testing_in_progress_(false),
- remove_zombie_streams_(
- GetQuicReloadableFlag(quic_remove_zombie_streams)) {
+ liveness_testing_in_progress_(false) {
closed_streams_clean_up_alarm_ =
QuicWrapUnique<QuicAlarm>(connection_->alarm_factory()->CreateAlarm(
new ClosedStreamsCleanUpDelegate(this)));
@@ -147,12 +145,7 @@
GetMutableCryptoStream()->id());
}
-QuicSession::~QuicSession() {
- if (!remove_zombie_streams_) {
- QUIC_LOG_IF(WARNING, !zombie_streams_.empty())
- << "Still have zombie streams";
- }
-}
+QuicSession::~QuicSession() {}
void QuicSession::PendingStreamOnStreamFrame(const QuicStreamFrame& frame) {
DCHECK(VersionUsesHttp3(transport_version()));
@@ -395,30 +388,13 @@
stream->OnConnectionClosed(frame.quic_error_code, source);
auto it = stream_map_.find(id);
if (it != stream_map_.end()) {
- if (!remove_zombie_streams_) {
- QUIC_BUG << ENDPOINT << "Stream " << id
- << " failed to close under OnConnectionClosed";
- } else {
- QUIC_BUG_IF(!it->second->IsZombie())
- << ENDPOINT << "Non-zombie stream " << id
- << " failed to close under OnConnectionClosed";
- }
+ QUIC_BUG_IF(!it->second->IsZombie())
+ << ENDPOINT << "Non-zombie stream " << id
+ << " failed to close under OnConnectionClosed";
}
return true;
});
- if (!remove_zombie_streams_) {
- // Cleanup zombie stream map on connection close.
- while (!zombie_streams_.empty()) {
- ZombieStreamMap::iterator it = zombie_streams_.begin();
- closed_streams_.push_back(std::move(it->second));
- zombie_streams_.erase(it);
- }
- } else {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_remove_zombie_streams, 1, 4);
- DCHECK(zombie_streams_.empty());
- }
-
closed_streams_clean_up_alarm_->Cancel();
if (visitor_) {
@@ -918,20 +894,11 @@
const bool stream_waiting_for_acks = stream->IsWaitingForAcks();
if (stream_waiting_for_acks) {
- if (remove_zombie_streams_) {
- // The stream needs to be kept alive because it's waiting for acks.
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_remove_zombie_streams, 2, 4);
- ++num_zombie_streams_;
- } else {
- zombie_streams_[stream_id] = std::move(it->second);
- }
+ // The stream needs to be kept alive because it's waiting for acks.
+ ++num_zombie_streams_;
} else {
closed_streams_.push_back(std::move(it->second));
- if (remove_zombie_streams_) {
- // When zombie_streams_ is removed, stream is only erased from stream map
- // if it's not zombie.
- stream_map_.erase(it);
- }
+ stream_map_.erase(it);
// Do not retransmit data of a closed stream.
streams_with_pending_retransmission_.erase(stream_id);
if (!closed_streams_clean_up_alarm_->IsSet()) {
@@ -949,18 +916,12 @@
DCHECK(!stream->was_draining());
InsertLocallyClosedStreamsHighestOffset(
stream_id, stream->highest_received_byte_offset());
- if (!remove_zombie_streams_) {
- stream_map_.erase(it);
- }
return;
}
const bool stream_was_draining = stream->was_draining();
QUIC_DVLOG_IF(1, stream_was_draining)
<< ENDPOINT << "Stream " << stream_id << " was draining";
- if (!remove_zombie_streams_) {
- stream_map_.erase(it);
- }
if (stream_was_draining) {
QUIC_BUG_IF(num_draining_streams_ == 0);
--num_draining_streams_;
@@ -1804,11 +1765,7 @@
StreamMap::iterator it = stream_map_.find(stream_id);
if (it != stream_map_.end()) {
- if (remove_zombie_streams_ && it->second->IsZombie()) {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_remove_zombie_streams, 3, 4);
- return nullptr;
- }
- return it->second.get();
+ return it->second->IsZombie() ? nullptr : it->second.get();
}
if (IsClosedStream(stream_id)) {
@@ -1954,7 +1911,7 @@
DCHECK_NE(QuicUtils::GetInvalidStreamId(transport_version()), id);
const StreamMap::iterator it = stream_map_.find(id);
if (it != stream_map_.end()) {
- return remove_zombie_streams_ ? !it->second->IsZombie() : true;
+ return !it->second->IsZombie();
}
if (QuicContainsKey(pending_stream_map_, id) ||
QuicUtils::IsCryptoStreamId(transport_version(), id)) {
@@ -2046,24 +2003,13 @@
}
void QuicSession::OnStreamDoneWaitingForAcks(QuicStreamId id) {
- if (!remove_zombie_streams_) {
- auto it = zombie_streams_.find(id);
- if (it == zombie_streams_.end()) {
- return;
- }
-
- closed_streams_.push_back(std::move(it->second));
- zombie_streams_.erase(it);
- } else {
- QUIC_RELOADABLE_FLAG_COUNT_N(quic_remove_zombie_streams, 4, 4);
- auto it = stream_map_.find(id);
- if (it == stream_map_.end()) {
- return;
- }
- --num_zombie_streams_;
- closed_streams_.push_back(std::move(it->second));
- stream_map_.erase(it);
+ auto it = stream_map_.find(id);
+ if (it == stream_map_.end()) {
+ return;
}
+ --num_zombie_streams_;
+ closed_streams_.push_back(std::move(it->second));
+ stream_map_.erase(it);
if (!closed_streams_clean_up_alarm_->IsSet()) {
closed_streams_clean_up_alarm_->Set(connection_->clock()->ApproximateNow());
@@ -2078,12 +2024,6 @@
return active_stream->second.get();
}
- DCHECK(!remove_zombie_streams_ || zombie_streams_.empty());
- auto zombie_stream = zombie_streams_.find(id);
- if (zombie_stream != zombie_streams_.end()) {
- return zombie_stream->second.get();
- }
-
if (QuicUtils::IsCryptoStreamId(transport_version(), id)) {
return const_cast<QuicCryptoStream*>(GetCryptoStream());
}
@@ -2224,10 +2164,6 @@
}
bool QuicSession::HasUnackedStreamData() const {
- DCHECK(!remove_zombie_streams_ || zombie_streams_.empty());
- if (!zombie_streams().empty()) {
- return true;
- }
for (const auto& it : stream_map_) {
if (it.second->IsWaitingForAcks()) {
return true;
@@ -2475,8 +2411,7 @@
std::function<bool(QuicStream*)> action) {
std::vector<QuicStream*> active_streams;
for (const auto& it : stream_map_) {
- if (!it.second->is_static() &&
- (!remove_zombie_streams_ || !it.second->IsZombie())) {
+ if (!it.second->is_static() && !it.second->IsZombie()) {
active_streams.push_back(it.second.get());
}
}
@@ -2491,8 +2426,7 @@
void QuicSession::PerformActionOnActiveStreams(
std::function<bool(QuicStream*)> action) const {
for (const auto& it : stream_map_) {
- if (!it.second->is_static() &&
- (!remove_zombie_streams_ || !it.second->IsZombie()) &&
+ if (!it.second->is_static() && !it.second->IsZombie() &&
!action(it.second.get())) {
return;
}
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h
index 302a47a..f5e01f8 100644
--- a/quic/core/quic_session.h
+++ b/quic/core/quic_session.h
@@ -491,8 +491,6 @@
return liveness_testing_in_progress_;
}
- bool remove_zombie_streams() const { return remove_zombie_streams_; }
-
protected:
using StreamMap = QuicHashMap<QuicStreamId, std::unique_ptr<QuicStream>>;
@@ -566,8 +564,6 @@
ClosedStreams* closed_streams() { return &closed_streams_; }
- const ZombieStreamMap& zombie_streams() const { return zombie_streams_; }
-
void set_largest_peer_created_stream_id(
QuicStreamId largest_peer_created_stream_id);
@@ -749,9 +745,6 @@
QuicWriteBlockedList write_blocked_streams_;
ClosedStreams closed_streams_;
- // Streams which are closed, but need to be kept alive. Currently, the only
- // reason is the stream's sent data (including FIN) does not get fully acked.
- ZombieStreamMap zombie_streams_;
QuicConfig config_;
@@ -844,9 +837,6 @@
// This indicates a liveness testing is in progress, and push back the
// creation of new outgoing bidirectional streams.
bool liveness_testing_in_progress_;
-
- // Latched value of flag quic_remove_zombie_streams.
- const bool remove_zombie_streams_;
};
} // namespace quic
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc
index a5d9b5f..e5b6cff 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc
@@ -367,7 +367,6 @@
using QuicSession::GetNextOutgoingBidirectionalStreamId;
using QuicSession::GetNextOutgoingUnidirectionalStreamId;
using QuicSession::stream_map;
- using QuicSession::zombie_streams;
private:
StrictMock<TestCryptoStream> crypto_stream_;
@@ -2261,11 +2260,9 @@
EXPECT_TRUE(stream2->IsWaitingForAcks());
CloseStream(stream2->id());
- EXPECT_FALSE(QuicContainsKey(session_.zombie_streams(), stream2->id()));
ASSERT_EQ(1u, session_.closed_streams()->size());
EXPECT_EQ(stream2->id(), session_.closed_streams()->front()->id());
session_.OnStreamDoneWaitingForAcks(stream2->id());
- EXPECT_FALSE(QuicContainsKey(session_.zombie_streams(), stream2->id()));
EXPECT_EQ(1u, session_.closed_streams()->size());
EXPECT_EQ(stream2->id(), session_.closed_streams()->front()->id());
}
@@ -2320,7 +2317,6 @@
EXPECT_CALL(*connection_, CloseConnection(_, _, _)).Times(0);
session_.OnStopSendingFrame(frame);
}
- EXPECT_FALSE(QuicContainsKey(session_.zombie_streams(), stream2->id()));
ASSERT_EQ(1u, session_.closed_streams()->size());
EXPECT_EQ(stream2->id(), session_.closed_streams()->front()->id());
@@ -2342,7 +2338,6 @@
// QUIC it sends both a RST_STREAM and a STOP_SENDING (each of which
// closes in only one direction).
stream4->Reset(QUIC_STREAM_CANCELLED);
- EXPECT_FALSE(QuicContainsKey(session_.zombie_streams(), stream4->id()));
EXPECT_EQ(2u, session_.closed_streams()->size());
}
@@ -2590,13 +2585,9 @@
stream2->WriteOrBufferData(body, true, nullptr);
EXPECT_TRUE(stream2->IsWaitingForAcks());
// Verify stream2 is a zombie streams.
- if (!session_.remove_zombie_streams()) {
- EXPECT_TRUE(QuicContainsKey(session_.zombie_streams(), stream2->id()));
- } else {
- ASSERT_TRUE(QuicContainsKey(session_.stream_map(), stream2->id()));
- auto* stream = session_.stream_map().find(stream2->id())->second.get();
- EXPECT_TRUE(stream->IsZombie());
- }
+ ASSERT_TRUE(QuicContainsKey(session_.stream_map(), stream2->id()));
+ auto* stream = session_.stream_map().find(stream2->id())->second.get();
+ EXPECT_TRUE(stream->IsZombie());
QuicStreamFrame frame(stream2->id(), true, 0, 100);
EXPECT_CALL(*stream2, HasPendingRetransmission())
@@ -2610,7 +2601,6 @@
stream2->Reset(QUIC_STREAM_CANCELLED);
// Verify stream 2 gets closed.
- EXPECT_FALSE(QuicContainsKey(session_.zombie_streams(), stream2->id()));
EXPECT_TRUE(session_.IsClosedStream(stream2->id()));
EXPECT_CALL(*stream2, OnCanWrite()).Times(0);
session_.OnCanWrite();
@@ -2625,7 +2615,6 @@
EXPECT_FALSE(stream2->IsWaitingForAcks());
CloseStream(stream2->id());
- EXPECT_FALSE(QuicContainsKey(session_.zombie_streams(), stream2->id()));
EXPECT_EQ(1u, session_.closed_streams()->size());
EXPECT_TRUE(
QuicSessionPeer::GetCleanUpClosedStreamsAlarm(&session_)->IsSet());
@@ -2642,15 +2631,10 @@
session_.ActivateStream(QuicWrapUnique(stream4));
std::string body(100, '.');
stream4->WriteOrBufferData(body, false, nullptr);
- EXPECT_FALSE(QuicContainsKey(session_.zombie_streams(), stream4->id()));
stream4->WriteOrBufferData(body, true, nullptr);
- if (!session_.remove_zombie_streams()) {
- EXPECT_TRUE(QuicContainsKey(session_.zombie_streams(), stream4->id()));
- } else {
- ASSERT_TRUE(QuicContainsKey(session_.stream_map(), stream4->id()));
- auto* stream = session_.stream_map().find(stream4->id())->second.get();
- EXPECT_TRUE(stream->IsZombie());
- }
+ ASSERT_TRUE(QuicContainsKey(session_.stream_map(), stream4->id()));
+ auto* stream = session_.stream_map().find(stream4->id())->second.get();
+ EXPECT_TRUE(stream->IsZombie());
}
TEST_P(QuicSessionTestServer, ReceivedDataOnWriteUnidirectionalStream) {
diff --git a/quic/core/quic_stream.cc b/quic/core/quic_stream.cc
index 9849239..d973549 100644
--- a/quic/core/quic_stream.cc
+++ b/quic/core/quic_stream.cc
@@ -1018,8 +1018,7 @@
fin_outstanding_ = false;
fin_lost_ = false;
}
- if (!IsWaitingForAcks() && (!session()->remove_zombie_streams() ||
- (read_side_closed_ && write_side_closed_))) {
+ if (!IsWaitingForAcks() && read_side_closed_ && write_side_closed_) {
session_->OnStreamDoneWaitingForAcks(id_);
}
return new_data_acked;
diff --git a/quic/quic_transport/quic_transport_client_session_test.cc b/quic/quic_transport/quic_transport_client_session_test.cc
index 9c2f138..fe85576 100644
--- a/quic/quic_transport/quic_transport_client_session_test.cc
+++ b/quic/quic_transport/quic_transport_client_session_test.cc
@@ -109,15 +109,9 @@
EXPECT_TRUE(session_->IsSessionReady());
QuicStream* client_indication_stream;
- if (session_->remove_zombie_streams()) {
- client_indication_stream =
- QuicSessionPeer::stream_map(session_.get())[ClientIndicationStream()]
- .get();
- } else {
- client_indication_stream = QuicSessionPeer::zombie_streams(
- session_.get())[ClientIndicationStream()]
- .get();
- }
+ client_indication_stream =
+ QuicSessionPeer::stream_map(session_.get())[ClientIndicationStream()]
+ .get();
ASSERT_TRUE(client_indication_stream != nullptr);
const std::string client_indication = DataInStream(client_indication_stream);
const std::string expected_client_indication{
@@ -141,15 +135,9 @@
EXPECT_TRUE(session_->IsSessionReady());
QuicStream* client_indication_stream;
- if (session_->remove_zombie_streams()) {
- client_indication_stream =
- QuicSessionPeer::stream_map(session_.get())[ClientIndicationStream()]
- .get();
- } else {
- client_indication_stream = QuicSessionPeer::zombie_streams(
- session_.get())[ClientIndicationStream()]
- .get();
- }
+ client_indication_stream =
+ QuicSessionPeer::stream_map(session_.get())[ClientIndicationStream()]
+ .get();
ASSERT_TRUE(client_indication_stream != nullptr);
const std::string client_indication = DataInStream(client_indication_stream);
const std::string expected_client_indication{
diff --git a/quic/test_tools/quic_session_peer.cc b/quic/test_tools/quic_session_peer.cc
index 6823691..05d74d7 100644
--- a/quic/test_tools/quic_session_peer.cc
+++ b/quic/test_tools/quic_session_peer.cc
@@ -137,12 +137,6 @@
}
// static
-QuicSession::ZombieStreamMap& QuicSessionPeer::zombie_streams(
- QuicSession* session) {
- return session->zombie_streams_;
-}
-
-// static
void QuicSessionPeer::ActivateStream(QuicSession* session,
std::unique_ptr<QuicStream> stream) {
return session->ActivateStream(std::move(stream));
diff --git a/quic/test_tools/quic_session_peer.h b/quic/test_tools/quic_session_peer.h
index 1366ddb..ffd6c6f 100644
--- a/quic/test_tools/quic_session_peer.h
+++ b/quic/test_tools/quic_session_peer.h
@@ -58,7 +58,6 @@
GetLocallyClosedStreamsHighestOffset(QuicSession* session);
static QuicSession::StreamMap& stream_map(QuicSession* session);
static const QuicSession::ClosedStreams& closed_streams(QuicSession* session);
- static QuicSession::ZombieStreamMap& zombie_streams(QuicSession* session);
static void ActivateStream(QuicSession* session,
std::unique_ptr<QuicStream> stream);
