MB-21568: Don't double-free newdb if rewind_db_header fails

If couchstore_rewind_db_header() fails, then it will free the DB
before returning, so we need to ensure that the DbHolder doesn't cause
a double-free by also trying to free the Db.

Found during merge to master where we have improved couchstore testing
(CouchKVStoreErrorInjectionTest)

Change-Id: I49ba9e7e91eb73aaae90ef8aa8b41f56bbe056c9
Reviewed-on: http://review.couchbase.org/70080
Reviewed-by: Jim Walker <jim@couchbase.com>
Tested-by: buildbot <build@couchbase.com>

diff --git a/src/couch-kvstore/couch-kvstore.cc b/src/couch-kvstore/couch-kvstore.cc
index c5080b2..6fcbc86 100644 (file)
--- a/src/couch-kvstore/couch-kvstore.cc
+++ b/src/couch-kvstore/couch-kvstore.cc
@@ -2317,6 +2317,9 @@ RollbackResult CouchKVStore::rollback(uint16_t vbid, uint64_t rollbackSeqno,
     while (info.last_sequence > rollbackSeqno) {
         errCode = couchstore_rewind_db_header(newdb.getDb());
         if (errCode != COUCHSTORE_SUCCESS) {
+            // rewind_db_header cleans up (frees DB) on error; so
+            // release db in DbHolder to prevent a double-free.
+            newdb.releaseDb();
             LOG(EXTENSION_LOG_WARNING,
                     "Failed to rewind Db pointer "
                     "for couch file with vbid: %u, whose "

diff --git a/src/couch-kvstore/couch-kvstore.h b/src/couch-kvstore/couch-kvstore.h
index d6e7ffe..69f011d 100644 (file)
--- a/src/couch-kvstore/couch-kvstore.h
+++ b/src/couch-kvstore/couch-kvstore.h
@@ -504,6 +504,12 @@ private:
             return db;
         }
 
+        Db* releaseDb() {
+            auto* result = db;
+            db = nullptr;
+            return result;
+        }
+
         ~DbHolder() {
             if (db) {
                 kvstore->closeDatabaseHandle(db);