skarupke / flat_hash_map

A very fast hashtable
1.69k stars 186 forks source link

bytell_hash_map: Duplicate item found while traversing map after calling erase() #41

Open PaoloBaerlocher opened 3 years ago

PaoloBaerlocher commented 3 years ago

Hello, while using bytell_hash_map I came across this issue: after erasing an item while traversing the map, I get the same item two times. Here is a small repro case:


    typedef ska::bytell_hash_map<const void*, int> StaticObjectsToUpdate;
    //typedef std::unordered_map<const void*, int> StaticObjectsToUpdate;
    StaticObjectsToUpdate testMap;
    int iteration = 0;
    while (1)
    {
        iteration++;
        // Insert some items in the map
        for (int i = 0; i < 10; i++)
        {
            void *ptr = malloc(4);
            testMap[ptr] = 3;
        }
        // Process
        std::vector<const void *> tmpList;
        for (auto it = testMap.begin(); it != testMap.end();)
        {
            tmpList.push_back(it->first);
            auto& nbFrames = it->second;
            if (--nbFrames == 0)
            {
                it = testMap.erase(it);
            }
            else
            {
                it++;
            }
        }

        // Test consistency : look for duplicates
        for (int i = 0; i < tmpList.size(); i++)
        {
            for (int j = i + 1; j < tmpList.size(); j++)
            {
                if (tmpList[i] == tmpList[j])
                {
                    printf("iteration %d: same items found : %x.\n", iteration, tmpList[i]);  // Should not happen
                    exit(0);
                }
            }
        }
    }

The issue does not happen when using flat_hash_map or unordered_map.

ljluestc commented 10 months ago
#include <iostream>
#include <vector>
#include <cstdlib> // For malloc and free
#include <ska/flat_hash_map.hpp> // Include the bytell_hash_map header

typedef ska::bytell_hash_map<const void*, int> StaticObjectsToUpdate;

int main() {
    StaticObjectsToUpdate testMap;
    int iteration = 0;

    while (1) {
        iteration++;

        // Insert some items in the map
        for (int i = 0; i < 10; i++) {
            void *ptr = malloc(4);
            testMap[ptr] = 3;
        }

        // Process
        std::vector<const void *> tmpList;
        for (auto it = testMap.begin(); it != testMap.end();) {
            tmpList.push_back(it->first);
            auto& nbFrames = it->second;
            if (--nbFrames == 0) {
                void *ptr = const_cast<void*>(it->first);
                free(ptr); // Free memory here before erasing
                it = testMap.erase(it);
            } else {
                it++;
            }
        }

        // Test consistency: look for duplicates
        for (size_t i = 0; i < tmpList.size(); i++) {
            for (size_t j = i + 1; j < tmpList.size(); j++) {
                if (tmpList[i] == tmpList[j]) {
                    std::cout << "iteration " << iteration << ": same items found : " << tmpList[i] << ".\n";
                    return 0;
                }
            }
        }
    }

    return 0;
}
alex-budkar-amplitude commented 9 months ago

Found the problem and fixed it ^. If somebody will approve will merge to the master.