Fix priority queue assertion which enforce the item to have strict order (#14496)

Author: cherylEnkidu

Firestore/CHANGELOG.md

@@ -1,3 +1,6 @@
+# Unreleased
+- [fixed] Fixed the customized priority queue compare function used cache index manager. (#14496)
+
 # 11.9.0
 - [fixed] Fixed memory leak in `Query.whereField()`. (#13978)
 

Firestore/core/src/local/leveldb_index_manager.cc

@@ -188,10 +188,15 @@ LevelDbIndexManager::LevelDbIndexManager(const User& user,
   // The contract for this comparison expected by priority queue is
   // `std::less`, but std::priority_queue's default order is descending.
   // We change the order to be ascending by doing left >= right instead.
+  // Note: priority queue has to have a strict ordering, so here using unique_id
+  // to order Field Indexes having same `sequence_number` and `collection_group`
   auto cmp = [](FieldIndex* left, FieldIndex* right) {
     if (left->index_state().sequence_number() ==
         right->index_state().sequence_number()) {
-      return left->collection_group() >= right->collection_group();
+      if (left->collection_group() == right->collection_group()) {
+        return left->unique_id() > right->unique_id();
+      }
+      return left->collection_group() > right->collection_group();
     }
     return left->index_state().sequence_number() >
            right->index_state().sequence_number();

Firestore/core/src/model/field_index.cc

@@ -20,6 +20,8 @@ namespace firebase {
 namespace firestore {
 namespace model {
 
+std::atomic<int> FieldIndex::ref_count_{0};
+
 util::ComparisonResult Segment::CompareTo(const Segment& rhs) const {
   auto result = field_path().CompareTo(rhs.field_path());
   if (result != util::ComparisonResult::Same) {

Firestore/core/src/model/field_index.h

@@ -243,7 +243,9 @@ class FieldIndex {
   static util::ComparisonResult SemanticCompare(const FieldIndex& left,
                                                 const FieldIndex& right);
 
-  FieldIndex() : index_id_(UnknownId()) {
+  FieldIndex()
+      : index_id_(UnknownId()),
+        unique_id_(ref_count_.fetch_add(1, std::memory_order_acq_rel)) {
   }
 
   FieldIndex(int32_t index_id,
@@ -253,7 +255,50 @@ class FieldIndex {
       : index_id_(index_id),
         collection_group_(std::move(collection_group)),
         segments_(std::move(segments)),
-        state_(std::move(state)) {
+        state_(std::move(state)),
+        unique_id_(ref_count_.fetch_add(1, std::memory_order_acq_rel)) {
+  }
+
+  // Copy constructor
+  FieldIndex(const FieldIndex& other)
+      : index_id_(other.index_id_),
+        collection_group_(other.collection_group_),
+        segments_(other.segments_),
+        state_(other.state_),
+        unique_id_(ref_count_.fetch_add(1, std::memory_order_acq_rel)) {
+  }
+
+  // Copy assignment operator
+  FieldIndex& operator=(const FieldIndex& other) {
+    if (this != &other) {
+      index_id_ = other.index_id_;
+      collection_group_ = other.collection_group_;
+      segments_ = other.segments_;
+      state_ = other.state_;
+      unique_id_ = ref_count_.fetch_add(1, std::memory_order_acq_rel);
+    }
+    return *this;
+  }
+
+  // Move constructor
+  FieldIndex(FieldIndex&& other) noexcept
+      : index_id_(other.index_id_),
+        collection_group_(std::move(other.collection_group_)),
+        segments_(std::move(other.segments_)),
+        state_(std::move(other.state_)),
+        unique_id_(ref_count_.fetch_add(1, std::memory_order_acq_rel)) {
+  }
+
+  // Move assignment operator
+  FieldIndex& operator=(FieldIndex&& other) noexcept {
+    if (this != &other) {
+      index_id_ = other.index_id_;
+      collection_group_ = std::move(other.collection_group_);
+      segments_ = std::move(other.segments_);
+      state_ = std::move(other.state_);
+      unique_id_ = ref_count_.fetch_add(1, std::memory_order_acq_rel);
+    }
+    return *this;
   }
 
   /**
@@ -285,6 +330,14 @@ class FieldIndex {
   /** Returns the ArrayContains/ArrayContainsAny segment for this index. */
   absl::optional<Segment> GetArraySegment() const;
 
+  /**
+   * Returns the unique identifier for this object, ensuring a strict ordering
+   * in the priority queue's comparison function.
+   */
+  int unique_id() const {
+    return unique_id_;
+  }
+
   /**
    * A type that can be used as the "Compare" template parameter of ordered
    * collections to have the elements ordered using
@@ -308,6 +361,10 @@ class FieldIndex {
   std::string collection_group_;
   std::vector<Segment> segments_;
   IndexState state_;
+  int unique_id_;
+
+  // TODO(C++17): Replace with inline static std::atomic<int> ref_count_ = 0;
+  static std::atomic<int> ref_count_;
 };
 
 inline bool operator==(const FieldIndex& lhs, const FieldIndex& rhs) {