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

Corrupted iterators when using unordered_node_map [SOLVED] #75

Closed roncapat closed 4 years ago

roncapat commented 4 years ago

I was refactoring a loop of my code, where at the very beginning I pick a reference (node iterator) to an element in the unordered_hash_map and use it a few times to update values in each cycle.

The refactor was like bringing out of the loop the iterator retrieval, eg. fill a std::vector with all the iterator in a first loop, then loop again over retrieved node iterators to perform value manipulation in a second loop. Decoupling things was the goal, since it would have enabled me to apply openMP (long story short) on some section of the original loop.

However, the code broke. I debugged it for some hours, but the only thing I noticed is that some mData fields inside the node iterators begin to corrupt after I store the 5th node iterator in the vector (tried also and std::array, vector preallocation, no effect). The application tries to use the iterators and ends up with a segfault.

Any help? Tomorrow I'll try to make small example for you, but it will be difficult to extract something small and meaningful from my code. If you know some debugging tips or ways for me to provide you useful data, I'll listen.

roncapat commented 4 years ago

Schermata da 2020-06-19 04-14-19 Schermata da 2020-06-19 04-14-33

x and y values should be either 0x7 0x8 0x9. I checked step by step, and at each insertion the last element pushed is valid. Then after it gets some insertions old, corruption begins.

roncapat commented 4 years ago

checks with -fsanitize=address -fsanitize=undefined, valgrind memcheck passed without remarks from the instrumentation. The code is NOT multithreaded, for now.

roncapat commented 4 years ago

I misunderstood the invalidation rules of iterators.

From C++17 standard

Rehashing invalidates iterators, changes ordering between elements, and changes which buckets elements appear in, but does not invalidate pointers or references to elements. [26.2.7/9]

This means that I was doing bad things storing iterators instead of raw pointers. I fixed my implementation, with ASAP conversions from unordered_node_map::iterator to simple pointer-to-pair using something like auto ptr = &(*iterator) and for now my refactored version works.

Sorry for opening this issue. I'll leave it open for a bit to gain some additional considerations (if you have to provide some), then I'll close it.

roncapat commented 4 years ago

Maybe, I suggest to modify your README to include better phrasing of

Node based map has stable references

that should tell something more explicit, in line with the standard (and save poor souls like me in the future). If you want, I'll make a PR for the README

martinus commented 4 years ago

I've updated the README a bit