From: Peter Dillinger <peterd@fb.com>
Date: Wed, 28 Jul 2021 04:30:54 +0000 (-0700)
Subject: Fix missing Handle release in TableCache::GetRangeTombstoneIterator (#8589)
X-Git-Tag: v6.24.2~77
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=e352bd574275a4ab7612831e8308ee08116809a7;p=rocksdb.git

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 8b7bba7cf..918fadbaf 100644
--- 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 2e4d2a58a..a80f69223 100644
--- 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;
 }