mirror of
https://codeberg.org/redict/redict.git
synced 2025-01-22 16:18:28 -05:00
dict.c iterator API misuse protection.
dict.c allows the user to create unsafe iterators, that are iterators that will not touch the dictionary data structure in any way, preventing copy on write, but at the same time are limited in their usage. The limitation is that when itearting with an unsafe iterator, no call to other dictionary functions must be done inside the iteration loop, otherwise the dictionary may be incrementally rehashed resulting into missing elements in the set of the elements returned by the iterator. However after introducing this kind of iterators a number of bugs were found due to misuses of the API, and we are still finding bugs about this issue. The bugs are not trivial to track because the effect is just missing elements during the iteartion. This commit introduces auto-detection of the API misuse. The idea is that an unsafe iterator has a contract: from initialization to the release of the iterator the dictionary should not change. So we take a fingerprint of the dictionary state, xoring a few important dict properties when the unsafe iteartor is initialized. We later check when the iterator is released if the fingerprint is still the same. If it is not, we found a misuse of the iterator, as not allowed API calls changed the internal state of the dictionary. This code was checked against a real bug, issue #1240. This is what Redis prints (aborting) when a misuse is detected: Assertion failed: (iter->fingerprint == dictFingerprint(iter->d)), function dictReleaseIterator, file dict.c, line 587.
This commit is contained in:
parent
7398930756
commit
48cde3fe47
34
src/dict.c
34
src/dict.c
@ -505,6 +505,24 @@ void *dictFetchValue(dict *d, const void *key) {
|
||||
return he ? dictGetVal(he) : NULL;
|
||||
}
|
||||
|
||||
/* A fingerprint is a 64 bit number that represents the state of the dictionary
|
||||
* at a given time, it's just a few dict properties xored together.
|
||||
* When an unsafe iterator is initialized, we get the dict fingerprint, and check
|
||||
* the fingerprint again when the iterator is released.
|
||||
* If the two fingerprints are different it means that the user of the iterator
|
||||
* performed forbidden operations against the dictionary while iterating. */
|
||||
long long dictFingerprint(dict *d) {
|
||||
long long fingerprint = 0;
|
||||
|
||||
fingerprint ^= (long long) d->ht[0].table;
|
||||
fingerprint ^= (long long) d->ht[0].size;
|
||||
fingerprint ^= (long long) d->ht[0].used;
|
||||
fingerprint ^= (long long) d->ht[1].table;
|
||||
fingerprint ^= (long long) d->ht[1].size;
|
||||
fingerprint ^= (long long) d->ht[1].used;
|
||||
return fingerprint;
|
||||
}
|
||||
|
||||
dictIterator *dictGetIterator(dict *d)
|
||||
{
|
||||
dictIterator *iter = zmalloc(sizeof(*iter));
|
||||
@ -530,8 +548,12 @@ dictEntry *dictNext(dictIterator *iter)
|
||||
while (1) {
|
||||
if (iter->entry == NULL) {
|
||||
dictht *ht = &iter->d->ht[iter->table];
|
||||
if (iter->safe && iter->index == -1 && iter->table == 0)
|
||||
iter->d->iterators++;
|
||||
if (iter->index == -1 && iter->table == 0) {
|
||||
if (iter->safe)
|
||||
iter->d->iterators++;
|
||||
else
|
||||
iter->fingerprint = dictFingerprint(iter->d);
|
||||
}
|
||||
iter->index++;
|
||||
if (iter->index >= (signed) ht->size) {
|
||||
if (dictIsRehashing(iter->d) && iter->table == 0) {
|
||||
@ -558,8 +580,12 @@ dictEntry *dictNext(dictIterator *iter)
|
||||
|
||||
void dictReleaseIterator(dictIterator *iter)
|
||||
{
|
||||
if (iter->safe && !(iter->index == -1 && iter->table == 0))
|
||||
iter->d->iterators--;
|
||||
if (!(iter->index == -1 && iter->table == 0)) {
|
||||
if (iter->safe)
|
||||
iter->d->iterators--;
|
||||
else
|
||||
assert(iter->fingerprint == dictFingerprint(iter->d));
|
||||
}
|
||||
zfree(iter);
|
||||
}
|
||||
|
||||
|
@ -88,6 +88,7 @@ typedef struct dictIterator {
|
||||
dict *d;
|
||||
int table, index, safe;
|
||||
dictEntry *entry, *nextEntry;
|
||||
long long fingerprint; /* unsafe iterator fingerprint for misuse detection */
|
||||
} dictIterator;
|
||||
|
||||
/* This is the initial size of every hash table */
|
||||
|
Loading…
Reference in New Issue
Block a user