Deprecate --gfe2_reloadable_flag_quic_session_map_consistency_check
PiperOrigin-RevId: 477725360
diff --git a/quiche/quic/core/quic_dispatcher.cc b/quiche/quic/core/quic_dispatcher.cc
index be05103..acce420 100644
--- a/quiche/quic/core/quic_dispatcher.cc
+++ b/quiche/quic/core/quic_dispatcher.cc
@@ -1099,10 +1099,38 @@
}
closed_session_list_.push_back(std::move(it->second));
CleanUpSession(it->first, connection, error, error_details, source);
+ bool session_removed = false;
for (const QuicConnectionId& cid :
connection->GetActiveServerConnectionIds()) {
- reference_counted_session_map_.erase(cid);
+ auto it1 = reference_counted_session_map_.find(cid);
+ if (it1 != reference_counted_session_map_.end()) {
+ const QuicSession* session2 = it1->second.get();
+ // For cid == server_connection_id, session2 is a nullptr (and hence
+ // session2 != session) now since we have std::move the session into
+ // closed_session_list_ above.
+ if (session2 == session || cid == server_connection_id) {
+ reference_counted_session_map_.erase(it1);
+ session_removed = true;
+ } else {
+ // Leave this session in the map.
+ QUIC_BUG(quic_dispatcher_session_mismatch)
+ << "Session is mismatched in the map. server_connection_id: "
+ << server_connection_id << ". Current cid: " << cid
+ << ". Cid of the other session "
+ << (session2 == nullptr
+ ? "null"
+ : session2->connection()->connection_id().ToString());
+ }
+ } else {
+ // GetActiveServerConnectionIds might return the original destination
+ // ID, which is not contained in the session map.
+ QUIC_BUG_IF(quic_dispatcher_session_not_found,
+ cid != connection->GetOriginalDestinationConnectionId())
+ << "Missing session for cid " << cid
+ << ". server_connection_id: " << server_connection_id;
+ }
}
+ QUIC_BUG_IF(quic_session_is_not_removed, !session_removed);
--num_sessions_in_session_map_;
}
diff --git a/quiche/quic/core/quic_dispatcher_test.cc b/quiche/quic/core/quic_dispatcher_test.cc
index 8d14a8c..0382fdb 100644
--- a/quiche/quic/core/quic_dispatcher_test.cc
+++ b/quiche/quic/core/quic_dispatcher_test.cc
@@ -2281,6 +2281,82 @@
}
TEST_P(QuicDispatcherSupportMultipleConnectionIdPerConnectionTest,
+ TryAddNewConnectionIdWithCollision) {
+ AddConnection1();
+ AddConnection2();
+ ASSERT_EQ(dispatcher_->NumSessions(), 2u);
+ ASSERT_THAT(session1_, testing::NotNull());
+ ASSERT_THAT(session2_, testing::NotNull());
+ MockServerConnection* mock_server_connection1 =
+ reinterpret_cast<MockServerConnection*>(connection1());
+ MockServerConnection* mock_server_connection2 =
+ reinterpret_cast<MockServerConnection*>(connection2());
+
+ {
+ // TestConnectionId(2) is already claimed by connection2 but connection1
+ // still thinks it owns it.
+ mock_server_connection1->UnconditionallyAddNewConnectionIdForTest(
+ TestConnectionId(2));
+ EXPECT_EQ(dispatcher_->NumSessions(), 2u);
+ auto* session =
+ QuicDispatcherPeer::FindSession(dispatcher_.get(), TestConnectionId(2));
+ ASSERT_EQ(session, session2_);
+ EXPECT_THAT(mock_server_connection1->GetActiveServerConnectionIds(),
+ testing::ElementsAre(TestConnectionId(1), TestConnectionId(2)));
+ }
+
+ {
+ mock_server_connection2->AddNewConnectionId(TestConnectionId(3));
+ EXPECT_EQ(dispatcher_->NumSessions(), 2u);
+ auto* session =
+ QuicDispatcherPeer::FindSession(dispatcher_.get(), TestConnectionId(3));
+ ASSERT_EQ(session, session2_);
+ EXPECT_THAT(mock_server_connection2->GetActiveServerConnectionIds(),
+ testing::ElementsAre(TestConnectionId(2), TestConnectionId(3)));
+ }
+
+ // Connection2 removes both TestConnectionId(2) & TestConnectionId(3) from the
+ // session map.
+ dispatcher_->OnConnectionClosed(TestConnectionId(2),
+ QuicErrorCode::QUIC_NO_ERROR, "detail",
+ quic::ConnectionCloseSource::FROM_SELF);
+ // QUICHE_BUG fires when connection1 tries to remove TestConnectionId(2)
+ // again from the session_map.
+ EXPECT_QUICHE_BUG(dispatcher_->OnConnectionClosed(
+ TestConnectionId(1), QuicErrorCode::QUIC_NO_ERROR,
+ "detail", quic::ConnectionCloseSource::FROM_SELF),
+ "Missing session for cid");
+}
+
+TEST_P(QuicDispatcherSupportMultipleConnectionIdPerConnectionTest,
+ MismatchedSessionAfterAddingCollidedConnectionId) {
+ AddConnection1();
+ AddConnection2();
+ MockServerConnection* mock_server_connection1 =
+ reinterpret_cast<MockServerConnection*>(connection1());
+
+ {
+ // TestConnectionId(2) is already claimed by connection2 but connection1
+ // still thinks it owns it.
+ mock_server_connection1->UnconditionallyAddNewConnectionIdForTest(
+ TestConnectionId(2));
+ EXPECT_EQ(dispatcher_->NumSessions(), 2u);
+ auto* session =
+ QuicDispatcherPeer::FindSession(dispatcher_.get(), TestConnectionId(2));
+ ASSERT_EQ(session, session2_);
+ EXPECT_THAT(mock_server_connection1->GetActiveServerConnectionIds(),
+ testing::ElementsAre(TestConnectionId(1), TestConnectionId(2)));
+ }
+
+ // Connection1 tries to remove both Cid1 & Cid2, but they point to different
+ // sessions.
+ EXPECT_QUIC_BUG(dispatcher_->OnConnectionClosed(
+ TestConnectionId(1), QuicErrorCode::QUIC_NO_ERROR,
+ "detail", quic::ConnectionCloseSource::FROM_SELF),
+ "Session is mismatched in the map");
+}
+
+TEST_P(QuicDispatcherSupportMultipleConnectionIdPerConnectionTest,
RetireConnectionIdFromSingleConnection) {
AddConnection1();
ASSERT_EQ(dispatcher_->NumSessions(), 1u);
