Fix: hash set iterator returns deleted tombstones in equal_range

[DavIvek]

The equal_range iterator in flat_hash_multi_set_gt did not check the deleted flag when advancing. After add/remove cycles that create tombstones with the same key, the iterator would yield deleted entries as live matches. In index_dense_gt::remove() this caused already-freed slots to be pushed to free_keys_ again, inflating free_keys_.size() and producing unsigned underflow in size().

Reproduces in both multi and non-multi mode (single-threaded) whenever two keys hash-collide and one tombstone is reused by a different key.

Closes #697

[DavIvek]

I was able to reproduce corruption with next test:

#include <usearch/index_plugins.hpp>

namespace {

struct KeySlot {
    std::uint64_t key;
    std::uint64_t slot;
};

// Guaranteed collisions.
struct CollidingHash {
    using is_transparent = void;
    std::size_t operator()(KeySlot const &) const noexcept { return 0; }
    std::size_t operator()(std::uint64_t) const noexcept { return 0; }
};

struct KeySlotEqual {
    using is_transparent = void;
    bool operator()(KeySlot const &a, std::uint64_t b) const noexcept { return a.key == b; }
    bool operator()(std::uint64_t a, KeySlot const &b) const noexcept { return a == b.key; }
    bool operator()(KeySlot const &a, KeySlot const &b) const noexcept { return a.key == b.key; }
};

using HashSet = unum::usearch::flat_hash_multi_set_gt<KeySlot, CollidingHash, KeySlotEqual>;

}  // namespace

// Simulates index_dense_gt's add/remove cycle and shows size() corruption.
TEST(UsearchTombstoneBug, SizeCorruption) {
    HashSet slot_lookup;

    // Simulated counters (mirrors index_dense_gt internals)
    std::size_t typed_size = 0;
    std::vector<std::uint64_t> free_keys;  // simulated ring buffer

    auto simulated_size = [&]() -> std::int64_t {
        return static_cast<std::int64_t>(typed_size) - static_cast<std::int64_t>(free_keys.size());
    };

    auto simulate_add = [&](std::uint64_t key) {
        std::uint64_t slot;
        if (!free_keys.empty()) {
            slot = free_keys.back();
            free_keys.pop_back();
        } else {
            slot = typed_size++;
        }
        slot_lookup.try_emplace({key, slot});
    };

    auto simulate_remove = [&](std::uint64_t key) {
        auto [begin, end] = slot_lookup.equal_range(key);
        for (auto it = begin; it != end; ++it) {
            free_keys.push_back((*it).slot);
        }
        slot_lookup.erase(key);
    };

    // Build the toxic pattern: [live-K, tombstone-K] in the same cluster.
    //
    // 1. add(J)    → [pos 0: {J, slot0}]
    // 2. add(K)    → [pos 0: {J, slot0}, pos 1: {K, slot1}]  (collision)
    // 3. remove(J) → [pos 0: {J, slot0}☠, pos 1: {K, slot1}]
    // 4. remove(K) → [pos 0: {J, slot0}☠, pos 1: {K, slot1}☠]
    // 5. add(K)    → reuses tombstone at pos 0: [pos 0: {K, slot0}, pos 1: {K, slot1}☠]
    //                                            ^^^^^^^^ live       ^^^^^^^^ tombstone
    //
    // Now remove(K) calls equal_range(K). The buggy iterator sees BOTH the
    // live entry at pos 0 AND the tombstone at pos 1 as matches.
    // It pushes slot0 AND slot1 to free_keys_, but only slot0 was actually live.
    // slot1 was already in free_keys_ from step 4. → phantom duplicate → size() drift.

    constexpr std::uint64_t K = 1;
    constexpr std::uint64_t J = 2;

    simulate_add(J);                            // step 1
    simulate_add(K);                            // step 2
    simulate_remove(J);                         // step 3
    simulate_remove(K);                         // step 4
    simulate_add(K);                            // step 5

    ASSERT_EQ(simulated_size(), 1) << "One live entry before the critical remove";

    simulate_remove(K);                         // step 6: THE BUGGY REMOVE

    // Bug: free_keys has a phantom duplicate
    EXPECT_EQ(simulated_size(), 0)
        << "size() should be 0 after removing the only entry, but free_keys_ has "
        << free_keys.size() << " entries vs typed_size " << typed_size
        << " → size() = " << simulated_size() << " (phantom duplicates in free_keys)";
}

[DavIvek]

I believe that failing checks aren't related with proposed changes.

[DavIvek]

Hi @ashvardanian. I know things can get busy, so no rush on the review/merge.

In the meantime, I’m preparing a patch on our side (https://github.com/memgraph/memgraph) since we have a release coming up in a couple of days. I wanted to check if the proposed fix in the PR generally makes sense from your perspective before we proceed with that approach.

Thanks!

[ashvardanian]

Hi @DavIvek! Sorry it takes some time to review, I'm fully consumed by ashvardanian/NumKong#220. I need to push it through the line before validating the PRs here 🤗