Deprecate --gfe2_reloadable_flag_quic_session_map_consistency_check

PiperOrigin-RevId: 477725360

quiche/quic/core/quic_dispatcher.cc

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_;
 }
 

quiche/quic/core/quic_dispatcher_test.cc

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);