fixed leaking log::Writers

Summary: Fixes valgrind errors in column_family_test.

Test Plan: `make check`, `make valgrind_check`

Reviewers: igor, yhchiang

Reviewed By: yhchiang

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D41181
(cherry picked from commit 4ce5be4255f654442986f0a9c0269e41cce7d362)

diff --git a/db/column_family.cc b/db/column_family.cc
index 4a600f6116590cc1a98a2ffd7f7869e7c9a56af0..b39518e08844a565c337fe9d8a82b1d7ce74e6e2 100644 (file)
--- a/db/column_family.cc
+++ b/db/column_family.cc
@@ -59,6 +59,7 @@ ColumnFamilyHandleImpl::~ColumnFamilyHandleImpl() {
     if (job_context.HaveSomethingToDelete()) {
       db_->PurgeObsoleteFiles(job_context);
     }
+    job_context.Clean();
   }
 }
 

diff --git a/db/db_impl.cc b/db/db_impl.cc
index c599f1ef298c505f593e159605ec75da58d5a907..66eac6ad07085cd8ff273d148f9d69a6b2d136c0 100644 (file)
--- a/db/db_impl.cc
+++ b/db/db_impl.cc
@@ -542,6 +542,7 @@ void DBImpl::FindObsoleteFiles(JobContext* job_context, bool force,
   }
 
   // We're just cleaning up for DB::Write().
+  assert(job_context->logs_to_free.empty());
   job_context->logs_to_free = logs_to_free_;
   logs_to_free_.clear();
 

diff --git a/db/db_test.cc b/db/db_test.cc
index 9a8fe6d9ef2411e5b639bbf82ab3b3712fd88e93..cb919c09f7f76a11adfb3c0ad1e461f00462c2c3 100644 (file)
--- a/db/db_test.cc
+++ b/db/db_test.cc
@@ -12670,6 +12670,7 @@ TEST_F(DBTest, DontDeletePendingOutputs) {
     dbfull()->FindObsoleteFiles(&job_context, true /*force*/);
     dbfull()->TEST_UnlockMutex();
     dbfull()->PurgeObsoleteFiles(job_context);
+    job_context.Clean();
   };
 
   env_->table_write_callback_ = &purge_obsolete_files_function;

diff --git a/db/forward_iterator.cc b/db/forward_iterator.cc
index b4410199e2679307c7495327ac17bd35e5d53c0c..5ed125930e487806a894f394ebae8fa6b4a997eb 100644 (file)
--- a/db/forward_iterator.cc
+++ b/db/forward_iterator.cc
@@ -169,6 +169,7 @@ void ForwardIterator::Cleanup(bool release_sv) {
       if (job_context.HaveSomethingToDelete()) {
         db_->PurgeObsoleteFiles(job_context);
       }
+      job_context.Clean();
     }
   }
 }

diff --git a/db/job_context.h b/db/job_context.h
index d0281443e80bca7abb01dc381cb73cc91ff9ed9c..5a54e2d85f96bccbd45d742ef9fa28dc444561e9 100644 (file)
--- a/db/job_context.h
+++ b/db/job_context.h
@@ -83,6 +83,10 @@ struct JobContext {
     new_superversion = create_superversion ? new SuperVersion() : nullptr;
   }
 
+  // For non-empty JobContext Clean() has to be called at least once before
+  // before destruction (see asserts in ~JobContext()). Should be called with
+  // unlocked DB mutex. Destructor doesn't call Clean() to avoid accidentally
+  // doing potentially slow Clean() with locked DB mutex.
   void Clean() {
     // free pending memtables
     for (auto m : memtables_to_free) {
@@ -109,6 +113,7 @@ struct JobContext {
     assert(memtables_to_free.size() == 0);
     assert(superversions_to_free.size() == 0);
     assert(new_superversion == nullptr);
+    assert(logs_to_free.size() == 0);
   }
 };