From f7e6c856ab3fa3c37c5cd3f8ba2a7cf59f037385 Mon Sep 17 00:00:00 2001
From: sdong <siying.d@fb.com>
Date: Fri, 31 Oct 2014 12:16:35 -0700
Subject: [PATCH] Fix BaseReferencedVersionBuilder's destructor order

Summary: BaseReferencedVersionBuilder now unreference version before destructing VersionBuilder, which is wrong. Fix it.

Test Plan:
make all check
valgrind test to tests that used to fail

Reviewers: igor, yhchiang, rven, ljin

Reviewed By: ljin

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D28101
---
 db/version_set.cc | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/db/version_set.cc b/db/version_set.cc
index fc37460b8..3c2c0d42e 100644
--- a/db/version_set.cc
+++ b/db/version_set.cc
@@ -515,19 +515,24 @@ class LevelFileIteratorState : public TwoLevelIteratorState {
 
 // A wrapper of version builder which references the current version in
 // constructor and unref it in the destructor.
+// Both of the constructor and destructor need to be called inside DB Mutex.
 class BaseReferencedVersionBuilder {
  public:
   explicit BaseReferencedVersionBuilder(ColumnFamilyData* cfd)
-      : version_builder_(cfd->current()->version_set()->GetEnvOptions(),
-                         cfd->table_cache(), cfd->current()->storage_info()),
+      : version_builder_(new VersionBuilder(
+            cfd->current()->version_set()->GetEnvOptions(), cfd->table_cache(),
+            cfd->current()->storage_info())),
         version_(cfd->current()) {
     version_->Ref();
   }
-  ~BaseReferencedVersionBuilder() { version_->Unref(); }
-  VersionBuilder* GetVersionBuilder() { return &version_builder_; }
+  ~BaseReferencedVersionBuilder() {
+    delete version_builder_;
+    version_->Unref();
+  }
+  VersionBuilder* GetVersionBuilder() { return version_builder_; }
 
  private:
-  VersionBuilder version_builder_;
+  VersionBuilder* version_builder_;
   Version* version_;
 };
 }  // anonymous namespace
@@ -2322,7 +2327,7 @@ Status VersionSet::Recover(
     for (auto cfd : *column_family_set_) {
       auto builders_iter = builders.find(cfd->GetID());
       assert(builders_iter != builders.end());
-      auto builder = builders_iter->second->GetVersionBuilder();
+      auto* builder = builders_iter->second->GetVersionBuilder();
 
       if (db_options_->max_open_files == -1) {
       // unlimited table cache. Pre-load table handle now.