diff --git a/HISTORY.md b/HISTORY.md index f22c5115a..45c4b7761 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -2,8 +2,9 @@ ## Unreleased ### Bug Fixes * Fix a bug in io_uring_prep_cancel in AbortIO API for posix which expects sqe->addr to match with read request submitted and wrong paramter was being passed. -* Fixed a regression in iterator performance when the entire DB is a single memtable introduced in #10449. The fix is in #10705 and #10716. +* Fixed a regression in iterator performance when the entire DB is a single memtable introduced in #10449. The fix is in #10705 and #10716. * Fixed an optimistic transaction validation bug caused by DBImpl::GetLatestSequenceForKey() returning non-latest seq for merge (#10724). +* Fixed a bug in iterator refresh which could segfault for DeleteRange users (#10739). ## 7.7.0 (09/18/2022) ### Bug Fixes diff --git a/db/arena_wrapped_db_iter.cc b/db/arena_wrapped_db_iter.cc index d2552a2af..b7959491c 100644 --- a/db/arena_wrapped_db_iter.cc +++ b/db/arena_wrapped_db_iter.cc @@ -45,6 +45,7 @@ void ArenaWrappedDBIter::Init( sv_number_ = version_number; read_options_ = read_options; allow_refresh_ = allow_refresh; + memtable_range_tombstone_iter_ = nullptr; } Status ArenaWrappedDBIter::Refresh() { @@ -92,9 +93,15 @@ Status ArenaWrappedDBIter::Refresh() { auto t = sv->mem->NewRangeTombstoneIterator( read_options_, latest_seq, false /* immutable_memtable */); if (!t || t->empty()) { + // If memtable_range_tombstone_iter_ points to a non-empty tombstone + // iterator, then it means sv->mem is not the memtable that + // memtable_range_tombstone_iter_ points to, so SV must have changed + // after the sv_number_ != cur_sv_number check above. We will fall + // back to re-init the InternalIterator, and the tombstone iterator + // will be freed during db_iter destruction there. if (memtable_range_tombstone_iter_) { - delete *memtable_range_tombstone_iter_; - *memtable_range_tombstone_iter_ = nullptr; + assert(!*memtable_range_tombstone_iter_ || + sv_number_ != cfd_->GetSuperVersionNumber()); } delete t; } else { // current mutable memtable has range tombstones diff --git a/db/db_iter.cc b/db/db_iter.cc index 6e026763e..bce471129 100644 --- a/db/db_iter.cc +++ b/db/db_iter.cc @@ -89,6 +89,7 @@ DBIter::DBIter(Env* _env, const ReadOptions& read_options, if (iter_.iter()) { iter_.iter()->SetPinnedItersMgr(&pinned_iters_mgr_); } + status_.PermitUncheckedError(); assert(timestamp_size_ == user_comparator_.timestamp_size()); } diff --git a/db/db_range_del_test.cc b/db/db_range_del_test.cc index a6e8d05f5..0b6962b64 100644 --- a/db/db_range_del_test.cc +++ b/db/db_range_del_test.cc @@ -2687,6 +2687,23 @@ TEST_F(DBRangeDelTest, PrefixSentinelKey) { delete iter; } +TEST_F(DBRangeDelTest, RefreshMemtableIter) { + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + DestroyAndReopen(options); + ASSERT_OK( + db_->DeleteRange(WriteOptions(), db_->DefaultColumnFamily(), "a", "z")); + ReadOptions ro; + ro.read_tier = kMemtableTier; + std::unique_ptr iter{db_->NewIterator(ro)}; + ASSERT_OK(Flush()); + // First refresh reinits iter, which had a bug where + // iter.memtable_range_tombstone_iter_ was not set to nullptr, and caused + // subsequent refresh to double free. + ASSERT_OK(iter->Refresh()); + ASSERT_OK(iter->Refresh()); +} + #endif // ROCKSDB_LITE } // namespace ROCKSDB_NAMESPACE