This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push: new 3912a97cd KUDU-3570 fix use-after-free in MajorDeltaCompactionOp 3912a97cd is described below commit 3912a97cd8998ef04c4e6f9c38bd365c582e8171 Author: Alexey Serbin <ale...@apache.org> AuthorDate: Fri Apr 26 16:55:37 2024 -0700 KUDU-3570 fix use-after-free in MajorDeltaCompactionOp This patch addresses heap-use-after-free and data race issues reported in KUDU-3570. With this and one prior patch, neither TSAN nor ASAN reports any warnings when running alter_table-randomized-test, at least that's the stats collected from more than 100 iterations. Change-Id: I491c6d98bed8780bcfb62f152db471d7a260d305 Reviewed-on: http://gerrit.cloudera.org:8080/21362 Tested-by: Alexey Serbin <ale...@apache.org> Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com> --- src/kudu/tablet/diskrowset.cc | 22 +++++++++++++--------- src/kudu/tablet/diskrowset.h | 3 ++- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/kudu/tablet/diskrowset.cc b/src/kudu/tablet/diskrowset.cc index f6c92c9ed..47b616fa4 100644 --- a/src/kudu/tablet/diskrowset.cc +++ b/src/kudu/tablet/diskrowset.cc @@ -580,10 +580,19 @@ Status DiskRowSet::MajorCompactDeltaStoresWithColumnIds(const vector<ColumnId>& std::lock_guard<Mutex> l(*mutable_delta_tracker()->compact_flush_lock()); RETURN_NOT_OK(mutable_delta_tracker()->CheckWritableUnlocked()); + // Keep a reference to the tablet's schema. This is to prevent race condition + // when concurrently running Tablet::AlterSchema() destroys the underlying + // Schema object by calling TabletMetadata::SetSchema(). + // // TODO(todd): do we need to lock schema or anything here? + SchemaPtr schema_ptr = rowset_metadata_->tablet_schema(); + + RowIteratorOptions opts; + opts.projection = schema_ptr.get(); + opts.io_context = io_context; unique_ptr<MajorDeltaCompaction> compaction; - RETURN_NOT_OK(NewMajorDeltaCompaction(col_ids, std::move(history_gc_opts), - io_context, &compaction)); + RETURN_NOT_OK(NewMajorDeltaCompaction( + col_ids, opts, std::move(history_gc_opts), &compaction)); RETURN_NOT_OK(compaction->Compact(io_context)); @@ -629,24 +638,19 @@ Status DiskRowSet::MajorCompactDeltaStoresWithColumnIds(const vector<ColumnId>& } Status DiskRowSet::NewMajorDeltaCompaction(const vector<ColumnId>& col_ids, + const RowIteratorOptions& opts, HistoryGcOpts history_gc_opts, - const IOContext* io_context, unique_ptr<MajorDeltaCompaction>* out) const { DCHECK(open_); shared_lock<rw_spinlock> l(component_lock_); - const SchemaPtr schema_ptr = rowset_metadata_->tablet_schema(); - - RowIteratorOptions opts; - opts.projection = schema_ptr.get(); - opts.io_context = io_context; vector<shared_ptr<DeltaStore>> included_stores; unique_ptr<DeltaIterator> delta_iter; RETURN_NOT_OK(delta_tracker_->NewDeltaFileIterator( opts, REDO, &included_stores, &delta_iter)); out->reset(new MajorDeltaCompaction(rowset_metadata_->fs_manager(), - *schema_ptr, + *opts.projection, base_data_.get(), std::move(delta_iter), std::move(included_stores), diff --git a/src/kudu/tablet/diskrowset.h b/src/kudu/tablet/diskrowset.h index ea1250e3e..a6b75ee6f 100644 --- a/src/kudu/tablet/diskrowset.h +++ b/src/kudu/tablet/diskrowset.h @@ -500,9 +500,10 @@ class DiskRowSet : Status Open(const fs::IOContext* io_context); // Create a new major delta compaction object to compact the specified columns. + // TODO(aserbin): use the move semantics for RowIteratorOptions Status NewMajorDeltaCompaction(const std::vector<ColumnId>& col_ids, + const RowIteratorOptions& opts, HistoryGcOpts history_gc_opts, - const fs::IOContext* io_context, std::unique_ptr<MajorDeltaCompaction>* out) const; // Major compacts all the delta files for the specified columns.