Fix leak when shrinking a hashtable without entries (#2288)

Fixes #2271

When we shrink a hash table and it is empty, we do it without iterating
over it to rehash the entries. However, there may still be empty child
buckets (`used[0]==0 && child_buckets[0]!=0`). These were leaked in this
case.

This fix is to check for child buckets and don't skip the incremental
rehashing if any child buckets exist. The incremental rehashing pass
will free them.

An additional fix is to compact bucket chains in scan when the scan
callback has deleted some entries. This was already implemented for the
case when rehashing is ongoing but it was missing in the case rehashing
is not ongoing.

Additionally, a test case for #2257 was added.

---------

Signed-off-by: yzc-yzc <[email protected]>
Co-authored-by: Viktor Söderqvist <[email protected]>
Co-authored-by: Yakov Gusakov <[email protected]>

Author: 3 people

src/hashtable.c

@@ -649,7 +649,7 @@ static int resize(hashtable *ht, size_t min_capacity, int *malloc_failed) {
     if (ht->type->rehashingStarted) ht->type->rehashingStarted(ht);
 
     /* If the old table was empty, the rehashing is completed immediately. */
-    if (ht->tables[0] == NULL || ht->used[0] == 0) {
+    if (ht->tables[0] == NULL || (ht->used[0] == 0 && ht->child_buckets[0] == 0)) {
         rehashingCompleted(ht);
     } else if (ht->type->instant_rehashing) {
         while (hashtableIsRehashing(ht)) {
@@ -1718,7 +1718,9 @@ size_t hashtableScanDefrag(hashtable *ht, size_t cursor, hashtableScanFunction f
     if (!hashtableIsRehashing(ht)) {
         /* Emit entries at the cursor index. */
         size_t mask = expToMask(ht->bucket_exp[0]);
-        bucket *b = &ht->tables[0][cursor & mask];
+        size_t idx = cursor & mask;
+        size_t used_before = ht->used[0];
+        bucket *b = &ht->tables[0][idx];
         do {
             if (b->presence != 0) {
                 int pos;
@@ -1736,6 +1738,11 @@ size_t hashtableScanDefrag(hashtable *ht, size_t cursor, hashtableScanFunction f
             b = next;
         } while (b != NULL);
 
+        /* If any entries were deleted, fill the holes. */
+        if (ht->used[0] < used_before) {
+            compactBucketChain(ht, idx, 0);
+        }
+
         /* Advance cursor. */
         cursor = nextCursor(cursor, mask);
     } else {

tests/unit/expire.tcl

@@ -951,6 +951,43 @@ start_server {tags {"expire"}} {
     }
 }
 
+start_server {tags {expire} overrides {hz 100}} {
+    test {Active expiration triggers hashtable shrink} {
+        set persistent_keys 5
+        set volatile_keys 100
+        set total_keys [expr $persistent_keys + $volatile_keys]
+
+        for {set i 0} {$i < $persistent_keys} {incr i} {
+            r set "key_$i" "value_$i"
+        }
+        for {set i 0} {$i < $volatile_keys} {incr i} {
+            r psetex "expire_key_${i}" 100 "expire_value_${i}"
+        }
+        set table_size_before_expire [main_hash_table_size]
+
+        # Verify keys are set
+        assert_equal $total_keys [r dbsize]
+
+        # Wait for active expiration
+        wait_for_condition 100 50 {
+            [r dbsize] eq $persistent_keys
+        } else {
+            fail "Keys not expired"
+        }
+
+        # Wait for the table to shrink and active rehashing finish
+        wait_for_condition 100 50 {
+            [main_hash_table_size] < $table_size_before_expire
+        } else {
+            puts [r debug htstats 9]
+            fail "Table didn't shrink"
+        }
+
+        # Verify server is still responsive
+        assert_equal [r ping] {PONG}
+    } {} {needs:debug}
+}
+
 start_cluster 1 0 {tags {"expire external:skip cluster"}} {
     test "expire scan should skip dictionaries with lot's of empty buckets" {
         r debug set-active-expire 0