fix(dfm-search): eliminate cross-thread race on m_strategy causing heap corruption (cherry-pick to master) - #376
Conversation
There was a problem hiding this comment.
Sorry @pppanghu77, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Reviewer's GuideRefactors the search cancellation mechanism to use a shared engine-level atomic flag instead of cross-thread access to the worker’s strategy pointer, eliminating a race that caused heap corruption and tightening cancellation handling across synchronous and asynchronous searches. Sequence diagram for updated search cancellation flowsequenceDiagram
actor Client
participant GenericSearchEngine
participant SearchWorker
participant BaseSearchStrategy
Client->>GenericSearchEngine: init()
GenericSearchEngine->>SearchWorker: setEngineCancelledFlag(&m_cancelled)
Client->>GenericSearchEngine: search(query, options, searchType)
GenericSearchEngine->>SearchWorker: doSearch(query, options, searchType)
SearchWorker->>BaseSearchStrategy: search(query)
SearchWorker->>BaseSearchStrategy: setCancelledFlag(m_engineCancelled)
Client->>GenericSearchEngine: cancel()
GenericSearchEngine->>GenericSearchEngine: m_cancelled.store(true)
loop [search in worker thread]
BaseSearchStrategy->>BaseSearchStrategy: [check m_cancelledRef->load()]
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review★ 总体评分:95分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // basesearchstrategy.h
class BaseSearchStrategy : public QObject
{
// ... 其他代码保持不变 ...
protected:
/**
* @brief 检查当前是否已被取消
* @return 若标志已注入且为true则返回true,否则返回false
*/
inline bool isCancelled() const
{
return m_cancelledRef && m_cancelledRef->load(std::memory_order_relaxed);
}
SearchOptions m_options;
SearchResultList m_results;
std::atomic<bool> *m_cancelledRef { nullptr };
};
// 各策略中简化调用(以 ContentIndexedStrategy 为例)
void ContentIndexedStrategy::processSearchResults(const Lucene::IndexSearcherPtr &searcher, ...)
{
// ...
for (int32_t i = 0; i < docsSize; ++i) {
if (isCancelled()) { // 旧:if (m_cancelledRef && m_cancelledRef->load())
qInfo() << "Content search cancelled";
break;
}
// ...
}
}
void FileNameRealTimeStrategy::search(const SearchQuery &query)
{
// ...
while (!directoryStack.isEmpty() && count < maxResults && !isCancelled()) {
// ...
for (const QFileInfo &info : std::as_const(entries)) {
if (isCancelled() || count >= maxResults) {
break;
}
// ...
}
}
} |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pppanghu77 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
2c6d10b to
0b86533
Compare
Cherry-pick of PR #374 (commit 290eec5) to master.
修复内容
修复 BUG-372535:搜索压测过程中产生文件管理器 coredump。
根因:util-dfm 的可取消搜索设计中,
SearchWorker::cancelSearch()经Qt::DirectConnection在调用线程读取 worker 的std::unique_ptr<BaseSearchStrategy> m_strategy,与doSearch()(引擎工作线程)对该指针的创建/重置形成跨线程无锁竞争,导致堆损坏 → Lucene++ 分配内存时 SIGBUS。改动
cancelSearch()槽与requestCancel信号,主线程不再经DirectConnection读 worker 的m_strategysetCancelledFlag()向策略注入引擎级 atomic 取消标志,策略改读m_cancelledRefBaseSearchStrategy::m_cancelled,改用运行时检查(非 Q_ASSERT,Release 构建会被移除)m_engineCancelled为空,doSearch报错并中止m_cancelledRef解引用处增加空指针防御SearchCancellationGuard构造前用局部 dummy 兜底,避免空指针解引用doSyncSearch()开头重置m_cancelled,超时置位,修复同步搜索取消PMS: https://pms.uniontech.com/bug-view-372535.html
Original PR (V20 develop/meagle-20260526): #374
Cherry-pick of commit 290eec5, clean (no conflicts).
Summary by Sourcery
Refactor the search cancellation mechanism to use an engine-level atomic flag instead of cross-thread access to strategy state, preventing races and heap corruption during dfm search.
Bug Fixes:
Enhancements: