Storages: use-after-free race between SegmentReadTaskScheduler and SegmentReaderPoolManager shutdown (#10917)#10922
Conversation
…ReaderPoolManager shutdown (pingcap#10903) The schedLoop background thread could still be running when SegmentReaderPoolManager::stop() destroyed all reader_pools and their WorkQueue mutexes. Subsequent scheduleOneRound() calls would then access already-destroyed objects, causing SIGSEGV. Fix by adding a public stop() method to SegmentReadTaskScheduler and calling it before SegmentReaderPoolManager::stop() in both production (Server.cpp) and test (gtests_dbms_main.cpp) shutdown paths. Also fix: - rename std::atomic<bool> stop -> stop_flag to avoid name collision - fix typo: reapPeningPools -> reapPendingPools in header comment - fix typo: max_free_threds -> max_free_threads in test main
SegmentReader threads may call pushMergedTask() for unfinished work after stop() has joined schedLoop. Since no scheduler remains to drain merged_task_pool, guard the method with isStop() to discard tasks instead of leaking them until singleton teardown.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/test pull-unit-next-gen |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JaySon-Huang, yongman The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
|
/test pull-unit-next-gen |
e25e749
into
pingcap:release-nextgen-202603
This is an automated cherry-pick of #10917
What problem does this PR solve?
Issue Number: close #10903
Problem Summary:
During shutdown,
SegmentReaderPoolManager::stop()destroys allreader_poolsand theirWorkQueuemutexes, butSegmentReadTaskScheduler'sschedLoopbackground thread is still running. WhenschedLoopwakes from its 2ms sleep, it callsscheduleOneRound()→SegmentReaderPoolManager::addTask()→ accesses already-destroyedreader_pools, causing SIGSEGV (use-after-free).What is changed and how it works?
Check List
Tests
Manual test:
./gtests_dbms --gtest_filter="DeltaMergeStoreTest.ReadLegacyStringDataCFTiny"— test passes and process exits cleanly without SIGSEGV.Side effects
Documentation
Release note
Summary by CodeRabbit
Release Notes