[fix](be) Own the allocated LSN map in SharedMemtable to fix UAF on flush task teardown - #67442
Open
bosswnx wants to merge 1 commit into
Open
[fix](be) Own the allocated LSN map in SharedMemtable to fix UAF on flush task teardown#67442bosswnx wants to merge 1 commit into
bosswnx wants to merge 1 commit into
Conversation
…lush task teardown PartOfGroupMemtableFlushTask holds its SharedMemtable by shared_ptr but only a weak_ptr to the FlushToken. When run()'s local shared_ptr<FlushToken> drops the last reference at the end of run(), destruction cascades FlushToken -> RowsetWriter -> RowsetWriterContext, while the thread pool destroys the task object right after; ~SharedMemtable() then dereferences the raw RowsetWriterContext* rowset_ctx in remove_segment_allocated_lsns(), reading freed memory (ASAN heap-use-after-free, issue apache#67428, reproduced by the nonConcurrent pipeline on PRs apache#67404 and apache#67402). Fix: SharedMemtable now owns the shared_ptr<SegmentAllocatedLsnMap> captured from the group writer context at submission, and inserts/removes LSN entries through that map. A null map stands for "no LSN allocation", which is equivalent to RowsetWriterContext::need_allocated_lsn() because GroupRowsetWriter::init() creates the map exactly when LSN allocation is required. This keeps the precise cleanup dependency alive for the task teardown without extending the whole RowsetWriter lifetime. Also add SegmentAllocatedLsnMap::contains_segment() for test assertions, and regression tests covering: (a) the last token/writer owner released while a group flush task finishes, (b) a queued subtask running after its weak token expired, (c) cancellation, plus LSN-entry cleanup assertions on the flush-error path. Verified with and without the fix under ASAN: the new tests reproduce the exact use-after-free at RowsetWriterContext::remove_segment_allocated_lsns without the fix and pass with it.
bosswnx
requested review from
gavinchou,
liaoxin01 and
yiguolei
as code owners
September 2, 2026 07:57
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 16915 ms |
Contributor
TPC-DS: Total hot run time: 82483 ms |
Contributor
ClickBench: Total hot run time: 14.58 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: close #67428
Related PR: #66889 (introduced the UAF)
Problem Summary:
ASAN heap-use-after-free in
SharedMemtable::~SharedMemtable()during group-commit memtable flush task teardown, introduced by #66889.PartOfGroupMemtableFlushTaskholds itsSharedMemtablebyshared_ptrbut only aweak_ptrto theFlushToken. Whenrun()'s localshared_ptr<FlushToken>drops the last reference at the end ofrun(), destruction cascadesFlushToken->RowsetWriter->RowsetWriterContext; the thread pool then destroys the task object, and~SharedMemtable()dereferences the dangling rawRowsetWriterContext* rowset_ctxinremove_segment_allocated_lsns(). Reproduced by the nonConcurrent pipeline on two unrelated PRs (#67404, #67402).What changed?
SharedMemtablenow ownsstd::shared_ptr<SegmentAllocatedLsnMap>captured from the group writer context at submission; insert/remove go through the owned map. A null map stands for "no LSN allocation" (equivalent toneed_allocated_lsn(), sinceGroupRowsetWriter::init()creates the map exactly when needed). This keeps the precise cleanup dependency alive without extending the wholeRowsetWriterlifetime — the approach recommended in the [Bug] BE crash (ASAN heap-use-after-free) in SharedMemtable dtor during group commit flush #67428 triage.SegmentAllocatedLsnMap::contains_segment()for test assertions.Verification
ASAN BE UT, both directions: without the fix the new tests abort with the exact reported UAF (
SUMMARY: AddressSanitizer: heap-use-after-free rowset_writer_context.h:202:9 in doris::RowsetWriterContext::remove_segment_allocated_lsns); with the fix theMemTableFlushExecutor*tests pass 8/8 (two runs).Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)