Skip to content

Conversation

cherylEnkidu
Copy link
Contributor

@cherylEnkidu cherylEnkidu commented Feb 25, 2025

The crash was triggered by recent changes in standard library. In C++ priority queue library, it forces the item inside the queue to have strict order, which means when A > B, B < A is true.

Reference standard library code:

template <class _Tp, class _Up>
  _LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI bool operator()(_Tp& __x, _Up& __y) {
    bool __r = __comp_(__x, __y);
    if (__r)
      __do_compare_assert(0, __y, __x);
    return __r;
  }

However, in customized compare function for FieldIndex, it only compares sequence_number and collection_group, which leads to two Field Index could potentially evaluate to equal.

Hence this PR add a unique identifier to the field index to make sure they will never evaluate to equal.

The fix can be verified by running Firestore_Tests_iOS in Xcode. It should fail in main branch and success with this change.

Copy link
Contributor

@wu-hui wu-hui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to create a unit test for this change?

@cherylEnkidu cherylEnkidu removed their assignment Mar 18, 2025
@wu-hui
Copy link
Contributor

wu-hui commented Mar 18, 2025

Is it possible to create a unit test for this change?

Scratch that, yeah, there is already a test for this.

@cherylEnkidu cherylEnkidu merged commit f504fae into main Mar 18, 2025
35 checks passed
@cherylEnkidu cherylEnkidu deleted the cheryllin/fixPriorityQueue branch March 18, 2025 20:18
@firebase firebase locked and limited conversation to collaborators Jul 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants