From 6491db66172be7d7408a98c06b97ff9ea5f9d7b8 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Tue, 12 Apr 2022 17:37:49 +0200 Subject: [PATCH] Fix assertions in dictionary that can trigger for benign reasons. These assertions were checking for a situation that I believe can happen legitimately: a robust iterator pointing to an index that, after some table resizing, happens to be inside the overflow area and hence empty. We'll now move it to the end of the table in the case. --- src/Dict.cc | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Dict.cc b/src/Dict.cc index bd90014d9b..c9d3fab0df 100644 --- a/src/Dict.cc +++ b/src/Dict.cc @@ -1832,7 +1832,20 @@ detail::DictEntry Dictionary::GetNextRobustIteration(RobustDictIterator* iter) if ( iter->next < 0 ) iter->next = Next(-1); - ASSERT(iter->next >= Capacity() || ! table[iter->next].Empty()); + if ( iter->next < Capacity() && table[iter->next].Empty() ) + { + // [Robin] I believe this means that the table has resized in a way + // that we're now inside the overflow area where elements are empty, + // because elsewhere empty slots aren't allowed. Assuming that's right, + // then it means we'll always be at the end of the table now and could + // also just set `next` to capacity. However, just to be sure, we + // instead reuse logic from below to move forward "to a valid position" + // and then double check, through an assertion in debug mode, that it's + // actually the end. If this ever triggered, the above assumption would + // be wrong (but the Next() call would probably still be right). + iter->next = Next(iter->next); + ASSERT(iter->next == Capacity()); + } // Filter out visited keys. int capacity = Capacity();