Fix missing Handle release in TableCache::GetRangeTombstoneIterator (#8589)

Summary:
This appears to be little used code so not a major bug, but is
blocking https://github.com/facebook/rocksdb/issues/8544

Pull Request resolved: https://github.com/facebook/rocksdb/pull/8589

Test Plan:
Added regression test to the end of
DBRangeDelTest::TableEvictedDuringScan. Without this fix, ASAN reports
memory leak.

Reviewed By: ajkr

Differential Revision: D29943623

Pulled By: pdillinger

fbshipit-source-id: f7115fa6d4440aef83888ff609aa03d09216463b

diff --git a/db/db_range_del_test.cc b/db/db_range_del_test.cc
index 8b7bba7cfc6cceb5419e87be63e60804028c9b43..918fadbaf930b10a95169ba6e4fa45247f0351db 100644 (file)
--- a/db/db_range_del_test.cc
+++ b/db/db_range_del_test.cc
@@ -660,6 +660,16 @@ TEST_F(DBRangeDelTest, TableEvictedDuringScan) {
   ASSERT_EQ(kNum, expected);
   delete iter;
   db_->ReleaseSnapshot(snapshot);
+
+  // Also test proper cache handling in GetRangeTombstoneIterator,
+  // via TablesRangeTombstoneSummary. (This once triggered memory leak
+  // report with ASAN.)
+  opts.max_open_files = 1;
+  Reopen(opts);
+
+  std::string str;
+  ASSERT_OK(dbfull()->TablesRangeTombstoneSummary(db_->DefaultColumnFamily(),
+                                                  100, &str));
 }
 
 TEST_F(DBRangeDelTest, GetCoveredKeyFromMutableMemtable) {

diff --git a/db/table_cache.cc b/db/table_cache.cc
index 2e4d2a58ae474256439fc10c646adccb7d30f13a..a80f69223ec781fe6e00839fd5f6e23d17902d57 100644 (file)
--- a/db/table_cache.cc
+++ b/db/table_cache.cc
@@ -297,6 +297,7 @@ Status TableCache::GetRangeTombstoneIterator(
     const InternalKeyComparator& internal_comparator,
     const FileMetaData& file_meta,
     std::unique_ptr<FragmentedRangeTombstoneIterator>* out_iter) {
+  assert(out_iter);
   const FileDescriptor& fd = file_meta.fd;
   Status s;
   TableReader* t = fd.table_reader;
@@ -308,8 +309,15 @@ Status TableCache::GetRangeTombstoneIterator(
     }
   }
   if (s.ok()) {
+    // Note: NewRangeTombstoneIterator could return nullptr
     out_iter->reset(t->NewRangeTombstoneIterator(options));
-    assert(out_iter);
+  }
+  if (handle) {
+    if (*out_iter) {
+      (*out_iter)->RegisterCleanup(&UnrefEntry, cache_, handle);
+    } else {
+      ReleaseHandle(handle);
+    }
   }
   return s;
 }