-
Notifications
You must be signed in to change notification settings - Fork 663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix misuse of safe iterators #612
Conversation
Signed-off-by: Madelyn Olson <[email protected]>
Signed-off-by: Madelyn Olson <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #612 +/- ##
============================================
- Coverage 70.20% 70.06% -0.15%
============================================
Files 110 110
Lines 60007 60037 +30
============================================
- Hits 42130 42066 -64
- Misses 17877 17971 +94
|
Signed-off-by: Madelyn Olson <[email protected]>
Signed-off-by: Madelyn Olson <[email protected]>
Signed-off-by: Madelyn Olson <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Not sure if you would like to rename dictInitSafeIterator
as part of this PR or not - the safe
word is misleading as we discussed offline.
I want to do a broader refactoring of that and fix the naming, but this is more scoped specifically to the bug fixes at least. |
Safe iterators must call resetIterators when they are done being used. Fix one issue where a safe iterator was not correctly calling reset during cluster slot caching and fixed a second issue where reset iterator was being called twice. For the double release case, kvstoreIteratorNextDict is responsible for patching up the iterator, but we were calling it a second time in kvstoreIteratorNext.
In addition, I added some documentation around initializing iterators, added an assert to prevent double initialization, and remove a function from the public interface which isn't needed and might lead to incorrect usage of the safe iterators.
Bumping @srgsanky for finding it here: c478206#r142867004.