Fix reference counting failure in the dict. This is caused by std::swap also swapping refcounts

JohnSully · JohnSully · commit a1ae21d64639 · 2022-05-16T22:34:50.000Z
diff --git a/src/dict.cpp b/src/dict.cpp
@@ -204,15 +204,15 @@ int dictMerge(dict *dst, dict *src)
     
     if (dictSize(dst) == 0)
     {
-        std::swap(*dst, *src);
+        dict::swap(*dst, *src);
         std::swap(dst->pauserehash, src->pauserehash);
         return DICT_OK;
     }
 
     size_t expectedSize = dictSize(src) + dictSize(dst);
     if (dictSize(src) > dictSize(dst) && src->asyncdata == nullptr && dst->asyncdata == nullptr)
     {
-        std::swap(*dst, *src);
+        dict::swap(*dst, *src);
         std::swap(dst->pauserehash, src->pauserehash);
     }
 
@@ -465,6 +465,18 @@ bool dictRehashSomeAsync(dictAsyncRehashCtl *ctl, size_t hashes) {
     return ctl->hashIdx < ctl->queue.size();
 }
 
+
+void discontinueAsyncRehash(dict *d) {
+    if (d->asyncdata != nullptr) {
+        auto adata = d->asyncdata;
+        while (adata != nullptr) {
+            adata->abondon = true;
+            adata = adata->next;
+        }
+        d->rehashidx = 0;
+    }
+}
+
 void dictCompleteRehashAsync(dictAsyncRehashCtl *ctl, bool fFree) {
     dict *d = ctl->dict;
     assert(ctl->done);
diff --git a/src/dict.h b/src/dict.h
@@ -110,12 +110,16 @@ struct dictAsyncRehashCtl {
     std::atomic<bool> abondon { false };
 
     dictAsyncRehashCtl(struct dict *d, dictAsyncRehashCtl *next);
+    dictAsyncRehashCtl(const dictAsyncRehashCtl&) = delete;
+    dictAsyncRehashCtl(dictAsyncRehashCtl&&) = delete;
     ~dictAsyncRehashCtl();
 };
 #else
 struct  dictAsyncRehashCtl;
 #endif
 
+void discontinueAsyncRehash(dict *d);
+
 typedef struct dict {
     dictType *type;
     void *privdata;
@@ -125,6 +129,24 @@ typedef struct dict {
     dictAsyncRehashCtl *asyncdata;
     int16_t pauserehash; /* If >0 rehashing is paused (<0 indicates coding error) */
     uint8_t noshrink = false;
+
+#ifdef __cplusplus
+    dict() = default;
+    dict(dict &) = delete;  // No Copy Ctor
+
+    static void swap(dict& a, dict& b) {
+        discontinueAsyncRehash(&a);
+        discontinueAsyncRehash(&b);
+        std::swap(a.type, b.type);
+        std::swap(a.privdata, b.privdata);
+        std::swap(a.ht[0], b.ht[0]);
+        std::swap(a.ht[1], b.ht[1]);
+        std::swap(a.rehashidx, b.rehashidx);
+        // Never swap refcount - they are attached to the specific dict obj
+        std::swap(a.pauserehash, b.pauserehash);
+        std::swap(a.noshrink, b.noshrink);
+    }
+#endif
 } dict;
 
 /* If safe is set to 1 this is a safe iterator, that means, you can call
diff --git a/src/snapshot.cpp b/src/snapshot.cpp
@@ -26,17 +26,6 @@ class LazyFree : public ICollectable
     std::vector<dictEntry*> vecde;
 };
 
-void discontinueAsyncRehash(dict *d) {
-    if (d->asyncdata != nullptr) {
-        auto adata = d->asyncdata;
-        while (adata != nullptr) {
-            adata->abondon = true;
-            adata = adata->next;
-        }
-        d->rehashidx = 0;
-    }
-}
-
 const redisDbPersistentDataSnapshot *redisDbPersistentData::createSnapshot(uint64_t mvccCheckpoint, bool fOptional)
 {
     serverAssert(GlobalLocksAcquired());
