sparse-map
sparse-map copied to clipboard
Avoid iterator invalidation without effective insert
There's an issue with accessing an existing value with operator[] when hashtable size reaches a certain value: iterator is sometimes being invalidated leading to broken iterators invalidation contract:
- insert, emplace, emplace_hint, operator[]: if there is an effective insert, invalidate the iterators.
or use-after-free from calling code if user is unaware of such behaviour.
Fix this issue by actually issuing a rehash in effective erase and insert routines.
PS: docs also say that erase()
always invalidates the iterators, though the current implementation does that only on effective erase. Should the doc be adjusted as well?
Thank you, we effectively should avoid rehashing when no actual insert is done.
I added some comments to the PR code.
Thanks for the feedback. I have pushed fixes for review and additionally reverted + 1
in load threshold check to preserve original behaviour
Thank you for the change, looks good. Just a small comment left.
Note that the map doesn't guarantee that erase is noexcept
per the commit message. The problem is more that an exception could be thrown during the rehash after an element was successfully erased which goes against the strong exception guarantee (see https://en.cppreference.com/w/cpp/language/exceptions).
I also would like to limit the rehash to insertions as done in std::unordered_map
I think I have addressed all of your comments in the last revision and it is safe to merge. Also, I have squashed all the commits into a single one (since they no longer make sense on their own).
Sorry for the rather long delay, been busy with other things.
Thank you, it looks good, I merged the change.