diff --git a/src/hashtable.c b/src/hashtable.c index f5f896823b..c8d6173468 100644 --- a/src/hashtable.c +++ b/src/hashtable.c @@ -46,6 +46,7 @@ */ #include "hashtable.h" #include "serverassert.h" +#include "server.h" #include "zmalloc.h" #include "mt19937-64.h" #include "monotonic.h" @@ -1934,6 +1935,11 @@ size_t hashtableScanDefrag(hashtable *ht, size_t cursor, hashtableScanFunction f /* --- Iterator --- */ +#define HASHTABLE_ITER_UNINITIALIZED -1 +#define HASHTABLE_ITER_PRIMARY_TABLE 0 +#define HASHTABLE_ITER_REHASH_TABLE 1 +#define HASHTABLE_ITER_FINISHED 2 + /* Initialize an iterator for a hashtable. * * The 'flags' argument can be used to tweak the behaviour. It's a bitwise-or @@ -1976,11 +1982,12 @@ size_t hashtableScanDefrag(hashtable *ht, size_t cursor, hashtableScanFunction f * Call hashtableNext to fetch each entry. You must call hashtableResetIterator * when you are done with the iterator. */ + void hashtableInitIterator(hashtableIterator *iterator, hashtable *ht, uint8_t flags) { iter *iter; iter = iteratorFromOpaque(iterator); iter->hashtable = ht; - iter->table = 0; + iter->table = HASHTABLE_ITER_PRIMARY_TABLE; iter->index = -1; iter->flags = flags; } @@ -1995,7 +2002,7 @@ void hashtableReinitIterator(hashtableIterator *iterator, hashtable *ht) { /* Resets a stack-allocated iterator. */ void hashtableResetIterator(hashtableIterator *iterator) { iter *iter = iteratorFromOpaque(iterator); - if (!(iter->index == -1 && iter->table == 0)) { + if (!(iter->index == -1 && iter->table == HASHTABLE_ITER_PRIMARY_TABLE)) { if (isSafe(iter)) { hashtableResumeRehashing(iter->hashtable); assert(iter->hashtable->pause_rehash >= 0); @@ -2025,8 +2032,20 @@ void hashtableReleaseIterator(hashtableIterator *iterator) { * Returns false if there are no more entries. */ bool hashtableNext(hashtableIterator *iterator, void **elemptr) { iter *iter = iteratorFromOpaque(iterator); + + if (iter->table == HASHTABLE_ITER_FINISHED) { + return false; + } + + /* Check for unexpected iterator states that indicate bugs */ + debugServerAssert(iter->table < HASHTABLE_ITER_FINISHED && + (iter->table != HASHTABLE_ITER_REHASH_TABLE || hashtableIsRehashing(iter->hashtable)) && + (iter->index < 0 || !iter->hashtable->tables[iter->table] || + (size_t)iter->index < numBuckets(iter->hashtable->bucket_exp[iter->table]))); + + while (1) { - if (iter->index == -1 && iter->table == 0) { + if (iter->index == -1 && iter->table == HASHTABLE_ITER_PRIMARY_TABLE) { /* It's the first call to next. */ if (isSafe(iter)) { hashtablePauseRehashing(iter->hashtable); @@ -2071,9 +2090,9 @@ bool hashtableNext(hashtableIterator *iterator, void **elemptr) { iter->pos_in_bucket = 0; iter->index++; if ((size_t)iter->index >= numBuckets(iter->hashtable->bucket_exp[iter->table])) { - if (hashtableIsRehashing(iter->hashtable) && iter->table == 0) { + if (hashtableIsRehashing(iter->hashtable) && iter->table == HASHTABLE_ITER_PRIMARY_TABLE) { iter->index = 0; - iter->table++; + iter->table = HASHTABLE_ITER_REHASH_TABLE; } else { /* Done. */ break; @@ -2102,6 +2121,7 @@ bool hashtableNext(hashtableIterator *iterator, void **elemptr) { } return true; } + iter->table = HASHTABLE_ITER_FINISHED; return false; } diff --git a/src/unit/test_files.h b/src/unit/test_files.h index 851bacbbf7..e65024e61e 100644 --- a/src/unit/test_files.h +++ b/src/unit/test_files.h @@ -41,6 +41,7 @@ int test_compact_bucket_chain(int argc, char **argv, int flags); int test_random_entry(int argc, char **argv, int flags); int test_random_entry_with_long_chain(int argc, char **argv, int flags); int test_random_entry_sparse_table(int argc, char **argv, int flags); +int test_iterator_bounds_check(int argc, char **argv, int flags); int test_intsetValueEncodings(int argc, char **argv, int flags); int test_intsetBasicAdding(int argc, char **argv, int flags); int test_intsetLargeNumberRandomAdd(int argc, char **argv, int flags); @@ -258,7 +259,7 @@ unitTest __test_crc64combine_c[] = {{"test_crc64combine", test_crc64combine}, {N unitTest __test_dict_c[] = {{"test_dictCreate", test_dictCreate}, {"test_dictAdd16Keys", test_dictAdd16Keys}, {"test_dictDisableResize", test_dictDisableResize}, {"test_dictAddOneKeyTriggerResize", test_dictAddOneKeyTriggerResize}, {"test_dictDeleteKeys", test_dictDeleteKeys}, {"test_dictDeleteOneKeyTriggerResize", test_dictDeleteOneKeyTriggerResize}, {"test_dictEmptyDirAdd128Keys", test_dictEmptyDirAdd128Keys}, {"test_dictDisableResizeReduceTo3", test_dictDisableResizeReduceTo3}, {"test_dictDeleteOneKeyTriggerResizeAgain", test_dictDeleteOneKeyTriggerResizeAgain}, {"test_dictBenchmark", test_dictBenchmark}, {NULL, NULL}}; unitTest __test_endianconv_c[] = {{"test_endianconv", test_endianconv}, {NULL, NULL}}; unitTest __test_entry_c[] = {{"test_entryCreate", test_entryCreate}, {"test_entryUpdate", test_entryUpdate}, {"test_entryHasexpiry_entrySetExpiry", test_entryHasexpiry_entrySetExpiry}, {"test_entryIsExpired", test_entryIsExpired}, {"test_entryMemUsage_entrySetExpiry_entrySetValue", test_entryMemUsage_entrySetExpiry_entrySetValue}, {NULL, NULL}}; -unitTest __test_hashtable_c[] = {{"test_cursor", test_cursor}, {"test_set_hash_function_seed", test_set_hash_function_seed}, {"test_add_find_delete", test_add_find_delete}, {"test_add_find_delete_avoid_resize", test_add_find_delete_avoid_resize}, {"test_instant_rehashing", test_instant_rehashing}, {"test_bucket_chain_length", test_bucket_chain_length}, {"test_two_phase_insert_and_pop", test_two_phase_insert_and_pop}, {"test_replace_reallocated_entry", test_replace_reallocated_entry}, {"test_incremental_find", test_incremental_find}, {"test_scan", test_scan}, {"test_iterator", test_iterator}, {"test_safe_iterator", test_safe_iterator}, {"test_compact_bucket_chain", test_compact_bucket_chain}, {"test_random_entry", test_random_entry}, {"test_random_entry_with_long_chain", test_random_entry_with_long_chain}, {"test_random_entry_sparse_table", test_random_entry_sparse_table}, {NULL, NULL}}; +unitTest __test_hashtable_c[] = {{"test_cursor", test_cursor}, {"test_set_hash_function_seed", test_set_hash_function_seed}, {"test_add_find_delete", test_add_find_delete}, {"test_add_find_delete_avoid_resize", test_add_find_delete_avoid_resize}, {"test_instant_rehashing", test_instant_rehashing}, {"test_bucket_chain_length", test_bucket_chain_length}, {"test_two_phase_insert_and_pop", test_two_phase_insert_and_pop}, {"test_replace_reallocated_entry", test_replace_reallocated_entry}, {"test_incremental_find", test_incremental_find}, {"test_scan", test_scan}, {"test_iterator", test_iterator}, {"test_safe_iterator", test_safe_iterator}, {"test_compact_bucket_chain", test_compact_bucket_chain}, {"test_random_entry", test_random_entry}, {"test_random_entry_with_long_chain", test_random_entry_with_long_chain}, {"test_random_entry_sparse_table", test_random_entry_sparse_table}, {"test_iterator_bounds_check", test_iterator_bounds_check}, {NULL, NULL}}; unitTest __test_intset_c[] = {{"test_intsetValueEncodings", test_intsetValueEncodings}, {"test_intsetBasicAdding", test_intsetBasicAdding}, {"test_intsetLargeNumberRandomAdd", test_intsetLargeNumberRandomAdd}, {"test_intsetUpgradeFromint16Toint32", test_intsetUpgradeFromint16Toint32}, {"test_intsetUpgradeFromint16Toint64", test_intsetUpgradeFromint16Toint64}, {"test_intsetUpgradeFromint32Toint64", test_intsetUpgradeFromint32Toint64}, {"test_intsetStressLookups", test_intsetStressLookups}, {"test_intsetStressAddDelete", test_intsetStressAddDelete}, {NULL, NULL}}; unitTest __test_kvstore_c[] = {{"test_kvstoreAdd16Keys", test_kvstoreAdd16Keys}, {"test_kvstoreIteratorRemoveAllKeysNoDeleteEmptyHashtable", test_kvstoreIteratorRemoveAllKeysNoDeleteEmptyHashtable}, {"test_kvstoreIteratorRemoveAllKeysDeleteEmptyHashtable", test_kvstoreIteratorRemoveAllKeysDeleteEmptyHashtable}, {"test_kvstoreHashtableIteratorRemoveAllKeysNoDeleteEmptyHashtable", test_kvstoreHashtableIteratorRemoveAllKeysNoDeleteEmptyHashtable}, {"test_kvstoreHashtableIteratorRemoveAllKeysDeleteEmptyHashtable", test_kvstoreHashtableIteratorRemoveAllKeysDeleteEmptyHashtable}, {"test_kvstoreHashtableExpand", test_kvstoreHashtableExpand}, {NULL, NULL}}; unitTest __test_listpack_c[] = {{"test_listpackCreateIntList", test_listpackCreateIntList}, {"test_listpackCreateList", test_listpackCreateList}, {"test_listpackLpPrepend", test_listpackLpPrepend}, {"test_listpackLpPrependInteger", test_listpackLpPrependInteger}, {"test_listpackGetELementAtIndex", test_listpackGetELementAtIndex}, {"test_listpackPop", test_listpackPop}, {"test_listpackGetELementAtIndex2", test_listpackGetELementAtIndex2}, {"test_listpackIterate0toEnd", test_listpackIterate0toEnd}, {"test_listpackIterate1toEnd", test_listpackIterate1toEnd}, {"test_listpackIterate2toEnd", test_listpackIterate2toEnd}, {"test_listpackIterateBackToFront", test_listpackIterateBackToFront}, {"test_listpackIterateBackToFrontWithDelete", test_listpackIterateBackToFrontWithDelete}, {"test_listpackDeleteWhenNumIsMinusOne", test_listpackDeleteWhenNumIsMinusOne}, {"test_listpackDeleteWithNegativeIndex", test_listpackDeleteWithNegativeIndex}, {"test_listpackDeleteInclusiveRange0_0", test_listpackDeleteInclusiveRange0_0}, {"test_listpackDeleteInclusiveRange0_1", test_listpackDeleteInclusiveRange0_1}, {"test_listpackDeleteInclusiveRange1_2", test_listpackDeleteInclusiveRange1_2}, {"test_listpackDeleteWitStartIndexOutOfRange", test_listpackDeleteWitStartIndexOutOfRange}, {"test_listpackDeleteWitNumOverflow", test_listpackDeleteWitNumOverflow}, {"test_listpackBatchDelete", test_listpackBatchDelete}, {"test_listpackDeleteFooWhileIterating", test_listpackDeleteFooWhileIterating}, {"test_listpackReplaceWithSameSize", test_listpackReplaceWithSameSize}, {"test_listpackReplaceWithDifferentSize", test_listpackReplaceWithDifferentSize}, {"test_listpackRegressionGt255Bytes", test_listpackRegressionGt255Bytes}, {"test_listpackCreateLongListAndCheckIndices", test_listpackCreateLongListAndCheckIndices}, {"test_listpackCompareStrsWithLpEntries", test_listpackCompareStrsWithLpEntries}, {"test_listpackLpMergeEmptyLps", test_listpackLpMergeEmptyLps}, {"test_listpackLpMergeLp1Larger", test_listpackLpMergeLp1Larger}, {"test_listpackLpMergeLp2Larger", test_listpackLpMergeLp2Larger}, {"test_listpackLpNextRandom", test_listpackLpNextRandom}, {"test_listpackLpNextRandomCC", test_listpackLpNextRandomCC}, {"test_listpackRandomPairWithOneElement", test_listpackRandomPairWithOneElement}, {"test_listpackRandomPairWithManyElements", test_listpackRandomPairWithManyElements}, {"test_listpackRandomPairsWithOneElement", test_listpackRandomPairsWithOneElement}, {"test_listpackRandomPairsWithManyElements", test_listpackRandomPairsWithManyElements}, {"test_listpackRandomPairsUniqueWithOneElement", test_listpackRandomPairsUniqueWithOneElement}, {"test_listpackRandomPairsUniqueWithManyElements", test_listpackRandomPairsUniqueWithManyElements}, {"test_listpackPushVariousEncodings", test_listpackPushVariousEncodings}, {"test_listpackLpFind", test_listpackLpFind}, {"test_listpackLpValidateIntegrity", test_listpackLpValidateIntegrity}, {"test_listpackNumberOfElementsExceedsLP_HDR_NUMELE_UNKNOWN", test_listpackNumberOfElementsExceedsLP_HDR_NUMELE_UNKNOWN}, {"test_listpackStressWithRandom", test_listpackStressWithRandom}, {"test_listpackSTressWithVariableSize", test_listpackSTressWithVariableSize}, {"test_listpackBenchmarkInit", test_listpackBenchmarkInit}, {"test_listpackBenchmarkLpAppend", test_listpackBenchmarkLpAppend}, {"test_listpackBenchmarkLpFindString", test_listpackBenchmarkLpFindString}, {"test_listpackBenchmarkLpFindNumber", test_listpackBenchmarkLpFindNumber}, {"test_listpackBenchmarkLpSeek", test_listpackBenchmarkLpSeek}, {"test_listpackBenchmarkLpValidateIntegrity", test_listpackBenchmarkLpValidateIntegrity}, {"test_listpackBenchmarkLpCompareWithString", test_listpackBenchmarkLpCompareWithString}, {"test_listpackBenchmarkLpCompareWithNumber", test_listpackBenchmarkLpCompareWithNumber}, {"test_listpackBenchmarkFree", test_listpackBenchmarkFree}, {NULL, NULL}}; diff --git a/src/unit/test_hashtable.c b/src/unit/test_hashtable.c index 0c9d197575..bb9bb75138 100644 --- a/src/unit/test_hashtable.c +++ b/src/unit/test_hashtable.c @@ -945,3 +945,32 @@ int test_random_entry_sparse_table(int argc, char **argv, int flags) { zfree(values); return 0; } + +int test_iterator_bounds_check(int argc, char **argv, int flags) { + UNUSED(argc); + UNUSED(argv); + UNUSED(flags); + + hashtableType type = {0}; + hashtable *ht = hashtableCreate(&type); + + for (size_t j = 0; j < 100; j++) { + TEST_ASSERT(hashtableAdd(ht, (void *)j)); + } + + hashtableIterator iter; + void *entry; + hashtableInitIterator(&iter, ht, 0); + + size_t count = 0; + while (hashtableNext(&iter, &entry)) count++; + TEST_ASSERT(count == 100); + + /* Calling next again should return false */ + TEST_ASSERT(!hashtableNext(&iter, &entry)); + TEST_ASSERT(!hashtableNext(&iter, &entry)); + + hashtableResetIterator(&iter); + hashtableRelease(ht); + return 0; +}