Mirror/zeek
1
0
Fork 0
mirror of https://github.com/zeek/zeek.git synced 2025-10-02 14:48:21 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Fix robust iterators when modifying dictionary during iteration.

Browse source 
When inserting/deleting elements, we now remove their `DictEntries`
from any robust iterators' bookkeeping. First, we don't need that
information anymore, and second the `DictEntries` contain pointers
that may become invalid.

I don't know how to write a unit test for this unfortunately because
it depends on where exactly things land in the hash table.

Btw, memory mgmt for DictEntries is pretty fragile: They contain
pointers to both memory they own (`key`) and memory they don't own
(`value`). The former type of pointers is shallow-copied on
assignment/copy-construction, meaning that there can be multiple
instances seemingly owning the same memory. That only works because
deletion is manual, and not part of standard destruction. The second
type of pointer has a similar problem, except that it's managed
externally. It's important to not end up with multiple `DictEntries`
pointing to the same value (which is actually what that iterator
bookkeeping did).

Addresses #2032.
This commit is contained in:
Robin Sommer 2022-04-11 13:40:19 +02:00
parent 2c9296120e
commit b4f52ff311
No known key found for this signature in database
GPG key ID: 6BEDA4DA6B8B23E3
1 changed files with 24 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

24
src/Dict.cc
View file

@ -1299,6 +1299,15 @@ void Dictionary::AdjustOnInsert(IterCookie* c, const detail::DictEntry& entry, i
{
ASSERT(c);
ASSERT_VALID(c);
// Remove any previous instances of this value that we may have recorded as
// their pointers will get invalid. We won't need that knowledge anymore
// anyways, will update with new information below as needed.
c->inserted->erase(std::remove(c->inserted->begin(), c->inserted->end(), entry),
c->inserted->end());
c->visited->erase(std::remove(c->visited->begin(), c->visited->end(), entry),
c->visited->end());
if ( insert_position < c->next )
c->inserted->push_back(entry);
if ( insert_position < c->next && c->next <= last_affected_position )
@ -1314,6 +1323,12 @@ void Dictionary::AdjustOnInsert(IterCookie* c, const detail::DictEntry& entry, i
void Dictionary::AdjustOnInsert(RobustDictIterator* c, const detail::DictEntry& entry,
int insert_position, int last_affected_position)
{
// See note in Dictionary::AdjustOnInsert() above.
c->inserted->erase(std::remove(c->inserted->begin(), c->inserted->end(), entry),
c->inserted->end());
c->visited->erase(std::remove(c->visited->begin(), c->visited->end(), entry),
c->visited->end());
if ( insert_position < c->next )
c->inserted->push_back(entry);
if ( insert_position < c->next && c->next <= last_affected_position )
@ -1442,8 +1457,13 @@ void Dictionary::AdjustOnRemove(IterCookie* c, const detail::DictEntry& entry, i
int last_affected_position)
{
ASSERT_VALID(c);
// See note in Dictionary::AdjustOnInsert() above.
c->inserted->erase(std::remove(c->inserted->begin(), c->inserted->end(), entry),
c->inserted->end());
c->visited->erase(std::remove(c->visited->begin(), c->visited->end(), entry),
c->visited->end());
if ( position < c->next && c->next <= last_affected_position )
{
int moved = HeadOfClusterByPosition(c->next - 1);
@ -1462,8 +1482,12 @@ void Dictionary::AdjustOnRemove(IterCookie* c, const detail::DictEntry& entry, i
void Dictionary::AdjustOnRemove(RobustDictIterator* c, const detail::DictEntry& entry, int position,
int last_affected_position)
{
// See note in Dictionary::AdjustOnInsert() above.
c->inserted->erase(std::remove(c->inserted->begin(), c->inserted->end(), entry),
c->inserted->end());
c->visited->erase(std::remove(c->visited->begin(), c->visited->end(), entry),
c->visited->end());
if ( position < c->next && c->next <= last_affected_position )
{
int moved = HeadOfClusterByPosition(c->next - 1);