martinus / robin-hood-hashing

Fast & memory efficient hashtable based on robin hood hashing for C++11/14/17/20
https://gitter.im/martinus/robin-hood-hashing
MIT License
1.5k stars 142 forks source link

Possible Data Corruption Bug? #163

Closed william-silversmith closed 1 year ago

william-silversmith commented 1 year ago

Hi!

I was attempting to replace std::unordered_map<uint64_t, edge_type> (edge_type defined below) with robin_hood::unordered_flat_map<uint64_t, edge_type>. This caused my code to spinlock somehow, so I assumed it had something to do with invalidated iterators and hunted for a bug. I tried using robin_hood::unordered_node_map and that gave correct results, though slower than std::unordered_map so initially it seemed my hypothesis about iterators was correct.

I spent some time tracing through the code and found that somehow data being inserted into the array was corrupted.

robin_hood::unordered_flat_map< uint64_t, edge_type >  edges_   ;
...
    void add_edge( uint32_t x, uint32_t y, uint32_t z, uint32_t f ) {
         const uint64_t e = detail::make_edge( x, y );
         ...
        std::cout << "e: " << e << " (" << f << ", " << z << ")" << std::endl;

        edges_.emplace(e, edge_type( f, z ));
        edge_type edg = edges_.at(e);
        std::cout << edg.face_ << " " << edg.vertex_ << std::endl;
        ZI_ASSERT( edges_.at(e) == edge_type(f,z) );

It turned out that the assertion statement was failing. I checked the output and added a few print statements to emplace:

e: 18446744073709551614 (1, 2)
new node
1 2
e: 18446744069414584317 (1, 0)
new node
1 0
e: 18446744065119617023 (1, 1)
new node
1 1
e: 18446744073709551613 (2, 3)
new node
2 3
e: 18446744065119617020 (2, 0)
new node
2 0
e: 18446744060824649727 (2, 2)
overwrite_node
1 2
Assertion failed: (edges_.at(e) == edge_type(f,z)), function add_edge, file tri_mesh.hpp, line 175.

As you can see at the last emplacement, the retrieved value is (1,2) when the value inserted was (2,2) for a unique key.

My C++ knowledge isn't the strongest, could I perhaps be failing to hash edge_type correctly?

The original std::unordered_map code in question can be found here:

I also gave this a spin with the newer ankerl::unordered_dense::map and had the same issue of spinlocking, though I didn't attempt to debug it in fine detail like I did with robin_hood. I presume it's a similar issue and probably my fault.

Thanks so much for such a helpful library!

william-silversmith commented 1 year ago

I think this is probably not this library's fault. I experimented with another flat map implementation from tsl and it exhibited the same issue. Closing this issue.

william-silversmith commented 1 year ago

It looks like the issue was a poorly designed overloaded copy assignment operator in the edge_type class. Sorry for bothering you.