quick fix

facebook · Feb 6, 2022 · c79bf82 · c79bf82
1 parent 7d1a6ac
commit c79bf82
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 8 deletions.
diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc
@@ -1881,6 +1881,8 @@ Status DBImpl::GetImpl(const ReadOptions& read_options, const Slice& key,
       return s;
     }
   }
+  std::shared_ptr<PinnedIteratorsManager> pinned_iters_mgr(
+      new PinnedIteratorsManager());
   if (!done) {
     PERF_TIMER_GUARD(get_from_output_files_time);
     sv->current->Get(
@@ -1890,7 +1892,7 @@ Status DBImpl::GetImpl(const ReadOptions& read_options, const Slice& key,
         nullptr, nullptr,
         get_impl_options.get_value ? get_impl_options.callback : nullptr,
         get_impl_options.get_value ? get_impl_options.is_blob_index : nullptr,
-        get_impl_options.get_value);
+        get_impl_options.get_value, pinned_iters_mgr);
     RecordTick(stats_, MEMTABLE_MISS);
   }
 

diff --git a/db/db_merge_operand_test.cc b/db/db_merge_operand_test.cc
@@ -54,7 +54,8 @@ TEST_F(DBMergeOperandTest, PinSelfUseAfterFree) {
   options.env = env_;
   BlockBasedTableOptions table_options;
 
-  // Small cache to simulate cache full
+  // Small cache to simulate cache full to repro UseAfterFree in pinning merge
+  // operands
   table_options.block_cache = NewLRUCache(1);
   options.table_factory.reset(NewBlockBasedTableFactory(table_options));
 

diff --git a/db/version_set.cc b/db/version_set.cc
@@ -1967,7 +1967,8 @@ void Version::Get(const ReadOptions& read_options, const LookupKey& k,
                   MergeContext* merge_context,
                   SequenceNumber* max_covering_tombstone_seq, bool* value_found,
                   bool* key_exists, SequenceNumber* seq, ReadCallback* callback,
-                  bool* is_blob, bool do_merge) {
+                  bool* is_blob, bool do_merge,
+                  std::shared_ptr<PinnedIteratorsManager> pinned_iters_mgr) {
   Slice ikey = k.internal_key();
   Slice user_key = k.user_key();
 
@@ -1978,7 +1979,6 @@ void Version::Get(const ReadOptions& read_options, const LookupKey& k,
     *key_exists = true;
   }
 
-  PinnedIteratorsManager pinned_iters_mgr;
   uint64_t tracing_get_id = BlockCacheTraceHelper::kReservedGetId;
   if (vset_ && vset_->block_cache_tracer_ &&
       vset_->block_cache_tracer_->is_tracing_enabled()) {
@@ -1997,12 +1997,12 @@ void Version::Get(const ReadOptions& read_options, const LookupKey& k,
       status->ok() ? GetContext::kNotFound : GetContext::kMerge, user_key,
       do_merge ? value : nullptr, do_merge ? timestamp : nullptr, value_found,
       merge_context, do_merge, max_covering_tombstone_seq, clock_, seq,
-      merge_operator_ ? &pinned_iters_mgr : nullptr, callback, is_blob_to_use,
-      tracing_get_id, &blob_fetcher);
+      merge_operator_ ? pinned_iters_mgr.get() : nullptr, callback,
+      is_blob_to_use, tracing_get_id, &blob_fetcher);
 
   // Pin blocks that we read to hold merge operands
   if (merge_operator_) {
-    pinned_iters_mgr.StartPinning();
+    pinned_iters_mgr->StartPinning();
   }
 
   FilePicker fp(user_key, ikey, &storage_info_.level_files_brief_,

diff --git a/db/version_set.h b/db/version_set.h
@@ -703,7 +703,8 @@ class Version {
            SequenceNumber* max_covering_tombstone_seq,
            bool* value_found = nullptr, bool* key_exists = nullptr,
            SequenceNumber* seq = nullptr, ReadCallback* callback = nullptr,
-           bool* is_blob = nullptr, bool do_merge = true);
+           bool* is_blob = nullptr, bool do_merge = true,
+           std::shared_ptr<PinnedIteratorsManager> pinned_iters_mgr = nullptr);
 
   void MultiGet(const ReadOptions&, MultiGetRange* range,
                 ReadCallback* callback = nullptr);