diff --git a/src/acl.c b/src/acl.c index 6f66bcbf01..ad4ba82cb2 100644 --- a/src/acl.c +++ b/src/acl.c @@ -649,7 +649,7 @@ static void ACLChangeSelectorPerm(aclSelector *selector, struct serverCommand *c struct serverCommand *sub = next; ACLSetSelectorCommandBit(selector, sub->id, allow); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } } @@ -672,7 +672,7 @@ static void ACLSetSelectorCommandBitsForCategory(hashtable *commands, aclSelecto ACLSetSelectorCommandBitsForCategory(cmd->subcommands_ht, selector, cflag, value); } } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } /* This function is responsible for recomputing the command bits for all selectors of the existing users. @@ -1925,7 +1925,7 @@ static int ACLShouldKillPubsubClient(client *c, list *upcoming) { int res = ACLCheckChannelAgainstList(upcoming, o->ptr, sdslen(o->ptr), 1); kill = (res == ACL_DENIED_CHANNEL); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); /* Check for channel violations. */ if (!kill) { @@ -1938,7 +1938,7 @@ static int ACLShouldKillPubsubClient(client *c, list *upcoming) { int res = ACLCheckChannelAgainstList(upcoming, o->ptr, sdslen(o->ptr), 0); kill = (res == ACL_DENIED_CHANNEL); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } if (!kill) { /* Check for shard channels violation. */ @@ -1950,7 +1950,7 @@ static int ACLShouldKillPubsubClient(client *c, list *upcoming) { int res = ACLCheckChannelAgainstList(upcoming, o->ptr, sdslen(o->ptr), 0); kill = (res == ACL_DENIED_CHANNEL); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } if (kill) { @@ -2744,7 +2744,7 @@ static void aclCatWithFlags(client *c, hashtable *commands, uint64_t cflag, int aclCatWithFlags(c, cmd->subcommands_ht, cflag, arraylen); } } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } /* Add the formatted response from a single selector to the ACL GETUSER diff --git a/src/aof.c b/src/aof.c index 62c0ab46be..ed3b706bb7 100644 --- a/src/aof.c +++ b/src/aof.c @@ -1906,19 +1906,19 @@ int rewriteSortedSetObject(rio *r, robj *key, robj *o) { if (!rioWriteBulkCount(r, '*', 2 + cmd_items * 2) || !rioWriteBulkString(r, "ZADD", 4) || !rioWriteBulkObject(r, key)) { - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); return 0; } } sds ele = node->ele; if (!rioWriteBulkDouble(r, node->score) || !rioWriteBulkString(r, ele, sdslen(ele))) { - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); return 0; } if (++count == AOF_REWRITE_ITEMS_PER_CMD) count = 0; items--; } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } else { serverPanic("Unknown sorted zset encoding"); } diff --git a/src/debug.c b/src/debug.c index 56dbdfbb20..8d3be9368c 100644 --- a/src/debug.c +++ b/src/debug.c @@ -219,7 +219,7 @@ void xorObjectDigest(serverDb *db, robj *keyobj, unsigned char *digest, robj *o) mixDigest(eledigest, buf, strlen(buf)); xorDigest(digest, eledigest, 20); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } else { serverPanic("Unknown sorted set encoding"); } diff --git a/src/defrag.c b/src/defrag.c index 79a8b473d2..ddf1174f99 100644 --- a/src/defrag.c +++ b/src/defrag.c @@ -775,7 +775,7 @@ static void defragPubsubScanCallback(void *privdata, void *elemref) { bool replaced = hashtableReplaceReallocatedEntry(client_channels, channel, newchannel); serverAssert(replaced); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } /* Try to defrag the dictionary of clients that is stored as the value part. */ diff --git a/src/hashtable.c b/src/hashtable.c index 0c0b2768db..075fbd5626 100644 --- a/src/hashtable.c +++ b/src/hashtable.c @@ -284,6 +284,9 @@ typedef struct hashtableBucket { /* A key property is that the bucket size is one cache line. */ static_assert(sizeof(bucket) == HASHTABLE_BUCKET_SIZE, "Bucket size mismatch"); +/* Forward declaration for iter type */ +typedef struct iter iter; + struct hashtable { hashtableType *type; ssize_t rehash_idx; /* -1 = rehashing not in progress. */ @@ -293,10 +296,11 @@ struct hashtable { int16_t pause_rehash; /* Non-zero = rehashing is paused */ int16_t pause_auto_shrink; /* Non-zero = automatic resizing disallowed. */ size_t child_buckets[2]; /* Number of allocated child buckets. */ + iter *safe_iterators; /* Head of linked list of safe iterators */ void *metadata[]; }; -typedef struct { +struct iter { hashtable *hashtable; bucket *bucket; long index; @@ -309,7 +313,8 @@ typedef struct { /* Safe iterator temporary storage for bucket chain compaction. */ uint64_t last_seen_size; }; -} iter; + iter *next_safe_iter; /* Next safe iterator in hashtable's list */ +}; /* The opaque hashtableIterator is defined as a blob of bytes. */ static_assert(sizeof(hashtableIterator) >= sizeof(iter), @@ -1084,6 +1089,37 @@ static inline int shouldPrefetchValues(iter *iter) { return (iter->flags & HASHTABLE_ITER_PREFETCH_VALUES); } +/* Add a safe iterator to the hashtable's tracking list */ +static void trackSafeIterator(iter *it) { + assert(it->next_safe_iter == NULL); + hashtable *ht = it->hashtable; + it->next_safe_iter = ht->safe_iterators; + ht->safe_iterators = it; +} + +/* Remove a safe iterator from the hashtable's tracking list */ +static void untrackSafeIterator(iter *it) { + hashtable *ht = it->hashtable; + if (ht->safe_iterators == it) { + ht->safe_iterators = it->next_safe_iter; + } else { + iter *current = ht->safe_iterators; + assert(current != NULL); + while (current->next_safe_iter != it) { + current = current->next_safe_iter; + assert(current != NULL); + } + current->next_safe_iter = it->next_safe_iter; + } + it->next_safe_iter = NULL; + it->hashtable = NULL; /* Mark as invalid */ +} + +/* Invalidate all safe iterators by setting hashtable = NULL */ +static void invalidateAllSafeIterators(hashtable *ht) { + while (ht->safe_iterators) untrackSafeIterator(ht->safe_iterators); +} + /* --- API functions --- */ /* Allocates and initializes a new hashtable specified by the given type. */ @@ -1098,6 +1134,7 @@ hashtable *hashtableCreate(hashtableType *type) { ht->rehash_idx = -1; ht->pause_rehash = 0; ht->pause_auto_shrink = 0; + ht->safe_iterators = NULL; resetTable(ht, 0); resetTable(ht, 1); if (type->trackMemUsage) type->trackMemUsage(ht, alloc_size); @@ -1153,6 +1190,7 @@ void hashtableEmpty(hashtable *ht, void(callback)(hashtable *)) { /* Deletes all the entries and frees the table. */ void hashtableRelease(hashtable *ht) { + invalidateAllSafeIterators(ht); hashtableEmpty(ht, NULL); /* Call trackMemUsage before zfree, so trackMemUsage can access ht. */ if (ht->type->trackMemUsage) { @@ -1242,6 +1280,7 @@ static void hashtablePauseRehashing(hashtable *ht) { /* Resumes incremental rehashing, after pausing it. */ static void hashtableResumeRehashing(hashtable *ht) { ht->pause_rehash--; + assert(ht->pause_rehash >= 0); hashtableResumeAutoShrink(ht); } @@ -1962,7 +2001,9 @@ size_t hashtableScanDefrag(hashtable *ht, size_t cursor, hashtableScanFunction f * rehashing to prevent entries from moving around. It's allowed to insert and * replace entries. Deleting entries is only allowed for the entry that was just * returned by hashtableNext. Deleting other entries is possible, but doing so - * can cause internal fragmentation, so don't. + * can cause internal fragmentation, so don't. The hash table itself can be + * safely deleted while safe iterators exist - they will be invalidated and + * subsequent calls to hashtableNext will return false. * * Guarantees for safe iterators: * @@ -1978,7 +2019,7 @@ size_t hashtableScanDefrag(hashtable *ht, size_t cursor, hashtableScanFunction f * - Entries that are inserted during the iteration may or may not be returned * by the iterator. * - * Call hashtableNext to fetch each entry. You must call hashtableResetIterator + * Call hashtableNext to fetch each entry. You must call hashtableCleanupIterator * when you are done with the iterator. */ void hashtableInitIterator(hashtableIterator *iterator, hashtable *ht, uint8_t flags) { @@ -1988,22 +2029,31 @@ void hashtableInitIterator(hashtableIterator *iterator, hashtable *ht, uint8_t f iter->table = 0; iter->index = -1; iter->flags = flags; + iter->next_safe_iter = NULL; + if (isSafe(iter) && ht != NULL) { + trackSafeIterator(iter); + } } -/* Reinitializes the iterator for the provided hashtable while - * preserving the flags from its previous initialization. */ -void hashtableReinitIterator(hashtableIterator *iterator, hashtable *ht) { +/* Reinitializes the iterator to begin a new iteration of the provided hashtable + * while preserving the flags from its previous initialization. */ +void hashtableRetargetIterator(hashtableIterator *iterator, hashtable *ht) { iter *iter = iteratorFromOpaque(iterator); - hashtableInitIterator(iterator, ht, iter->flags); + uint8_t flags = iter->flags; + + hashtableCleanupIterator(iterator); + hashtableInitIterator(iterator, ht, flags); } -/* Resets a stack-allocated iterator. */ -void hashtableResetIterator(hashtableIterator *iterator) { +/* Performs required cleanup for a stack-allocated iterator. */ +void hashtableCleanupIterator(hashtableIterator *iterator) { iter *iter = iteratorFromOpaque(iterator); + if (iter->hashtable == NULL) return; + if (!(iter->index == -1 && iter->table == 0)) { if (isSafe(iter)) { hashtableResumeRehashing(iter->hashtable); - assert(iter->hashtable->pause_rehash >= 0); + untrackSafeIterator(iter); } else { assert(iter->fingerprint == hashtableFingerprint(iter->hashtable)); } @@ -2021,7 +2071,7 @@ hashtableIterator *hashtableCreateIterator(hashtable *ht, uint8_t flags) { /* Resets and frees the memory of an allocated iterator, i.e. one created using * hashtableCreate(Safe)Iterator. */ void hashtableReleaseIterator(hashtableIterator *iterator) { - hashtableResetIterator(iterator); + hashtableCleanupIterator(iterator); iter *iter = iteratorFromOpaque(iterator); zfree(iter); } @@ -2030,6 +2080,9 @@ void hashtableReleaseIterator(hashtableIterator *iterator) { * Returns false if there are no more entries. */ bool hashtableNext(hashtableIterator *iterator, void **elemptr) { iter *iter = iteratorFromOpaque(iterator); + /* Check if iterator has been invalidated */ + if (iter->hashtable == NULL) return false; + while (1) { if (iter->index == -1 && iter->table == 0) { /* It's the first call to next. */ diff --git a/src/hashtable.h b/src/hashtable.h index b3507bfd0b..7bef8ab142 100644 --- a/src/hashtable.h +++ b/src/hashtable.h @@ -39,7 +39,7 @@ typedef struct hashtable hashtable; typedef struct hashtableStats hashtableStats; /* Can types that can be stack allocated. */ -typedef uint64_t hashtableIterator[5]; +typedef uint64_t hashtableIterator[6]; typedef uint64_t hashtablePosition[2]; typedef uint64_t hashtableIncrementalFindState[5]; @@ -160,8 +160,8 @@ bool hashtableIncrementalFindGetResult(hashtableIncrementalFindState *state, voi size_t hashtableScan(hashtable *ht, size_t cursor, hashtableScanFunction fn, void *privdata); size_t hashtableScanDefrag(hashtable *ht, size_t cursor, hashtableScanFunction fn, void *privdata, void *(*defragfn)(void *), int flags); void hashtableInitIterator(hashtableIterator *iter, hashtable *ht, uint8_t flags); -void hashtableReinitIterator(hashtableIterator *iterator, hashtable *ht); -void hashtableResetIterator(hashtableIterator *iter); +void hashtableRetargetIterator(hashtableIterator *iterator, hashtable *ht); +void hashtableCleanupIterator(hashtableIterator *iter); hashtableIterator *hashtableCreateIterator(hashtable *ht, uint8_t flags); void hashtableReleaseIterator(hashtableIterator *iter); bool hashtableNext(hashtableIterator *iter, void **elemptr); diff --git a/src/kvstore.c b/src/kvstore.c index bd1574727e..86078cfc1a 100644 --- a/src/kvstore.c +++ b/src/kvstore.c @@ -628,7 +628,7 @@ kvstoreIterator *kvstoreIteratorInit(kvstore *kvs, uint8_t flags) { /* Free the kvs_it returned by kvstoreIteratorInit. */ void kvstoreIteratorRelease(kvstoreIterator *kvs_it) { hashtableIterator *iter = &kvs_it->di; - hashtableResetIterator(iter); + hashtableCleanupIterator(iter); /* In the safe iterator context, we may delete entries. */ if (kvs_it->didx != KVSTORE_INDEX_NOT_FOUND) { freeHashtableIfNeeded(kvs_it->kvs, kvs_it->didx); @@ -672,7 +672,7 @@ static hashtable *kvstoreIteratorNextHashtable(kvstoreIterator *kvs_it) { if (kvs_it->didx != KVSTORE_INDEX_NOT_FOUND && kvstoreGetHashtable(kvs_it->kvs, kvs_it->didx)) { /* Before we move to the next hashtable, reset the iter of the previous hashtable. */ hashtableIterator *iter = &kvs_it->di; - hashtableResetIterator(iter); + hashtableCleanupIterator(iter); /* In the safe iterator context, we may delete entries. */ freeHashtableIfNeeded(kvs_it->kvs, kvs_it->didx); } @@ -697,7 +697,7 @@ bool kvstoreIteratorNext(kvstoreIterator *kvs_it, void **next) { /* No current hashtable or reached the end of the hash table. */ hashtable *ht = kvstoreIteratorNextHashtable(kvs_it); if (!ht) return false; - hashtableReinitIterator(&kvs_it->di, ht); + hashtableRetargetIterator(&kvs_it->di, ht); return hashtableNext(&kvs_it->di, next); } } @@ -779,7 +779,7 @@ kvstoreHashtableIterator *kvstoreGetHashtableIterator(kvstore *kvs, int didx, ui void kvstoreReleaseHashtableIterator(kvstoreHashtableIterator *kvs_di) { /* The hashtable may be deleted during the iteration process, so here need to check for NULL. */ if (kvstoreGetHashtable(kvs_di->kvs, kvs_di->didx)) { - hashtableResetIterator(&kvs_di->di); + hashtableCleanupIterator(&kvs_di->di); /* In the safe iterator context, we may delete entries. */ freeHashtableIfNeeded(kvs_di->kvs, kvs_di->didx); } diff --git a/src/latency.c b/src/latency.c index f39255bac9..7f79510534 100644 --- a/src/latency.c +++ b/src/latency.c @@ -547,7 +547,7 @@ void latencyAllCommandsFillCDF(client *c, hashtable *commands, int *command_with latencyAllCommandsFillCDF(c, cmd->subcommands_ht, command_with_data); } } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } /* latencyCommand() helper to produce for a specific command set, @@ -580,7 +580,7 @@ void latencySpecificCommandsFillCDF(client *c) { command_with_data++; } } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } } setDeferredMapLen(c, replylen, command_with_data); diff --git a/src/module.c b/src/module.c index 56dfbddd8c..d64c244fa7 100644 --- a/src/module.c +++ b/src/module.c @@ -12520,7 +12520,7 @@ int moduleFreeCommand(struct ValkeyModule *module, struct serverCommand *cmd) { sdsfree(sub->fullname); zfree(sub); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); hashtableRelease(cmd->subcommands_ht); } @@ -12544,7 +12544,7 @@ void moduleUnregisterCommands(struct ValkeyModule *module) { sdsfree(cmd->fullname); zfree(cmd); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } /* We parse argv to add sds "NAME VALUE" pairs to the server.module_configs_queue list of configs. diff --git a/src/object.c b/src/object.c index 41a7bd50cd..2cd5eb2992 100644 --- a/src/object.c +++ b/src/object.c @@ -643,7 +643,7 @@ void dismissSetObject(robj *o, size_t size_hint) { sds item = next; dismissSds(item); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } dismissHashtable(ht); @@ -694,7 +694,7 @@ void dismissHashObject(robj *o, size_t size_hint) { while (hashtableNext(&iter, &next)) { entryDismissMemory(next); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } dismissHashtable(ht); @@ -1171,7 +1171,7 @@ size_t objectComputeSize(robj *key, robj *o, size_t sample_size, int dbid) { elesize += sdsAllocSize(element); samples++; } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); if (samples) asize += (double)elesize / samples * hashtableSize(ht); } else if (o->encoding == OBJ_ENCODING_INTSET) { asize += zmalloc_size(o->ptr); @@ -1214,7 +1214,7 @@ size_t objectComputeSize(robj *key, robj *o, size_t sample_size, int dbid) { elesize += entryMemUsage(next); samples++; } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); if (samples) asize += (double)elesize / samples * hashtableSize(ht); if (vsetIsValid(volatile_fields)) asize += vsetMemUsage(volatile_fields); } else { diff --git a/src/pubsub.c b/src/pubsub.c index 0d79b9a3a2..a6371dfc05 100644 --- a/src/pubsub.c +++ b/src/pubsub.c @@ -389,7 +389,7 @@ void pubsubShardUnsubscribeAllChannelsInSlot(unsigned int slot) { unmarkClientAsPubSub(c); } } - hashtableResetIterator(&client_iter); + hashtableCleanupIterator(&client_iter); kvstoreHashtableDelete(server.pubsubshard_channels, slot, channel); } kvstoreReleaseHashtableIterator(kvs_di); @@ -454,7 +454,7 @@ int pubsubUnsubscribeAllChannelsInternal(client *c, int notify, pubsubtype type) while (hashtableNext(&iter, &channel)) { count += pubsubUnsubscribeChannel(c, channel, notify, type); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } /* We were subscribed to nothing? Still reply to the client. */ if (notify && count == 0) { @@ -494,7 +494,7 @@ int pubsubUnsubscribeAllPatterns(client *c, int notify) { count += pubsubUnsubscribePattern(c, pattern, notify); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } /* We were subscribed to nothing? Still reply to the client. */ @@ -527,7 +527,7 @@ int pubsubPublishMessageInternal(robj *channel, robj *message, pubsubtype type) updateClientMemUsageAndBucket(c); receivers++; } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } if (type.shard) { @@ -554,7 +554,7 @@ int pubsubPublishMessageInternal(robj *channel, robj *message, pubsubtype type) updateClientMemUsageAndBucket(c); receivers++; } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } decrRefCount(channel); dictReleaseIterator(di); diff --git a/src/rdb.c b/src/rdb.c index bec340cb05..07b122f591 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -919,12 +919,12 @@ ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key, int dbid) { while (hashtableNext(&iterator, &next)) { sds ele = next; if ((n = rdbSaveRawString(rdb, (unsigned char *)ele, sdslen(ele))) == -1) { - hashtableResetIterator(&iterator); + hashtableCleanupIterator(&iterator); return -1; } nwritten += n; } - hashtableResetIterator(&iterator); + hashtableCleanupIterator(&iterator); } else if (o->encoding == OBJ_ENCODING_INTSET) { size_t l = intsetBlobLen((intset *)o->ptr); @@ -994,25 +994,25 @@ ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key, int dbid) { sds value = entryGetValue(next); if ((n = rdbSaveRawString(rdb, (unsigned char *)field, sdslen(field))) == -1) { - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); return -1; } nwritten += n; if ((n = rdbSaveRawString(rdb, (unsigned char *)value, sdslen(value))) == -1) { - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); return -1; } nwritten += n; if (add_expiry) { long long expiry = entryGetExpiry(next); if ((n = rdbSaveMillisecondTime(rdb, expiry) == -1)) { - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); return -1; } nwritten += n; } } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } else { serverPanic("Unknown hash encoding"); diff --git a/src/server.c b/src/server.c index 453cc3f2d5..b32d7ab61f 100644 --- a/src/server.c +++ b/src/server.c @@ -3344,7 +3344,7 @@ void resetCommandTableStats(hashtable *commands) { } if (c->subcommands_ht) resetCommandTableStats(c->subcommands_ht); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } void resetErrorTableStats(void) { @@ -5218,7 +5218,7 @@ void addReplyCommandSubCommands(client *c, if (use_map) addReplyBulkCBuffer(c, sub->fullname, sdslen(sub->fullname)); reply_function(c, sub); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } /* Output the representation of a server command. Used by the COMMAND command and COMMAND INFO. */ @@ -5379,7 +5379,7 @@ void commandCommand(client *c) { struct serverCommand *cmd = next; addReplyCommandInfo(c, cmd); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } /* COMMAND COUNT */ @@ -5445,7 +5445,7 @@ void commandListWithFilter(client *c, hashtable *commands, commandListFilter fil commandListWithFilter(c, cmd->subcommands_ht, filter, numcmds); } } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } /* COMMAND LIST */ @@ -5462,7 +5462,7 @@ void commandListWithoutFilter(client *c, hashtable *commands, int *numcmds) { commandListWithoutFilter(c, cmd->subcommands_ht, numcmds); } } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } /* COMMAND LIST [FILTERBY (MODULE |ACLCAT |PATTERN )] */ @@ -5519,7 +5519,7 @@ void commandInfoCommand(client *c) { struct serverCommand *cmd = next; addReplyCommandInfo(c, cmd); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } else { addReplyArrayLen(c, c->argc - 2); for (i = 2; i < c->argc; i++) { @@ -5542,7 +5542,7 @@ void commandDocsCommand(client *c) { addReplyBulkCBuffer(c, cmd->fullname, sdslen(cmd->fullname)); addReplyCommandDocs(c, cmd); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } else { /* Reply with an array of the requested commands (if we find them) */ int numcmds = 0; @@ -5682,7 +5682,7 @@ sds genValkeyInfoStringCommandStats(sds info, hashtable *commands) { info = genValkeyInfoStringCommandStats(info, c->subcommands_ht); } } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); return info; } @@ -5717,7 +5717,7 @@ sds genValkeyInfoStringLatencyStats(sds info, hashtable *commands) { info = genValkeyInfoStringLatencyStats(info, c->subcommands_ht); } } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); return info; } diff --git a/src/sort.c b/src/sort.c index 754ebef4a2..0e8e1c6bdd 100644 --- a/src/sort.c +++ b/src/sort.c @@ -456,7 +456,7 @@ void sortCommandGeneric(client *c, int readonly) { vector[j].u.cmpobj = NULL; j++; } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } else { serverPanic("Unknown type"); } diff --git a/src/t_hash.c b/src/t_hash.c index b02ce59eeb..6c1c9037d1 100644 --- a/src/t_hash.c +++ b/src/t_hash.c @@ -613,7 +613,7 @@ void hashTypeInitVolatileIterator(robj *subject, hashTypeIterator *hi) { void hashTypeResetIterator(hashTypeIterator *hi) { if (hi->encoding == OBJ_ENCODING_HASHTABLE) { if (!hi->volatile_items_iter) - hashtableResetIterator(&hi->iter); + hashtableCleanupIterator(&hi->iter); else vsetResetIterator(&hi->viter); } @@ -2051,7 +2051,7 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) { reply_size++; } serverAssert(hashtableSize(ht) == reply_size); - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); /* Remove random elements to reach the right count. */ while (reply_size > count) { @@ -2072,7 +2072,7 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) { if (withvalues) addWritePreparedReplyBulkCBuffer(wpc, value, sdslen(value)); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); hashtableRelease(ht); } diff --git a/src/t_set.c b/src/t_set.c index abd7937640..770b3307d8 100644 --- a/src/t_set.c +++ b/src/t_set.c @@ -1189,7 +1189,7 @@ void srandmemberWithCountCommand(client *c) { serverAssert(count == hashtableSize(ht)); void *element; while (hashtableNext(&iter, &element)) addReplyBulkSds(c, (sds)element); - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); hashtableRelease(ht); } } diff --git a/src/t_zset.c b/src/t_zset.c index 421788dd40..6d1cf88020 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -2342,7 +2342,7 @@ static size_t zsetHashtableGetMaxElementLength(hashtable *ht, size_t *totallen) if (elelen > maxelelen) maxelelen = elelen; if (totallen) (*totallen) += elelen; } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); return maxelelen; } @@ -2745,7 +2745,7 @@ static void zunionInterDiffGenericCommand(client *c, robj *dstkey, int numkeysIn zskiplistNode *node = next; zslInsertNode(dstzset->zsl, node); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); } else if (op == SET_OP_DIFF) { zdiff(src, setnum, dstzset, &maxelelen, &totelelen); } else { @@ -4192,7 +4192,7 @@ void zrandmemberWithCountCommand(client *c, long l, int withscores) { hashtableDelete(ht, ((zskiplistNode *)element)->ele); size--; } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); /* Reply with what's in the temporary hashtable and release memory */ hashtableInitIterator(&iter, ht, 0); @@ -4205,7 +4205,7 @@ void zrandmemberWithCountCommand(client *c, long l, int withscores) { if (withscores) addReplyDouble(c, node->score); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); hashtableRelease(ht); } diff --git a/src/unit/test_files.h b/src/unit/test_files.h index ca557984be..09dbfe5904 100644 --- a/src/unit/test_files.h +++ b/src/unit/test_files.h @@ -41,6 +41,13 @@ 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_safe_iterator_invalidation(int argc, char **argv, int flags); +int test_safe_iterator_empty_no_invalidation(int argc, char **argv, int flags); +int test_safe_iterator_reset_invalidation(int argc, char **argv, int flags); +int test_safe_iterator_reset_untracking(int argc, char **argv, int flags); +int test_safe_iterator_pause_resume_tracking(int argc, char **argv, int flags); +int test_null_hashtable_iterator(int argc, char **argv, int flags); +int test_hashtable_retarget_iterator(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); @@ -261,7 +268,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_safe_iterator_invalidation", test_safe_iterator_invalidation}, {"test_safe_iterator_empty_no_invalidation", test_safe_iterator_empty_no_invalidation}, {"test_safe_iterator_reset_invalidation", test_safe_iterator_reset_invalidation}, {"test_safe_iterator_reset_untracking", test_safe_iterator_reset_untracking}, {"test_safe_iterator_pause_resume_tracking", test_safe_iterator_pause_resume_tracking}, {"test_null_hashtable_iterator", test_null_hashtable_iterator}, {"test_hashtable_retarget_iterator", test_hashtable_retarget_iterator}, {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..ddea23be1a 100644 --- a/src/unit/test_hashtable.c +++ b/src/unit/test_hashtable.c @@ -563,7 +563,7 @@ int test_iterator(int argc, char **argv, int flags) { /* increment entry at this position as a counter */ (*entry)++; } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); /* Check that all entries were returned exactly once. */ TEST_ASSERT(num_returned == count); @@ -616,7 +616,7 @@ int test_safe_iterator(int argc, char **argv, int flags) { TEST_ASSERT(hashtableAdd(ht, entry + count)); } } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); /* Check that all entries present during the whole iteration were returned * exactly once. (Some are deleted after being returned.) */ @@ -682,7 +682,7 @@ int test_compact_bucket_chain(int argc, char **argv, int flags) { hashtableHistogram(ht); } } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); /* Verify that the bucket chain has been compacted by filling the holes and * freeing empty child buckets. */ @@ -945,3 +945,315 @@ int test_random_entry_sparse_table(int argc, char **argv, int flags) { zfree(values); return 0; } + +int test_safe_iterator_invalidation(int argc, char **argv, int flags) { + UNUSED(argc); + UNUSED(argv); + UNUSED(flags); + + hashtableType type = {0}; + hashtable *ht = hashtableCreate(&type); + + /* Add some entries */ + for (int i = 0; i < 10; i++) { + TEST_ASSERT(hashtableAdd(ht, (void *)(long)i)); + } + + /* Create safe and non-safe iterators */ + hashtableIterator safe_iter1, safe_iter2, unsafe_iter; + hashtableInitIterator(&safe_iter1, ht, HASHTABLE_ITER_SAFE); + hashtableInitIterator(&safe_iter2, ht, HASHTABLE_ITER_SAFE); + hashtableInitIterator(&unsafe_iter, ht, 0); + + /* Verify iterators work before invalidation */ + void *entry; + TEST_ASSERT(hashtableNext(&safe_iter1, &entry)); + TEST_ASSERT(hashtableNext(&safe_iter2, &entry)); + + /* Reset iterators to test state */ + hashtableCleanupIterator(&safe_iter1); + hashtableCleanupIterator(&safe_iter2); + hashtableInitIterator(&safe_iter1, ht, HASHTABLE_ITER_SAFE); + hashtableInitIterator(&safe_iter2, ht, HASHTABLE_ITER_SAFE); + + /* Release hashtable - should invalidate safe iterators */ + hashtableRelease(ht); + + /* Test that safe iterators are now invalid */ + TEST_ASSERT(!hashtableNext(&safe_iter1, &entry)); + TEST_ASSERT(!hashtableNext(&safe_iter2, &entry)); + + /* Reset invalidated iterators (should handle gracefully) */ + hashtableCleanupIterator(&safe_iter1); + hashtableCleanupIterator(&safe_iter2); + hashtableCleanupIterator(&unsafe_iter); + + return 0; +} + +int test_safe_iterator_empty_no_invalidation(int argc, char **argv, int flags) { + UNUSED(argc); + UNUSED(argv); + UNUSED(flags); + + hashtableType type = {0}; + hashtable *ht = hashtableCreate(&type); + + /* Add some entries */ + for (int i = 0; i < 5; i++) { + TEST_ASSERT(hashtableAdd(ht, (void *)(long)i)); + } + + /* Create safe iterator */ + hashtableIterator safe_iter; + hashtableInitIterator(&safe_iter, ht, HASHTABLE_ITER_SAFE); + + /* Empty hashtable - should NOT invalidate safe iterators */ + hashtableEmpty(ht, NULL); + + /* Iterator should still be valid but return false since table is empty */ + void *entry; + TEST_ASSERT(!hashtableNext(&safe_iter, &entry)); + + hashtableCleanupIterator(&safe_iter); + hashtableRelease(ht); + + return 0; +} + +int test_safe_iterator_reset_invalidation(int argc, char **argv, int flags) { + UNUSED(argc); + UNUSED(argv); + UNUSED(flags); + + hashtableType type = {0}; + hashtable *ht = hashtableCreate(&type); + + /* Add some entries */ + for (int i = 0; i < 10; i++) { + TEST_ASSERT(hashtableAdd(ht, (void *)(long)i)); + } + + /* Create safe iterators */ + hashtableIterator safe_iter1, safe_iter2; + hashtableInitIterator(&safe_iter1, ht, HASHTABLE_ITER_SAFE); + hashtableInitIterator(&safe_iter2, ht, HASHTABLE_ITER_SAFE); + + /* Verify iterators work before reset */ + void *entry; + TEST_ASSERT(hashtableNext(&safe_iter1, &entry)); + TEST_ASSERT(hashtableNext(&safe_iter2, &entry)); + + /* Reset iterators to test state */ + hashtableCleanupIterator(&safe_iter1); + hashtableCleanupIterator(&safe_iter2); + hashtableInitIterator(&safe_iter1, ht, HASHTABLE_ITER_SAFE); + hashtableInitIterator(&safe_iter2, ht, HASHTABLE_ITER_SAFE); + + /* Reset one iterator before release - should untrack it */ + hashtableCleanupIterator(&safe_iter1); + /* Repeated calls are ok */ + hashtableCleanupIterator(&safe_iter1); + + /* Release hashtable - should invalidate remaining safe iterator */ + hashtableRelease(ht); + + /* Test that safe iterators are prevented from invalid access */ + TEST_ASSERT(!hashtableNext(&safe_iter1, &entry)); + TEST_ASSERT(!hashtableNext(&safe_iter2, &entry)); + + /* Reset invalidated iterators (should handle gracefully) */ + hashtableCleanupIterator(&safe_iter1); + hashtableCleanupIterator(&safe_iter2); + + return 0; +} + +int test_safe_iterator_reset_untracking(int argc, char **argv, int flags) { + UNUSED(argc); + UNUSED(argv); + UNUSED(flags); + + hashtableType type = {0}; + hashtable *ht = hashtableCreate(&type); + + /* Add some entries */ + for (int i = 0; i < 5; i++) { + TEST_ASSERT(hashtableAdd(ht, (void *)(long)i)); + } + + /* Create safe iterator */ + hashtableIterator safe_iter; + hashtableInitIterator(&safe_iter, ht, HASHTABLE_ITER_SAFE); + + /* Reset iterator - should remove from tracking */ + hashtableCleanupIterator(&safe_iter); + + /* Release hashtable - iterator should not be invalidated since it was reset */ + hashtableRelease(ht); + + /* Create new hashtable and reinit iterator */ + ht = hashtableCreate(&type); + for (int i = 0; i < 3; i++) { + TEST_ASSERT(hashtableAdd(ht, (void *)(long)i)); + } + hashtableInitIterator(&safe_iter, ht, HASHTABLE_ITER_SAFE); + + /* Should work since it's tracking the new hashtable */ + void *entry; + TEST_ASSERT(hashtableNext(&safe_iter, &entry)); + + hashtableCleanupIterator(&safe_iter); + hashtableRelease(ht); + + return 0; +} +int test_safe_iterator_pause_resume_tracking(int argc, char **argv, int flags) { + UNUSED(argc); + UNUSED(argv); + UNUSED(flags); + + hashtableType type = {0}; + hashtable *ht = hashtableCreate(&type); + + /* Add entries to trigger rehashing */ + for (int i = 0; i < 100; i++) { + TEST_ASSERT(hashtableAdd(ht, (void *)(long)i)); + } + + /* Create multiple safe iterators */ + hashtableIterator iter1, iter2, iter3; + hashtableInitIterator(&iter1, ht, HASHTABLE_ITER_SAFE); + hashtableInitIterator(&iter2, ht, HASHTABLE_ITER_SAFE); + hashtableInitIterator(&iter3, ht, HASHTABLE_ITER_SAFE); + + /* Start iterating with first iterator - should pause rehashing and track iterator */ + void *entry; + TEST_ASSERT(hashtableNext(&iter1, &entry)); + + /* Start iterating with second iterator - should also be tracked */ + TEST_ASSERT(hashtableNext(&iter2, &entry)); + + /* Verify rehashing is paused (safe iterators should pause it) */ + TEST_ASSERT(hashtableIsRehashingPaused(ht)); + + /* Reset first iterator - should untrack it but rehashing still paused due to iter2 */ + hashtableCleanupIterator(&iter1); + + /* Start third iterator */ + TEST_ASSERT(hashtableNext(&iter3, &entry)); + + /* Reset second iterator - rehashing should still be paused due to iter3 */ + hashtableCleanupIterator(&iter2); + TEST_ASSERT(hashtableIsRehashingPaused(ht)); + + /* Reset third iterator - now rehashing should be resumed */ + hashtableCleanupIterator(&iter3); + + /* Verify all iterators are properly untracked by releasing hashtable */ + hashtableRelease(ht); + + return 0; +} + +int test_null_hashtable_iterator(int argc, char **argv, int flags) { + UNUSED(argc); + UNUSED(argv); + UNUSED(flags); + + /* Test safe iterator with NULL hashtable */ + hashtableIterator safe_iter; + hashtableInitIterator(&safe_iter, NULL, HASHTABLE_ITER_SAFE); + + /* hashtableNext should return false for NULL hashtable */ + void *entry; + TEST_ASSERT(!hashtableNext(&safe_iter, &entry)); + + /* Reset should handle NULL hashtable gracefully */ + hashtableCleanupIterator(&safe_iter); + + /* Test non-safe iterator with NULL hashtable */ + hashtableIterator unsafe_iter; + hashtableInitIterator(&unsafe_iter, NULL, 0); + + /* hashtableNext should return false for NULL hashtable */ + TEST_ASSERT(!hashtableNext(&unsafe_iter, &entry)); + + /* Reset should handle NULL hashtable gracefully */ + hashtableCleanupIterator(&unsafe_iter); + + /* Test reinitializing NULL iterator with valid hashtable */ + hashtableType type = {0}; + hashtable *ht = hashtableCreate(&type); + TEST_ASSERT(hashtableAdd(ht, (void *)1)); + + hashtableRetargetIterator(&safe_iter, ht); + TEST_ASSERT(hashtableNext(&safe_iter, &entry)); + TEST_ASSERT(entry == (void *)1); + + hashtableCleanupIterator(&safe_iter); + hashtableRelease(ht); + + return 0; +} + +int test_hashtable_retarget_iterator(int argc, char **argv, int flags) { + UNUSED(argc); + UNUSED(argv); + UNUSED(flags); + + hashtableType type = {0}; + hashtable *ht1 = hashtableCreate(&type); + hashtable *ht2 = hashtableCreate(&type); + + /* Add different entries to each hashtable */ + for (int i = 0; i < 5; i++) { + TEST_ASSERT(hashtableAdd(ht1, (void *)(long)(i + 10))); + TEST_ASSERT(hashtableAdd(ht2, (void *)(long)(i + 20))); + } + + /* Create iterator on first hashtable */ + hashtableIterator iter; + hashtableInitIterator(&iter, ht1, HASHTABLE_ITER_SAFE); + + /* Iterate partially through first hashtable */ + void *entry; + int count1 = 0; + while (hashtableNext(&iter, &entry) && count1 < 3) { + long val = (long)entry; + TEST_ASSERT(val >= 10 && val < 15); + count1++; + } + + /* Retarget to second hashtable */ + hashtableRetargetIterator(&iter, ht2); + + /* Iterate partially through second hashtable */ + int count2 = 0; + while (hashtableNext(&iter, &entry) && count2 < 2) { + long val = (long)entry; + TEST_ASSERT(val >= 20 && val < 25); + count2++; + } + + /* Retarget back to first hashtable */ + hashtableRetargetIterator(&iter, ht1); + + /* Iterate partially through first hashtable again */ + int count3 = 0; + while (hashtableNext(&iter, &entry) && count3 < 4) { + long val = (long)entry; + TEST_ASSERT(val >= 10 && val < 15); + count3++; + } + + hashtableRelease(ht1); + hashtableRelease(ht2); + + TEST_ASSERT(!hashtableNext(&iter, &entry)); + + hashtableCleanupIterator(&iter); + + return 0; +} + \ No newline at end of file diff --git a/src/valkey-check-rdb.c b/src/valkey-check-rdb.c index 10aa12d313..f437b93594 100644 --- a/src/valkey-check-rdb.c +++ b/src/valkey-check-rdb.c @@ -331,7 +331,7 @@ void computeDatasetProfile(int dbid, robj *keyobj, robj *o, long long expiretime eleLen += sdslen(node->ele) + strlen(buf); statsRecordElementSize(eleLen, 1, stats); } - hashtableResetIterator(&iter); + hashtableCleanupIterator(&iter); statsRecordCount(hashtableSize(zs->ht), stats); } else { serverPanic("Unknown sorted set encoding"); diff --git a/src/valkey-microbench b/src/valkey-microbench new file mode 100755 index 0000000000..023ac33534 Binary files /dev/null and b/src/valkey-microbench differ diff --git a/src/vset.c b/src/vset.c index 1527df8334..d7e2fdcb22 100644 --- a/src/vset.c +++ b/src/vset.c @@ -1450,7 +1450,7 @@ static inline size_t vsetBucketRemoveExpired_HASHTABLE(vsetBucket **bucket, vset expiryFunc(entry, ctx); count++; } - hashtableResetIterator(&it); + hashtableCleanupIterator(&it); /* in case we completed scanning the hashtable which is now empty */ size_t ht_size = hashtableSize(ht); @@ -1557,7 +1557,7 @@ static inline int vsetBucketNext_HASHTABLE(vsetInternalIterator *it, void **entr hashtableInitIterator(&it->hiter, ht, 0); } if (!hashtableNext(&it->hiter, &it->entry)) { - hashtableResetIterator(&it->hiter); + hashtableCleanupIterator(&it->hiter); return 0; } if (entryptr) *entryptr = it->entry; @@ -2195,7 +2195,7 @@ void vsetResetIterator(vsetIterator *iter) { if (parent_bucket_type == VSET_BUCKET_RAX) raxStop(&it->riter); if (bucket_type == VSET_BUCKET_HT) - hashtableResetIterator(&it->hiter); + hashtableCleanupIterator(&it->hiter); } /* Initializes an empty volatile set. diff --git a/src/vset.h b/src/vset.h index 8c1074c2cb..23270395ac 100644 --- a/src/vset.h +++ b/src/vset.h @@ -75,7 +75,7 @@ typedef int (*vsetExpiryFunc)(void *entry, void *ctx); // vset is just a pointer to a bucket typedef void *vset; -typedef uint8_t vsetIterator[560]; +typedef uint8_t vsetIterator[600]; bool vsetAddEntry(vset *set, vsetGetExpiryFunc getExpiry, void *entry); bool vsetRemoveEntry(vset *set, vsetGetExpiryFunc getExpiry, void *entry);