From 51d3e2cbbc8f567a69537e0aa34ff7a0ac661492 Mon Sep 17 00:00:00 2001 From: John Sully Date: Mon, 16 May 2022 22:34:50 +0000 Subject: [PATCH] Fix reference counting failure in the dict. This is caused by std::swap also swapping refcounts --- src/dict.cpp | 16 ++++++++++++++-- src/dict.h | 22 ++++++++++++++++++++++ src/snapshot.cpp | 11 ----------- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/dict.cpp b/src/dict.cpp index e280d6071..7bf549d1f 100644 --- a/src/dict.cpp +++ b/src/dict.cpp @@ -204,7 +204,7 @@ 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; } @@ -212,7 +212,7 @@ int dictMerge(dict *dst, dict *src) 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 index 72d50dd2c..9fbe8ce4f 100644 --- a/src/dict.h +++ b/src/dict.h @@ -110,12 +110,16 @@ struct dictAsyncRehashCtl { std::atomic 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 index de3398818..dca1071d4 100644 --- a/src/snapshot.cpp +++ b/src/snapshot.cpp @@ -26,17 +26,6 @@ public: std::vector 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());